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

Log bucket liquid placement #3108

Merged
merged 4 commits into from
Apr 13, 2024

Conversation

Emojigit
Copy link
Contributor

@Emojigit Emojigit commented Apr 2, 2024

This PR adds code to log liquid placements done via buckets. Previously, only failed attempts were logged.

This PR is ready for review.

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.

Code looks fine. text in log_action should probably be called something like action? Reordering the parameters to be in the order the message is built (roughly "<name> did <action> at <pos>") could also help make this slightly more readable IMO.

I can definitely see the logging for placing liquids being useful against griefers.

(Why log picking up liquids though?)

Tested briefly, does what it says on the tin, doesn't blow up.

I'm not sure if this technically counts as maintenance, but adding some more logging is by no means problematic.

@Emojigit
Copy link
Contributor Author

Emojigit commented Apr 4, 2024

Reordering the parameters to be in the order the message is built (roughly " did at ") could also help make this slightly more readable IMO.

I kept the order mostly for consistency with the check_protection function above.

(Why log picking up liquids though?)

Like the engine log digging node, IMO logging this is also needed.

@Emojigit Emojigit requested a review from appgurueu April 4, 2024 22:53
@sfan5 sfan5 merged commit 0639681 into minetest:master Apr 13, 2024
2 checks passed
MoNTE48 pushed a commit to MoNTE48/minetest_game that referenced this pull request Jun 2, 2024
Emojigit added a commit to C-C-Minetest-Server/wooden_bucket that referenced this pull request Jun 28, 2024
This PR ports minetest/minetest_game#3108 to wooden buckets. It logs liquid placement and removal.

This PR is ready for review.
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.

3 participants