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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #10218: Sloped river tiles need water both up and downstream #10235

Merged
merged 2 commits into from Dec 16, 2022

Conversation

2TallTyler
Copy link
Member

Motivation / Problem

When widening rivers, we use a CircularTileSearch to attempt to terraform and make river tiles within the river's radius. This determines the desired slope automatically based on whether the target tile is part of a continuous row of sloped river tiles. This was missing a check to ensure the river is actually sloping down to the sea, so the row of sloped tiles could end up propagating inland, as seen in #10218.

I don't know what's going on with coast tiles being drawn in #10218, but this also avoids that happening. 馃檪

Description

Ensure that sloped tiles always have water both upstream and downstream from themselves.

New ocean tiles created by river terraforming will flood after river generation is complete, but on non-ocean tiles we can take a shortcut to create river tiles up and/or downstream, if they don't already exist.

Closes #10218.

A second commit moves a cur_slope update so it doesn't get run if no terraforming is needed, and gets rid of a RandomRange(1) (which always returns 0) that must be vestigial code from the patch's development which somehow snuck past review.

Limitations

It's still possible for a long river paralleling a coast to have a wide row of sloped tiles down to the sea. But this looks much less wrong than #10218 and is more of a design choice than a bug. 馃槈 I'm hesitant to write a lot of code to solve this and potentially introduce other edge cases where multiple rivers flow into the ocean near each other.

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 touches english.txt or translations? Check the guidelines
  • 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')

@2TallTyler 2TallTyler added the size: small This Pull Request is small, and should be relative easy to process label Dec 13, 2022
@2TallTyler 2TallTyler added this to the 13.0 milestone Dec 13, 2022
@2TallTyler 2TallTyler changed the title Fix 10218: Sloped river tiles need water both up and downstream Fix #10218: Sloped river tiles need water both up and downstream Dec 13, 2022
src/landscape.cpp Outdated Show resolved Hide resolved
@2TallTyler 2TallTyler merged commit 0116a42 into OpenTTD:master Dec 16, 2022
@2TallTyler 2TallTyler deleted the fix_rivers branch December 16, 2022 22:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size: small This Pull Request is small, and should be relative easy to process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Banks of wide rivers on sloped terrain
2 participants