Add Lua function to remove a registered decoration, fix #12881

[Jeinzi]

Add compact, short information about your PR for easier understanding:

• The goal of this PR is to enable the removal of decorations through the Lua API, so they can be disabled or re-registered with altered parameters.
• This is done by implementing the new method ObjDefManager::clearByName(const std::string &name) and exposing it via the Lua function clear_decoration(name).
• This resolves A possibility to override and/or remove a single decoration #12881
• If the issue falls under "API convenience", then this relates to the roadmap
• This feature facilitates the reuse of mods, as they can be included without forking and modifying them, while still being able to exclude unwanted features. I think this is in the same spirit as functions like minetest.clear_craft(). In my specific use case, the mod "moonflowers" depends on "default" to define nodes on which to place the flowers on. I want to decouple the mod from default by exposing a method to re-register the flower decoration while allowing overrides of .place_on and/or .biomes.

To do

This PR is Ready for Review.

• I'll gladly add documentation if this implementation is approved

How to test

• Create a MTG world and look for a decoration, for example "default:dry_shrub"
• Close world, delete map.sqlite
• Include a mod that executes minetest.clear_decoration("default:dry_shrub")
• Log in to world to regenerate it, no more dry_shrub
• If this does not work, it might be necessary to include "mapgen" or "default" as optional dependency, so the registration of the decoration happens before the clear.

[appgurueu]

Thanks for the PR. The implementation looks reasonable. clearByName should probably be called removeByName / eraseByName to be consistent with C++ terminology.

Besides docs, it would be useful to have some basic testing code for this in devtest.

(At a glance it looks like objdef.(h|cpp) should be refactored to use a std::unordered_map, but that is out of scope for this PR.)

[wsor4035]

how much more complicated to add support for removing ore registration/biome? i understand biome might be harder since decorations and ores can be bound to biomes. just mentioning it since the three make up the most issues for mods (takes one bad apple to ruin the bunch)

[sfence]

Potential conflict with #14191.

[appgurueu]

Potential conflict with #14191.

Indeed. If I see this correctly, #14191 should be a superset of this?

[sfence]

Potential conflict with #14191.

Indeed. If I see this correctly, #14191 should be a superset of this?

Yes, from the functional side.

#14191 is realized only in lua, no method is added to mapgen in C++.

[Jeinzi]

That's correct, yes, and I think sfence has found the correct naming scheme with unregister_*(). However, I don't think the reregister_*() methods are a good idea. Their only purpose is to fix the issues caused by unregister_biome(), which are rooted in the already existing unregister_biomes(), and I think it would be much wiser to just fix that method.

[sfence]

That's correct, yes, and I think sfence has found the correct naming scheme with unregister_*(). However, I don't think the reregister_*() methods are a good idea. Their only purpose is to fix the issues caused by unregister_biome(), which are rooted in the already existing unregister_biomes(), and I think it would be much wiser to just fix that method.

There is a problem with biome IDs. In the time of decoration/ore registration, the biome names are translated to biome ids on C++ side. So, every change in biomes can make them invalid.

But, if we want to prevent the need for reregistration, it is probably the best way to use a similar schema as is used in register_abm/lbm. This means that the register/unregister function in Lua will change only with Lua tables and real registration on C++ side will be done at the same time as abm/lbm registration (after on_mods_loaded callbacks). So there will be no need to do some C++ changes.

[appgurueu]

But, if we want to prevent the need for reregistration, it is probably the best way to use a similar schema as is used in register_abm/lbm. This means that the register/unregister function in Lua will change only with Lua tables and real registration on C++ side will be done at the same time as abm/lbm registration (after on_mods_loaded callbacks). So there will be no need to do some C++ changes.

This seems like the best approach. I especially like that this curtails the complexity of the C++ code and gives modders decent flexibility.

[sfence]

This seems like the best approach. I especially like that this curtails the complexity of the C++ code and gives modders decent flexibility.

Ok, I update #14191 soon.