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

Allow custom checks before allowing a player to protect new areas #74

Merged
merged 4 commits into from
Mar 22, 2024
Merged

Allow custom checks before allowing a player to protect new areas #74

merged 4 commits into from
Mar 22, 2024

Conversation

a-tour-ist
Copy link
Contributor

Implements #53, using the return values proposed there.
I removed the hardcoded restrictions in areas:canPlayerAddArea and re-registered them with the new api call.
This makes it possible to overwrite all conditions for modders and, more importantly, ensures that the error message from the first callback which failed is returned.

@a-tour-ist a-tour-ist changed the title Potection conditions Allow custom checks before allowing a player to protect new areas Mar 15, 2024
@a-tour-ist
Copy link
Contributor Author

My local luacheck runs fine with this, I would appreciate a hint on what I am doing wrong...

@Niklp09
Copy link
Member

Niklp09 commented Mar 15, 2024

I would appreciate a hint on what I am doing wrong...

I had to manually approve the luacheck run for this PR since you're a first time contributor to this repo.
Code lgtm, good idea to add an API function for this.

Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

This is a very welcome change to allow better mod interoperability.

Provided a few suggestions. Feel free to reject if you think they don't make sense.

internal.lua Show resolved Hide resolved
api.md Outdated Show resolved Hide resolved
api.md Outdated Show resolved Hide resolved
Copy link
Member

@SmallJoker SmallJoker left a comment

Choose a reason for hiding this comment

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

Tried to create an area that is too large for a regular player. As expected, it still works.

LGTM. Will merge in a few days unless someone's got objections.

@a-tour-ist
Copy link
Contributor Author

Minetest does it like this: https://github.com/minetest/minetest/blob/master/builtin/common/register.lua#L56-L59
At the first glance it might be tempting to use the same table (minetest.callback_origins), which however is not documented, thus not guaranteed to exist or work as intended. A separate table would be needed within this mod.

Thanks for that great hint! I hope I did it in the right way now. If not I would appreciate if someone with deeper knowledge would take that over...

@SmallJoker
Copy link
Member

SmallJoker commented Mar 17, 2024

Tested by tampering tha function in internal.lua at L237:

AsyncErr: Lua: Runtime error from mod 'areas' in callback on_chat_message(): mods/areas/internal.lua:227: 
[Mod] areas: Invalid api usage from mod 'areas' in callback registerProtectionCondition() at mods/areas/internal.lua:237
stack traceback:
....

Your implementation works.

@SmallJoker SmallJoker merged commit 0bad0ec into minetest-mods:master Mar 22, 2024
1 check passed
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