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 lower tree density if spreading is not enabled #8413

Merged
merged 1 commit into from Dec 24, 2020
Merged

Fix: Don't lower tree density if spreading is not enabled #8413

merged 1 commit into from Dec 24, 2020

Conversation

ldpl
Copy link
Contributor

@ldpl ldpl commented Dec 22, 2020

I started this in #8160 but apparently, it wasn't good enough as even though it prevented forests from disappearing completely they still thin out after a while which is undesirable, especially in realistic scenarios with custom tree density.

It may not be the best example (aka worst case) but still enough to illustrate the issue:
After map generation:
Screenshot from 2020-12-22 16-46-34
Current behaviour:
Screenshot from 2020-12-22 15-58-28
With this PR:
Screenshot from 2020-12-22 16-38-51

Savegame used for testing:
tree-test.tar.gz

@ldpl ldpl marked this pull request as draft December 22, 2020 13:52
@LordAro
Copy link
Member

LordAro commented Dec 22, 2020

LGTM, though there are spacing issues that need fixing ;)

@ldpl ldpl marked this pull request as ready for review December 22, 2020 14:34
@ldpl
Copy link
Contributor Author

ldpl commented Dec 22, 2020

I'm not quite used to such a fast reaction to PR's yet :p
But should be fine now, I fixed the issue with sparse forests.

@TrueBrain
Copy link
Member

Can you explain how your fix fixes the problem? Especially the moving of the if block, to me it reads as if that would make no different? Doesn't have to be lengthy, but if you can avoid me becoming a tree-expert, that would be appreciated :)

@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 22, 2020
@ldpl
Copy link
Contributor Author

ldpl commented Dec 22, 2020

@TrueBrain in previous order first block deletes old tree if there are > 1 on tile, second replants dead tree. So it deleted them until only 1 was left. Now replanting goes first so if spreading not allowed amount of trees on the tile (aka tree density) doesn't go lower.
It also adds a check before adding a new tree so it doesn't go higher either.

@TrueBrain TrueBrain added the preview This PR is receiving preview builds label Dec 22, 2020
@DorpsGek DorpsGek temporarily deployed to preview-pr-8413 December 22, 2020 18:54 Inactive
@ldpl ldpl mentioned this pull request Dec 22, 2020
Merged
@michicc michicc merged commit 04e5729 into OpenTTD:master Dec 24, 2020
@ldpl ldpl deleted the lush-forests branch September 19, 2021 20:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TrueBrain TrueBrain TrueBrain approved these changes

Assignees
No one assigned
Labels
candidate: yes This Pull Request is a candidate for being merged preview This PR is receiving preview builds 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
5 participants
@ldpl @LordAro @TrueBrain @michicc @DorpsGek