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

Protector should not call can_dig when player is not digging #7

Open
S-S-X opened this issue Jul 5, 2020 · 4 comments
Open

Protector should not call can_dig when player is not digging #7

S-S-X opened this issue Jul 5, 2020 · 4 comments
Labels
bug Something isn't working

Comments

@S-S-X
Copy link
Member

S-S-X commented Jul 5, 2020

Roughly following info / chat message stuff should probably stay in protector.can_dig method but onlyowner or not is_member(meta, digger) and similar things that actually do protection checks should go to different method tied to is_protected checks
https://github.com/mt-mods/protector/blob/master/init.lua#L234-L274

Not sure how this should be / can be done well but basically only digging should trigger any chat messages and any other call to is_protected should only return true/false to indicate if area is protected or not.

It should be up to caller what to do if area is protected or is not protected, caller might then either just call through minetest.record_protection_violation(pos, name), send chat messages to player if needed or take any other action needed. No action taken should also be perfectly valid decision.

Basically this can_dig call should not be directly inside is_protected as is
https://github.com/mt-mods/protector/blob/master/init.lua#L283-L288

I have feeling that same should be done protection hurt/flip functionality, should be only called if actually digging but not when some other mod is checking through minetest.is_protected:
https://github.com/mt-mods/protector/blob/master/init.lua#L292-L326

edit.
Linking another issue related to this one: pandorabox-io/pandorabox.io#377
It might look like there were some changes to also fix protector issue but things around protector were not actually about this issue but instead:

  • I just forked that repo and therefore it would have been possible to fix issue.
  • in reality no ticket was actually opened for fork and in the end there actually was no solution provided by protector mod.
  • hangglider was fixed by simply removing is_protected call mt-mods/hangglider@e2d8721 which is just fine for hangglider
@S-S-X S-S-X added enhancement New feature or request bug Something isn't working and removed enhancement New feature or request labels Jul 5, 2020
@S-S-X
Copy link
Member Author

S-S-X commented Jul 5, 2020

Related upstream issue (closed, not fixed) https://notabug.org/TenPlus1/protector/issues/17
Dropping ping @BobFred7 and @BuckarooBanzay as both of you commented on upstream issue, maybe you already have some ideas for this?

@BuckarooBanzay
Copy link
Member

i think the infolevel parameter was intended for something like this:

protector/init.lua

Lines 189 to 192 in e618c6d

-- 0 for no info
-- 1 for "This area is owned by <owner> !" if you can't dig
-- 2 for "This area is owned by <owner>.
-- 3 for checking protector overlaps

We are talking about the protection-message spamming, right?

@S-S-X
Copy link
Member Author

S-S-X commented Jul 6, 2020

We are talking about the protection-message spamming, right?

Yes and I think infolevel 0 is not actually implemented at all.
Also it seems that those functions can_dig and is_protected could benefit from some refactoring.

@S-S-X
Copy link
Member Author

S-S-X commented Jul 6, 2020

Seems it would be good to get #6 merged first as it also changes those functions. Also imo changes in pr from upstream are going to right direction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants