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

Better sapling grow system #3053

Merged
merged 8 commits into from
Sep 12, 2023
Merged

Conversation

aegroto
Copy link
Contributor

@aegroto aegroto commented Aug 23, 2023

This pull requests implements the changes proposed in issue #3050 to clean the sapling growth system implemented in the default game. My modifications add a generic register_sapling_growth function that may be linked as a public API, although I am not sure this is idiomatic in the way the game is developed. A further step may be to separate the tree growing system to a separate module such that other modules may register their own trees with ease and without interacting with "default".

@aegroto
Copy link
Contributor Author

aegroto commented Aug 30, 2023

@SmallJoker Thanks for your review. I have added a brief description of every public element added in my code, could you please take a look into it?

@aegroto
Copy link
Contributor Author

aegroto commented Sep 10, 2023

Are there any other patches required to my contribution? I may have some spare time during next week to fix any possible issue.

@appgurueu
Copy link
Contributor

I will try to get to reviewing this tomorrow.

@SmallJoker SmallJoker self-requested a review September 10, 2023 19:49
Copy link
Contributor

@appgurueu appgurueu left a comment

Choose a reason for hiding this comment

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

Made it a bit more concise, tested that saplings still grow.

Screenshot

Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

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

lgtm otherwise

game_api.txt Outdated Show resolved Hide resolved
mods/default/trees.lua Outdated Show resolved Hide resolved
mods/default/trees.lua Outdated Show resolved Hide resolved
@appgurueu appgurueu merged commit 59da46c into minetest:master Sep 12, 2023
1 check passed
MoNTE48 pushed a commit to MoNTE48/minetest_game that referenced this pull request Oct 8, 2023
---------
Co-authored-by: Lars Müller <[email protected]>
Co-authored-by: sfan5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants