Navigation Menu

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

added Bucket privilege to restrict bucket usage #2832

Closed
wants to merge 1 commit into from

Conversation

codeandfix
Copy link
Contributor

Griefers like the bucket, as it is not very safe if used near protected areas.

Griefers like the bucket, as it is not very safe if used near protected areas.
@sfan5
Copy link
Member

sfan5 commented Feb 17, 2021

As stated by the pinned issue, Minetest Game is in maintenance mode and not accepting new features. This is unlikely to be accepted.

@sfan5 sfan5 added the Feature label Feb 17, 2021
@SmallJoker
Copy link
Member

SmallJoker commented Feb 18, 2021

As stated by the pinned issue, Minetest Game is in maintenance mode and not accepting new features. This is unlikely to be accepted.

As a workaround proposal: expose and document the function check_protection as bucket.check_protection so that it can be overwritten/extended in mods. That would open up many more possibilities, such as height checks for lava buckets. For that, consider adding a 4th function parameter which contains the item/bucket name.

@codeandfix
Copy link
Contributor Author

smalljoker: you think about overwriting the check protection to check against a new priv bucket.
yes this could be a nice thing. maybe we should have a skeleton protection overwrite mod/api, where it is easier to customize more.
but in this case (bucket/fire) my problem is, that a player can start an ABM which will hurt protected areas.
so my proposal would be: check if an ABM runs from outside a protected area inside an protected area and deny this, if the first action was made by an unpriviliged player OR (if to complicate to check) just disallow any ABM acting from unprotected area into an protected area. otherwise you could ABM-grief (and thats what the bucket priv should prevent - ABM griefing)

@SmallJoker
Copy link
Member

SmallJoker commented Feb 20, 2021

@codeandfix I totally see why you'd like to have this in MTG, but the current state of the PR is not generic enough. If you would change check_protection to a public function, a custom mod of yours could append that privilege check to this function. Same result, but the final control about using buckets is within your mod, and not MTG.

@S-S-X
Copy link

S-S-X commented Feb 21, 2021

Add your own server customization mod to keep things clean, here for example XP is used to check if buckets can be used: https://github.com/pandorabox-io/pandorabox_custom/blob/master/onplace_restriction.lua

@codeandfix
Copy link
Contributor Author

i have to figure out more how overwriting works fine. i fear it wont work in most cases.
i have to overwrite mod functions instead of core functions.
most time i quick hack mods and have problems while upgrading.
but its much easier.

@S-S-X
Copy link

S-S-X commented Feb 19, 2023

i have to figure out more how overwriting works fine. i fear it wont work in most cases.
i have to overwrite mod functions instead of core functions.

How well overrides work depends on how overrides are implemented. For example that liquid onplace_restriction example does not actually outright override any existing code but wraps it inside custom privilege checks.

Actually rewriting parts of mod code will likely fail some day but wrapping mod code often seems to work forever without issues. That example been unchanged since 2019, some similar things been unchanged without issues way longer.
These kinds of overrides (straight wrapping without changes) also support chaining so multiple mods can override same thing without issues.

Basic function wrapping can often be easily implemented without overriding any of the original code, if it seems you'd need to override actual original code and have no option to wrap it unchanged then it is way more likely to cause issues with updates.

Simply put for usual cases:

  • if you're storing and calling original function then you're probably fine.
  • if you're not storing and not calling original function then you're probably asking for troubles.

@sfan5
Copy link
Member

sfan5 commented Dec 24, 2023

Stale, closing.

@sfan5 sfan5 closed this Dec 24, 2023
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.

None yet

4 participants