mt-mods / moretrees

Other
8 stars 10 forks source link

Adding support for the wood_types mod #4

Closed JordanL2 closed 2 years ago

JordanL2 commented 2 years ago

Hi,

This PR is mostly to get feedback on a new mod idea I've created which should massively simplify how different mods that provide types of wood, and items made of wood (by which I mean, items that have a different texture depending on the type of wood they were made from), work together. The idea is to take the current many-to-many relationship where each mod that adds wooden items needs to be aware of and check the presence of every mod that adds wood types to the game, and replace it with a single point that each mod interacts with.

This mod hasn't been submitted to ContentDB yet as I'd prefer some feedback on the idea first.

This is the mod: https://github.com/JordanL2/wood_types

The readme should describe the idea and approach pretty well, by all means ask me any questions if anything is unclear.

As an example, here is a commit showing how a wooden-item-providing mod, such as gates_long which adds a new type of wooden gate, can be massively simplified: https://github.com/JordanL2/gates_long/commit/0d52b39ef40e82bcf56d749cd4fc32d1d4caa8c8

If feedback is positive, my next steps will be to submit this mod to ContentDB, then get support added to the various mods that add wood types such as this mod and EtherealNG, then add support to the various mods that add wooden items such as ts_furniture.

JordanL2 commented 2 years ago

Well I've found a problem already (despite testing this yesterday) - depending on the order the mods get loaded, the callback to the item-providing mod could be called from the wood-providing mod, and this means it can't create items with the item-providing namespace as that's illegal. There's a chance this kills the whole idea, as the point is to avoid needing to load mods in a particular order as that requires mods knowing about each other.

What I'm after is a way for the item-providing mods to be able to trigger a function once all the other mods have finished loading, I'm not sure if there's a way to do that.

S-S-X commented 2 years ago

What I'm after is a way for the item-providing mods to be able to trigger a function once all the other mods have finished loading, I'm not sure if there's a way to do that.

This can be used if direct dependencies are not wanted or for some other reason things have to be done after other mods are loaded. Downside is only that overriding becomes more complicated but sometimes it is needed, cannot say if there's another way as I've not checked actual code and therefore probably do not understand whole concept yet.

JordanL2 commented 2 years ago

That function throws an error because core.get_current_modname() returns nil, which is called when you try registering a node.

The only working "solution" I've found so far is to not have a namespace for the items, which is obviously bad.

JordanL2 commented 2 years ago

It also "works" if I prefix ":" to the node name, eg ":gates_long:fence_wide_etc", but this doesn't seem to set the mod namespace properly either.

EDIT: Actually I think the problem is just that unified inventory doesn't recognise the namespace, probably an issue with Unified Inventory and how it gets a list of the various mods / items.

EDIT2: The actual problem is that item.mod_origin returns '??' and there's no apparent way to override that.

JordanL2 commented 2 years ago

I've pushed a change to the wood_types mod, it's actually simpler now.

JordanL2 commented 2 years ago

I've found a way to fix the mod_origin field, it's kinda hacky though. https://github.com/JordanL2/gates_long/commit/d928b2a2661f57c35507afed38f11a0dc08b675a

S-S-X commented 2 years ago

Checked out mod, for me it looks like it is simply reinventing groups and callbacks. This might be good as it does make sure that mod registering wood types and mod registering wood items is ensured to know about wood_types mod and what it does.

Having standard groups would be best because mod does not really add anything, instead it simply acts as group filter that ensures knowledge about final use. Making that happen would be simpler and would provide universal way to solve problem what wood_types is attempting to solve.

Group misuse however would still be more likely for example because of simply not knowing what group means. Easier because it would not need any dependencies or API calls in mods providing wood types, simply having correct group for wood nodes would be sufficient.

Any item mod could use following but that would probably cause some problems because there's no standard groups really. Some mods might use wood group for all wooden items while some use it only for planks. Some mods might register unneeded duplicate nodes for tree group just to have some minor differences that should not generate new items.

minetest.register_on_mods_loaded(function()
  for nodename, nodedef in pairs(minetest.registered_nodes) do
    if nodedef.groups and nodedef.groups.tree then
      -- register item for this wood type
    end
  end
end)
JordanL2 commented 2 years ago

Does the use of group and callbacks enable a mod to register a different textured item per wood type? For example in gates_long, each gate has a different texture (and desc) depending on the wood type it was crafted with.

S-S-X commented 2 years ago

Does the use of group and callbacks enable a mod to register a different textured item per wood type? For example in gates_long, each gate has a different texture (and desc) depending on the wood type it was crafted with.

Yes

JordanL2 commented 2 years ago

EDIT: Somehow didn't read your comment till the end.

S-S-X commented 2 years ago

Would you mind showing how that can be done, using gates_long as an example?

minetest.register_on_mods_loaded(function()
  for nodename, nodedef in pairs(minetest.registered_nodes) do
    if nodedef.groups and nodedef.groups.tree then
      gates_long.register_gate(nodename, nodename:gsub(":","_"), nodedef.description, nodedef.tiles[1])
    end
  end
end)

That is basics to get idea and will only work for some nodes, for real world use you'd want to either want to look into tiles and figure out definition type to get usable texture out of it. See moreblocks mod for examples (it does very similar things).

It is mostly about having more fine grained control (using API) or trusting groups (currently not really usable because there's no standrads).

JordanL2 commented 2 years ago

You're right that works exactly the same, still requires the hack to override mod_origin unfortunately.

S-S-X commented 2 years ago

You're right that works exactly the same, still requires the hack to override mod_origin unfortunately.

I think I do not understand this could you elaborate a bit why mod_origin should be overridden?

JordanL2 commented 2 years ago

You're right that works exactly the same, still requires the hack to override mod_origin unfortunately.

I think I do not understand this could you elaborate a bit why mod_origin should be overridden?

If a node is registered outside of the mod's init phase, then core.get_current_modname() returns nil and so the node/item's mod_origin field is set to '??'. This is visible if you use something like Unified Inventory which tells you what mod each item came from. The only time you can override this field is after the nodes have actually been properly registered, so you need to add a callback on something like minetest.register_on_prejoinplayer to override it.

JordanL2 commented 2 years ago

Here's the change to gates_long using the wood group: https://github.com/JordanL2/gates_long/commit/33baa9444053a810b8ddfa9314b4d240e5cbf069

JordanL2 commented 2 years ago

Anyway I agree, standard groups are the actual solution here.

S-S-X commented 2 years ago

Anyway I agree, standard groups are the actual solution here.

Yeah, it would be very nice if that could be pushed far enough to actually have standard groups. Going that route, while it definitely would be better solution, might cause troubles (because of how groups are actually used). I do think it is doable but it will probably take some time before reaching mutual agreement for actually usable groups, if you're in hurry then maybe just provide another Redundant API™ 😄 (but then there's trouble getting dependencies and API calls to all other mods).

JordanL2 commented 2 years ago

Well exactly, it's less work to simply add a special group (e.g. planks) to the wood types in moretrees, ethereal etc than to add the API calls. And the integration from the item-providing mods is basically the same.

It might be even better to just use the wood group and raise bug tickets / PRs for any mods that misuse it, if/when the community agrees that wood should just be for planks.

S-S-X commented 2 years ago

if/when the community agrees that wood should just be for planks.

Well, it is possible that I might not agree with that 🤣 I could imagine that wood group should be for every item that contains mostly wood, any wood. And then groups (if needed) that add detail: trunk, plank, sheet, pole, stick, etc. I've not really used too much time to think about what would be most useful while still allowing most generic use cases. {wood=1,pole=1} and {metal=1,pole=1}.

And only after that comes group rating, what would {wood=3,pole=2} mean and why?

Standard groups would be nice but it will be very hard to create complete system that everyone can agree with... maybe this could be start. 👍

JordanL2 commented 2 years ago

I suppose one could argue the standard (for any mods for Minetest Game, anyway) is set by the items in Minetest Game. And that only put planks in the wood group. I understand your point that that standard might not be the best, and adding any wooden item to wood and having separate planks group for planks would be better. But I'd say there is currently already a convention.

JordanL2 commented 2 years ago

I guess the question is, should mods follow the convention set by the base game? I guess a page setting out the conventions clearly would be good.

S-S-X commented 2 years ago

I guess the question is, should mods follow the convention set by the base game? I guess a page setting out the conventions clearly would be good.

Probably simplest way to get things forward is to use whatever is already used most and attempt to reinforce that even if it would not be "best" option, then add things that are needed on top of that (at this point, when adding "new" groups that are not used as much yet, it could help if there would be some place documenting intentions).