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

Fix #7944: Demolishing locks built on rivers didn't always restore the river #7946

Merged

Conversation

@SamuXarick
Copy link
Contributor

SamuXarick commented Jan 17, 2020

No description provided.

@LordAro
Copy link
Member

LordAro commented Jan 21, 2020

Having tested it, it's not immediately clear why IsWaterTile does not work for the sloped water, but does work for the flat water. Should HasTileWaterGround be used for the ends of the lock as well?

@SamuXarick
Copy link
Contributor Author

SamuXarick commented Jan 21, 2020

It works for the flat water because...

The lower and upper tiles of the lock are getting WaterClass after the clear command in the case of object on river (rocky land). In Test mode, the WaterClass becomes WATER_CLASS_CANAL by failing IsWaterTile check, because the clear command was also in test mode. But in Exec mode, the tile resulting by the clear command is a river tile, without the rocks, WaterClass then is WATER_CLASS_RIVER.

MakeLock(tile, _current_company, dir, wc_lower, wc_upper, wc_middle); is only called in DC_EXEC mode, garanteeing the correct wc_upper/wc_lower.

Then when removing the lock, MakeWaterKeepingClass will ensure rivers are restored, by looking at the WaterClass of those 2 tiles.

In short, DC_TEST and DC_EXEC are being toyed around with the WaterClass for the upper/lower tiles. The middle tile is not. WaterClass is being checked before the clear command. For a river with rocks on it, IsTileWater test false, thus WaterClass for middle tile is WATER_CLASS_CANAL, but for HasTileWaterGround it returns WATER_CLASS_RIVER.

@LordAro
Copy link
Member

LordAro commented Feb 9, 2020

@SamuXarick This needs a rebase due to commit-checker changes, can you do?

@SamuXarick SamuXarick force-pushed the SamuXarick:lock-restore-river-on-rocky-land-river branch from db019e4 to 20dc446 Feb 9, 2020
@LordAro
LordAro approved these changes Feb 9, 2020
@nielsmh nielsmh merged commit 30fe001 into OpenTTD:master Feb 9, 2020
8 checks passed
8 checks passed
Commit checker
Details
OpenTTD CI Build #20200209.13 succeeded
Details
OpenTTD CI (Linux linux-amd64-clang-3.9) Linux linux-amd64-clang-3.9 succeeded
Details
OpenTTD CI (Linux linux-amd64-gcc-6) Linux linux-amd64-gcc-6 succeeded
Details
OpenTTD CI (Linux linux-i386-gcc-6) Linux linux-i386-gcc-6 succeeded
Details
OpenTTD CI (MacOS) MacOS succeeded
Details
OpenTTD CI (Windows Win32) Windows Win32 succeeded
Details
OpenTTD CI (Windows Win64) Windows Win64 succeeded
Details
@SamuXarick SamuXarick deleted the SamuXarick:lock-restore-river-on-rocky-land-river branch Feb 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.