Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Codechange: Make void tiles flood edge tiles, instead of edge tiles flooding themselves #8517

Merged
merged 1 commit into from Jan 26, 2023

Conversation

SamuXarick
Copy link
Contributor

Motivation / Problem

Tiles at the edges of the map only start to be flooded if they are of type Clear. If they are of any other type, there is no flood. It also requires the tile to be flat, a setting and at height 0. And visually, the shores of tiles adjacent to void don't look like they're watered.
Unnamed, 1950-01-01#10

Description

All problems are solved by allowing Void tiles to have an active flood behaviour. The rest of the code already handles how to draw nearby tiles, what tile types to flood. The flooding mechanics are all done from TileLoop_Water now.
Unnamed, 1950-01-01#11

Limitations

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? (label: 'backport requested')
  • This PR affects the save game format? (label 'savegame upgrade')
  • This PR affects the GS/AI API? (label 'needs review: Script API')
    • ai_changelog.hpp, gs_changelog.hpp need updating.
    • The compatibility wrappers (compat_*.nut) need updating.
  • This PR affects the NewGRF API? (label 'needs review: NewGRF')

@glx22
Copy link
Contributor

glx22 commented Jan 7, 2021

I don't really understand the problem (except the visual issue for shores), but the fix seems to be a hack to me.

@andythenorth
Copy link
Contributor

It's a perceived problem of consistency; those tiles in the example should have water edge. 😄

But I should have a pony also. I don't see my pony anywhere.

@glx22
Copy link
Contributor

glx22 commented Jan 7, 2021

After more reading of the diff, I think the problem is "edge tiles are supposed to self flood but the checks are too strict and most of them don't flood". So yes, giving flood power to void tiles may be a good solution.

@@ -1237,7 +1240,7 @@ void TileLoop_Water(TileIndex tile)
uint dir;
FOR_EACH_SET_BIT(dir, _flood_from_dirs[slope_here]) {
TileIndex dest = tile + TileOffsByDir((Direction)dir);
if (!IsValidTile(dest)) continue;
if (dest >= MapSize()) continue;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change here is to allow the check on void tiles, which are now FLOOD_ACTIVE. We don't want tile to dry up in this situation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't add an optional allow_void bool to IsValidTile() ? (just a general question to everyone, not a suggestion for you)
I think it could simplify some tests in other parts of the code (based on my quick search for >= MapSize().

@@ -1277,7 +1280,7 @@ void ConvertGroundTilesIntoWaterTiles()
FOR_EACH_SET_BIT(dir, _flood_from_dirs[slope & ~SLOPE_STEEP]) {
TileIndex dest = TileAddByDir(tile, (Direction)dir);
Slope slope_dest = GetTileSlope(dest) & ~SLOPE_STEEP;
if (slope_dest == SLOPE_FLAT || IsSlopeWithOneCornerRaised(slope_dest)) {
if (slope_dest == SLOPE_FLAT || IsSlopeWithOneCornerRaised(slope_dest) || IsTileType(dest, MP_VOID)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Void tiles also have slopes, and dest could be a void tile. But it may not always have the desired slope. To comply with the new flood behaviour of MP_VOID tiles, it does not matter which slope dest have. It floods from the void.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed something interesting. This is what ConvertGroundTilesIntoWaterTilesdo if RunTileLoopnever happens, in master:
screenshot3
It already makes shores of those tiles adjacent to void tiles (except a few corner cases). It means it is RunTileLoop that dry them, that's why we never get to see them in this state in a real game. This also means, the original ConvertGroundTilesIntoWaterTiles isn't consistent, it's bugged.

The few corner cases are these...
screenshot5
...which || IsTileType(dest, MP_VOID) solve...
screenshot4

@michicc
Copy link
Member

michicc commented Jan 26, 2023

Hmm, could you theoretically cheese this by having a non-freeform edge north or west (that has no MP_VOID) and then destroy all water?

@rubidium42
Copy link
Contributor

Hmm, could you theoretically cheese this by having a non-freeform edge north or west (that has no MP_VOID) and then destroy all water?

Then you first need to disable freeform edges completely, with the hidden setting. Otherwise the configuration in the world generation UI has no effect on the MP_VOID tiles, just on whether it'll have only water at the edge or whether it allows land during map creation.
But... with that hidden setting there is always a ring of water tiles around the map that you can't bulldoze, not even with the magic bulldozer. So, there it does not even need flooding from MP_VOID.

@michicc michicc merged commit 46dc6da into OpenTTD:master Jan 26, 2023
@SamuXarick SamuXarick deleted the flood-from-void-tiles branch January 26, 2023 21:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants