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

Jump to bottom

Fix: Don't allow towns to terraform certain floodable tiles #8492

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix: Don't allow towns to terraform certain floodable tiles #8492

wants to merge 1 commit into from

Conversation

SamuXarick
Copy link
Contributor

@SamuXarick SamuXarick commented Jan 3, 2021 •
edited

Motivation / Problem

road and house flooded
During world generation, town growth algorithm is speed up, allowing the town to perform multiple sequential growth actions, like building houses, roads, and terraforming. If it happens to lower terrain adjacent to sea and then in one of those sequences it builds a road or a house there, it will end up flooded during the "run tile loop" phase of world generation.
The result could be similar to what the screenshot shows: disconnected roads and/or houses.

This happens very rarely. I tried 12k towns 4096x4096 map, and detected no more than 3 flooded road or house tiles.

Description

Prevent rare occurrences of roads and houses being flooded, resulting in disconnected towns.

If the terraform command finds a certain floodable tile, the terraform fails. This works even for a sequence of commands during town generation. So far, the tests have not yet produced any flooded houses or roads.

Limitations

Void tiles were a problem. Terraform command ignores them. If the town terraformed one of the tiles at the edges of the map, there was no checking happening for the void tiles, and it could result in a flood coming in the direction of the void tile. I added checks coming from the edge tiles. Terraform command shall fail if the resulting slope on an edge tile is flat and has a height of zero.

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')

@SamuXarick SamuXarick marked this pull request as draft January 4, 2021 11:14
@SamuXarick SamuXarick changed the title Fix: Don't allow towns to terraform floodable tiles Fix: Don't allow towns to terraform certain floodable tiles Jan 5, 2021
@SamuXarick SamuXarick force-pushed the town-terraforming branch 4 times, most recently from bb1550b to 348b3c0 Compare January 5, 2021 15:36
@SamuXarick SamuXarick marked this pull request as ready for review January 5, 2021 16:02
@SamuXarick SamuXarick marked this pull request as draft January 6, 2021 13:35
@SamuXarick SamuXarick force-pushed the town-terraforming branch 3 times, most recently from c214542 to 0ba13de Compare January 8, 2021 19:38
@SamuXarick SamuXarick marked this pull request as ready for review January 12, 2021 14:50
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this pull request Jun 12, 2022
@SamuXarick
Debug: mark tiles where a flood exist
3939097 
This commit is used for debugging OpenTTD#8492.  Intended to use when starting a new game. Affected tiles will have a sign placed on them, so you can quickly get to them after world generation.

During world generation, town growth algorithm is speed up, allowing the town to perform multiple sequential growth actions, like building houses, roads, and terraforming. If it happens to lower terrain adjacent to sea and then in one of those sequences it builds a road or a house there, it will end up flooded during the "run tile loop" phase of world generation.
@SamuXarick
Copy link
Contributor Author

You can use this commit SamuXarick@3939097 for debugging purposes, to check the existance of flooded tiles after a world is generated.

2TallTyler
2TallTyler previously approved these changes Nov 9, 2022
View reviewed changes
Copy link
Member

@2TallTyler 2TallTyler left a comment

Choose a reason for hiding this comment

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

I'm not convinced this problem occurs frequently enough to worry about it, but that's not a good reason to unilaterally close this PR or continue to leave it in limbo. Code looks fine to me.

michicc
michicc reviewed Nov 26, 2022
View reviewed changes
src/water_cmd.cpp Outdated Show resolved Hide resolved
src/water_cmd.cpp Outdated Show resolved Hide resolved
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this pull request Dec 7, 2022
@SamuXarick
Debug: mark tiles where a flood exist
5fa4e65 
This commit is used for debugging OpenTTD#8492.  Intended to use when starting a new game. Affected tiles will have a sign placed on them, so you can quickly get to them after world generation.

During world generation, town growth algorithm is speed up, allowing the town to perform multiple sequential growth actions, like building houses, roads, and terraforming. If it happens to lower terrain adjacent to sea and then in one of those sequences it builds a road or a house there, it will end up flooded during the "run tile loop" phase of world generation.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this pull request Dec 7, 2022
@SamuXarick
Debug: mark tiles where a flood exist
b2dc520 
This commit is used for debugging OpenTTD#8492.  Intended to use when starting a new game. Affected tiles will have a sign placed on them, so you can quickly get to them after world generation.

During world generation, town growth algorithm is speed up, allowing the town to perform multiple sequential growth actions, like building houses, roads, and terraforming. If it happens to lower terrain adjacent to sea and then in one of those sequences it builds a road or a house there, it will end up flooded during the "run tile loop" phase of world generation.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this pull request Dec 7, 2022
@SamuXarick
Debug: mark tiles where a flood exist
eb1b651 
This commit is used for debugging OpenTTD#8492.  Intended to use when starting a new game. Affected tiles will have a sign placed on them, so you can quickly get to them after world generation.

During world generation, town growth algorithm is speed up, allowing the town to perform multiple sequential growth actions, like building houses, roads, and terraforming. If it happens to lower terrain adjacent to sea and then in one of those sequences it builds a road or a house there, it will end up flooded during the "run tile loop" phase of world generation.
@2TallTyler
Copy link
Member

@SamuXarick You mentioned on Discord that this may have been broken by my fix to wide river terraforming. Is this ready for review?

@2TallTyler 2TallTyler added size: small This Pull Request is small, and should be relative easy to process waiting on author labels Dec 28, 2022
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this pull request Jan 13, 2023
@SamuXarick
Debug: mark tiles where a flood exist
6ff7cce 
This commit is used for debugging OpenTTD#8492.  Intended to use when starting a new game. Affected tiles will have a sign placed on them, so you can quickly get to them after world generation.

During world generation, town growth algorithm is speed up, allowing the town to perform multiple sequential growth actions, like building houses, roads, and terraforming. If it happens to lower terrain adjacent to sea and then in one of those sequences it builds a road or a house there, it will end up flooded during the "run tile loop" phase of world generation.
@SamuXarick
Copy link
Contributor Author

This is ready for review, but... Wide river generator needs to water the tiles that terraform left behind for my fix to work properly.

@2TallTyler 2TallTyler mentioned this pull request Jan 13, 2023
Merged
2TallTyler
2TallTyler reviewed Jan 16, 2023
View reviewed changes
src/clear_cmd.cpp Show resolved Hide resolved
src/water_cmd.cpp Outdated Show resolved Hide resolved
src/water_cmd.cpp Outdated Show resolved Hide resolved
src/water_cmd.cpp Outdated Show resolved Hide resolved
src/water_cmd.cpp Outdated Show resolved Hide resolved
2TallTyler
2TallTyler previously approved these changes Jan 16, 2023
View reviewed changes
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this pull request Jan 21, 2023
@SamuXarick
Debug: mark tiles where a flood exist
01dfa0d 
This commit is used for debugging OpenTTD#8492.  Intended to use when starting a new game. Affected tiles will have a sign placed on them, so you can quickly get to them after world generation.

During world generation, town growth algorithm is speed up, allowing the town to perform multiple sequential growth actions, like building houses, roads, and terraforming. If it happens to lower terrain adjacent to sea and then in one of those sequences it builds a road or a house there, it will end up flooded during the "run tile loop" phase of world generation.
@SamuXarick SamuXarick force-pushed the town-terraforming branch 2 times, most recently from 9587490 to 75b55ff Compare March 2, 2023 13:36
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this pull request Jan 23, 2024
@SamuXarick
Debug: mark tiles where a flood exist
f643c6e 
This commit is used for debugging OpenTTD#8492.  Intended to use when starting a new game. Affected tiles will have a sign placed on them, so you can quickly get to them after world generation.

During world generation, town growth algorithm is speed up, allowing the town to perform multiple sequential growth actions, like building houses, roads, and terraforming. If it happens to lower terrain adjacent to sea and then in one of those sequences it builds a road or a house there, it will end up flooded during the "run tile loop" phase of world generation.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this pull request Jan 23, 2024
@SamuXarick
Debug: mark tiles where a flood exist
0958ef0 
This commit is used for debugging OpenTTD#8492.  Intended to use when starting a new game. Affected tiles will have a sign placed on them, so you can quickly get to them after world generation.

During world generation, town growth algorithm is speed up, allowing the town to perform multiple sequential growth actions, like building houses, roads, and terraforming. If it happens to lower terrain adjacent to sea and then in one of those sequences it builds a road or a house there, it will end up flooded during the "run tile loop" phase of world generation.
@SamuXarick
Fix: Don't allow towns to terraform certain floodable tiles
2d4e397 
Prevent rare occurrences of roads and houses being flooded, resulting in disconnected towns.
SamuXarick added a commit to SamuXarick/OpenTTD that referenced this pull request Mar 9, 2024
@SamuXarick
Debug: mark tiles where a flood exist
c870ebe 
This commit is used for debugging OpenTTD#8492.  Intended to use when starting a new game. Affected tiles will have a sign placed on them, so you can quickly get to them after world generation.

During world generation, town growth algorithm is speed up, allowing the town to perform multiple sequential growth actions, like building houses, roads, and terraforming. If it happens to lower terrain adjacent to sea and then in one of those sequences it builds a road or a house there, it will end up flooded during the "run tile loop" phase of world generation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michicc michicc michicc left review comments

@2TallTyler 2TallTyler 2TallTyler left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
size: small This Pull Request is small, and should be relative easy to process
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@SamuXarick @2TallTyler @michicc