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: for original terrain generator, keep a single gap of water at the borders #7883

Merged

Conversation

SamuXarick
Copy link
Contributor

  • When freeform_edges was on, NE and NW borders could have terrain adjacent to the void tiles
  • SE and SW borders minimum gap was 2-tile wide instead of 1

@SamuXarick SamuXarick force-pushed the original-generator-vs-freeform-edges branch from dbb41b2 to ca93dbd Compare December 30, 2019 17:34
@SamuXarick SamuXarick force-pushed the original-generator-vs-freeform-edges branch from ca93dbd to 9135a4d Compare February 9, 2020 20:43
LordAro
LordAro previously approved these changes Apr 13, 2020
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

Probably fine.

LordAro
LordAro previously approved these changes Sep 24, 2020
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

Sure.

@TrueBrain TrueBrain added candidate: yes This Pull Request is a candidate for being merged size: small This Pull Request is small, and should be relative easy to process labels Dec 14, 2020
Copy link
Member

@TrueBrain TrueBrain left a comment

Choose a reason for hiding this comment

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

I fixed the issues locally; will push it in a sec. Wanted to leave the feedback behind for posterity.

src/landscape.cpp Outdated Show resolved Hide resolved
src/landscape.cpp Show resolved Hide resolved
@TrueBrain
Copy link
Member

Ah, figured it. freeform_edges only sets x=0 and y=0 to VOID. That explains.

Reworked the patch a bit to have less lines. Will merge later :)

TrueBrain
TrueBrain previously approved these changes Dec 15, 2020
@frosch123
Copy link
Member

I think this is wrong.
With fixed edges there is supposed to be a border with flat water tiles.
While with freeform edges also the border can be sloped.

@TrueBrain
Copy link
Member

Nothing changed there, it can still be sloped.

What this changes, is what the generator outputs. It used to be that with 2 borders it should be sloped, but the other two always had 2 tiles of water, during generation. That just looks odd.

The only real choice here is, I guess, if it should generate sloped borders; if so, it should do that in all 4 borders ;) But that really does look ugly :)

@TrueBrain TrueBrain changed the title Fix: Ensure a minimum gap of water upon generating terrain with original generator Fix: for original terrain generator, always keep a gap of water at the borders Dec 15, 2020
…e borders

This means that for NE/NW, it should have one more in case of
freeform-edges, and in case of SE/SW it should have one less.

Reminder: freeform-edges only adds VOID tiles on X=0 and Y=0.
@TrueBrain TrueBrain changed the title Fix: for original terrain generator, always keep a gap of water at the borders Fix: for original terrain generator, keep a single gap of water at the borders Dec 15, 2020
@TrueBrain TrueBrain force-pushed the original-generator-vs-freeform-edges branch from 0c4918f to 90db639 Compare December 15, 2020 19:58
@TrueBrain TrueBrain merged commit 1d85d71 into OpenTTD:master Dec 15, 2020
@SamuXarick SamuXarick deleted the original-generator-vs-freeform-edges branch December 16, 2020 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
candidate: yes This Pull Request is a candidate for being merged 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.

None yet

4 participants