Add Lua API function to retrieve node counts for an area#16948
Add Lua API function to retrieve node counts for an area#16948kromka-chleba wants to merge 4 commits intoluanti-org:masterfrom
Conversation
Desour
left a comment
There was a problem hiding this comment.
There is a content id cache, used by abms. This PR as of now doesn't use it, but iterates over all nodes.
Exposing the content id cache is dangerous in a way that it forces all future versions of luanti to keep a cache with the exact semantics of the api.
Hence it needs to be in a way that it allows false positives, or only returns hints. Note also that right now the cache is not updated on each set_node.
We could also go as far as to manage a content_id -> count map for each block.
==> The api design is the hard part here (and finding consensus).
I know the cache exists but I'd like to avoid touching that for the very reasons you mentioned.
Well, I could make it count nodes too, but I don't feel like maintaining a cache for this - I'm probably not competent enough to implement a caching system. Though I could try if you find this necessary for this feature to get merged. |
|
Ah, so what you want to add here is basically a new bulk node reading operation. (Like |
Pretty much yes. EDIT: So the idea is to simply know what's in a mapblock without fetching any extra position data which would be heavy. Though counting nodes by type sounds useful - this could be used to assign priority for mapblocks (e.g. process mapblock with more tree nodes first). |
I would prefer that (id + count) over the current PR implementation (id + name) as it's more helpful for VManip (which is often used for mapblock processing). We already have a LUT for node names <--> content ID. No need to have another API that does almost the same thing. |
Yeah, sounds reasonable. I'll do this. |
|
Reworked this into |
|
I added a test command for devtest. |
b163cfc to
91a7950
Compare
Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
91a7950 to
0765856
Compare
|
How often do mods want all nodes and not just a specific set of nodes they're interested in (which is already possible with Another important API design point to consider is: why are we needlessly introducing a "dependency" on map blocks?
This data ceases to exist as soon as the block is deserialized. |
After rethinking we may mimic the behavior of
This API is intended to be a low-level building block for ABM/LBM-like systems, so I picked mapblock for simplicity and performance. I could redo this to handle an arbitrary area though. As for why not use |
…ased signature Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
|
Honestly I find this new API more similar to |
Yes, I know, but APIs should have the broadest possible usability (to a reasonable extent).
Then this could rather be an extension of
And why is that? |
You mean as in returning node counts along positions (feels redundant) or returning only node counts when some argument passed to the function
The initial implementation of this function returned all the data we may ever need in a single call when given If you believe this API (or the previous iterations of this API) is not useful then say so and I will close this PR, alternatively propose a better solution. I could also try measuring time for |
I totally missed that I'll test these to see if there's a meaningful difference. |
Some initial benchmarks with this command: https://github.com/kromka-chleba/minetest/blob/9e4f13fdcf9cc4260d0eb7ebc51ed4654bef3531/games/devtest/mods/benchmarks/init.lua#L352 Need do confirm this is not related to execution order and that the AI benchmark doesn't lie. |
|
Note that I highlighted the |
Right, will recheck then. |
|
I mean I'm sure you will find out that your new function is still faster, after all it is doing less work. |
Still between 40-70x faster.
I don't think 150 ms instead of 2-3 ms is acceptable just to get node counts for an area roughly the size of a mapchunk. For my use case I don't want to get node positions, I just want node counts, as fast as possible. In 150 ms we could process an area the size of a mapchunk several times with a VM. This is wasted time. Or are you suggestting that I should rework |
|
Also the interface of So if I understand this correctly: If If
So you can either get grouped position lists (no counts provided by the API) or a flat position list plus exact counts. This API is asymmetric: So I could reverse the logic here: why stuff 2 use cases into a single function if there are 2 other use cases not covered by it (the above example and my use case) AND the proposed API can cover a huge chunk of this functionality in just 2 ms? Time saved by piggybacking the counting feature into the node finding feature is marginal and IMO these two should be separated and |
|
What are you working on that requires reimplementing custom lbm-like systems? And what would it do that does not require fetching the node positions in some other way? I agree with sfan5 that something being faster (even significantly) is enough unless it has a broad use case that is not covered by existing API functionality. The PR could probably also use a devtest benchmarking command to measure the difference (like |
I want to implement seasonal changes for Exile which requires changing soil to a dark winter variant, placing snow, etc. up to the horizon. ABMs/LBMs only work in the active block area which is smaller than the area of all loaded blocks, they're also slower because they work on single nodes, while my custom system spawns a Voxel Manipulator for the whole mapblock. They also suffer from other performance issues such as #14011 EDIT: Also until/unless #15992 gets merged, ABMs are really inflexible.
As mentioned above, I use VMs and need it to work for all loaded blocks. The voxel manipulator can detect positions by just iterating over the flat array. I believe passing positions to a VM could be actually slower.
The limit for ABMs is 200 ms per global step. Meanwhile with Will ABMs be changed to work for all loaded blocks? I doubt it. Are they working for my use case? No, they're slow which is why I tried implementing this #16910 TL;DR I wouldn't be wasting my time writing these PRs if what we already have was enough for my use case.
I had it on another branch but can add it here also. EDIT: I included the benchmark, it could be removed later or split into two benchmarks. |
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: kromka-chleba <65868699+kromka-chleba@users.noreply.github.com>
I think that use case sounds reasonable. Also saw the the mapblock callback API and I agree it would be useful. Also off-topic but I think an API to swap the node textures of clients at run time would be useful. That could for example be used for seasons by making green leaves brown ones during autumn without a performance cost. It would also work for any render distance. I have not bothered making a feature request for it because I am not working on anything that would use it. I understand your use case is broader than that (placing snow etc), but still want to throw out the idea. |
I considered that and sfence implemented something like this: #13811 |
I was more thinking about something like |



Goal of the PR
To add a way to get the number of nodes of each type for a given mapblock from Lua by introducing the
core.get_node_counts_in_area(pos1, pos2, nodenames)function. This allows modders to know the contents of an area without having to use ABMs/LBMs or manually iterate over the VM array. Together with #16910 this would allow modders to implement custom ABM/LBM-like systems without the limitations of ABMs/LBMs.Does this relate to a goal in the roadmap?
Probably not.
If you have used an LLM/AI to help with code or assets, you must disclose this.
Copilot helped me implement this.
To do
This PR is Ready
How to test
Use the
/test_get_node_countscommand.