search

minetest-mods / 3d_armor

Visible player armor & wielded items for minetest
Other
17 stars 39 forks source link

[Question] About the use of detached inventories #126

Closed hlqkj closed 11 months ago

hlqkj commented 11 months ago

Why are they used in the first place?

Am I correct in assuming that this is just a legacy from the past, when we didn't have player inventory callbacks (that is, before https://github.com/minetest/minetest/pull/7185) or are there other reasons requiring them?

BuckarooBanzay commented 11 months ago

Why are they used in the first place?

Am I correct in assuming that this is just a legacy from the past

Highly likely, yes. If you have improvements feel free to open a PR :+1:

hlqkj commented 11 months ago

Thank you kindly for your answer. I was thinking about making some improvements indeed, but not being an API expert I was wondering if they were really worth.

In my understanding, the use of detached inventories should be kept at a minimum. I've read they were being "misused" in the past, causing heavy performance issues (see https://github.com/minetest/minetest/issues/4403). Fixes had been committed at the time, with the remark of them being just "temporary" and not to be relied on (see https://github.com/minetest/minetest/pull/4812 and API docs). I guess that, with time, they become a standard instead.

The point I'm now looking to clear, is if we could get a performance improvement by getting rid of per-user detached inventories, now that we could just use the player inventory callbacks on the player armor inventory, while letting the player acting on that one directly.

I wouldn't be able to say if that would give a real performance improvement or not, though? (Perhaps, a little less memory usage?) For sure the code would be simpler and cleaner.

Any thoughts on this would be appreciated :)

Edit: add reference to LUA API docs.