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

Meta set nodedef #1118

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
@Ekdohibs
Copy link
Member

commented Jan 26, 2014

It is not finished yet, but I'm still opening the pull request...
I rebased celeron55's work, and added a few fixes.

@sfan5 sfan5 added the rebase needed label May 6, 2014

@RealBadAngel

This comment has been minimized.

Copy link
Contributor

commented Jun 25, 2014

Any news on it?

@ShadowNinja

This comment has been minimized.

Copy link
Member

commented Jun 25, 2014

Does this add the entire definition to the meta or does it only add the differences? IMO it should only store the differences, so that if you update the base definition later (eg, make it walkable) all nodes already in the world will work accordingly. It would also take much less disk space. You can still keep the full definition in memory, to increase performance.

@Ekdohibs

This comment has been minimized.

Copy link
Member Author

commented Jun 25, 2014

The entire C++ definition is added to the meta (so additional Lua-only fields made by the modders won't be added), but yes, I agree that can take a lot of space. However, since the nodedef is stored as a nodedef is serialized when sending it to the client, there is currently no way to write only some of the fields, and not the others. Therefore, should another protocol be used for serializing the nodedef?

@ShadowNinja

This comment has been minimized.

Copy link
Member

commented Jun 25, 2014

You should definitely only store the differences, for the reasons I've outlined. Perhaps Lua module format (core.serialize) or JSON format would work. MsgPack or similar would be better since it's smaller, but if the meta is compressed this might not be a big difference.

This also uses set_def/get_nodedef which is inconsistent, and get_nodedef is undocumented. get_(node)def should also use metatables with __index (Lua inheritance) on the differing table, inheriting from the base nodedef (for less memory use).
For example (pseudocode):

NodeDef nd = getNodeDef(pos); // Returns only differing nodedef
push_node_def(nd);
push_node_def_from_registered_nodes(nd.name);
// ^ core.registered_nodes[name].__index = core.registered_nodes[name]
lua_setmetatable(L, -2);
return 1;

EDIT: Another option would be to simply return the difference table and make mods check minetest.registered_nodes for missing fields. Also, can you mention how this affects performance now? I remember it was very slow before but that you've made it much faster.

@Ekdohibs

This comment has been minimized.

Copy link
Member Author

commented Jun 28, 2014

I rebased the code against latest master. However, you should try it because it is still slow (it can sometimes take up to 2 seconds to update a node...). I had found what was slow, but was unable to make it faster.

continue;
v3s16 blockpos_nodes = bp*MAP_BLOCKSIZE;
std::map<v3s16, ContentFeatures> *defs = &block->m_special_nodedefs;
if(!defs->empty())

This comment has been minimized.

Copy link
@Ekdohibs

Ekdohibs Jun 28, 2014

Author Member

This is the block that makes it slow.

@Ekdohibs Ekdohibs removed the rebase needed label Jun 29, 2014

@ShadowNinja ShadowNinja added this to the 0.5.0 milestone Jun 29, 2014

@ShadowNinja ShadowNinja added the WIP label Jun 29, 2014

@ShadowNinja ShadowNinja changed the title [WIP] Meta set nodedef Meta set nodedef Jun 29, 2014

@sapier sapier force-pushed the minetest:master branch 2 times, most recently to c24e075 Aug 19, 2014

@ShadowNinja ShadowNinja force-pushed the minetest:master branch 4 times, most recently to 56195dc Sep 20, 2014

@ShadowNinja ShadowNinja force-pushed the minetest:master branch from cba038e to b98e8d6 Oct 7, 2014

@Zeno- Zeno- force-pushed the minetest:master branch to a1db83e Nov 25, 2014

@kwolekr kwolekr force-pushed the minetest:master branch to 3f83ca2 Dec 25, 2014

@celeron55

This comment has been minimized.

Copy link
Member

commented Feb 19, 2015

I think it could be a good idea to store the difference in node definition as a serialized Lua table in the node metadata instead of the actual serialized C++ node definition or parts of it. This would radically improve the cross-version compatibility, which is a serious problem in the serialized node definition (it's not designed for this, and should not need to be designed for this), and would allow easily storing only the fields that have been modified. It would also be more understandable to modders.

@Ekdohibs Ekdohibs force-pushed the Ekdohibs:meta_set_nodedef_4 branch to ee4706b Feb 19, 2015

@Ekdohibs

This comment has been minimized.

Copy link
Member Author

commented Feb 19, 2015

Rebased against current version, but I did not add the different suggestions, nor did I finish to code push_content_features.
Again, meta_set_nodedef-ing large numbers of nodes is very mapblock_mesh-unfriendly, as you can test; however, it is usable.

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 5, 2017

Meta-set nodedef: Part 2
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 5, 2017

Meta-set nodedef: Part 3
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 5, 2017

Meta-set nodedef: Part 3
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 5, 2017

Meta-set nodedef: Part 3
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 5, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 5, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 6, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 7, 2017

Meta-set nodedef: Part 3
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 7, 2017

Meta-set nodedef: Part 3
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 7, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 7, 2017

Meta-set nodedef: Part 3
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 7, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 7, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 7, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 8, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 8, 2017

Meta-set nodedef: Part 3
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 8, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 8, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 8, 2017

Meta-set nodedef: Part 3
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 8, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 9, 2017

Meta-set nodedef: Part 3
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 9, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 9, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118

Thomas--S added a commit to Thomas--S/minetest that referenced this pull request Jun 9, 2017

Meta-set nodedef: Part 4
Mainly a refactor of parts of minetest#1118
@nerzhul

This comment has been minimized.

Copy link
Member

commented Jun 11, 2017

adopted by @Thomas--S

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.