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

Implement grouped mode for find_nodes_in_area #9888

Merged
merged 2 commits into from
Jul 14, 2020

Conversation

sfan5
Copy link
Collaborator

@sfan5 sfan5 commented May 17, 2020

usecase: luanti-org/minetest_game#2683

To do

This PR is Ready for Review.

@sfan5 sfan5 added @ Script API Feature ✨ PRs that add or enhance a feature labels May 17, 2020
@KaylebJay
Copy link

Good feature, thanks sfan5.

@sfan5
Copy link
Collaborator Author

sfan5 commented May 17, 2020

If anyone feels like testing: it would be interesting to know if this is faster than using a vmanip to read all nodes and then iterating them in Lua.

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.

Good optimizations, and the code looks good but lacks of comments.

src/script/lua_api/l_env.cpp Show resolved Hide resolved
src/script/lua_api/l_env.cpp Show resolved Hide resolved
src/script/lua_api/l_env.cpp Show resolved Hide resolved
* First return value: Table with all node positions
* Second return value: Table with the count of each node with the node name
as index.
* If `grouped` is true the return value is a table indexed by node name
Copy link
Contributor

@LoneWolfHT LoneWolfHT Jul 14, 2020

Choose a reason for hiding this comment

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

Can the node name be something like 'group:lava' if that was the nodename provided?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

No, the names returned are always nodes even if you put in a group.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmmm, I guess a modder could use minetest.get_item_group() if they wanted to apply to all nodes in a certain group.
I have no problems with this then

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.

Works, looks good.

@sfan5 sfan5 merged commit 4b4513a into luanti-org:master Jul 14, 2020
@sfan5 sfan5 deleted the nodesinarea branch July 14, 2020 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants