Minetest_game: stairs nodes overriding

Created on 15 Jan 2018  路  21Comments  路  Source: minetest/minetest_game

When registering a stair, it doesn't test if the node already exists.
So because the mod name is not put into the stairs node name, mods registering their stairs may likely accidentally override some stair node, e.g. for the node "somemod:stone".
I think stairs should omit a warning when redefining a stairs node.

Request / Suggestion Supported by core dev

Most helpful comment

You're right. However, l'd prefer a warning, which might be seen by somebody at some point in time, over no warning. Silently overriding some stairs node may not be noticed at all.

All 21 comments

I think stairs should omit a warning when redefining a stairs node.

Better add the mod name as part of the node's name.

@4w That would litter existing maps with unknown nodes next time they update.

This is why node aliases exist.

You'd have to add them for so many nodes across so many mods. Remember that not only the stairs mod would need to change, but every mod that uses it to register stairs, including outside MTG.

I don't disagree that it would have been the best way to do this from the start, but since it wasn't done that way and since this is a very rare issue, as a fix it seems worse than the problem.

You'd have to add them for so many nodes across so many mods

Mod owners have to, yes.

Your suggested fix: every mod that uses stairs needs to update regardless of whether the mod ever had this problem or maps are broken. Bad for mod makers, bad for server owners, bad for people with singleplayer worlds. Lots and lots and lots of messy aliases.

The current situation: A very occasional node is broken. (I've encountered one node effected by this on my server, running over 160 mods total). Can be fixed by the owner of that specific mod updating and adding one single alias. Creators of unaffected mods don't need to do anything. Maps are not broken.

I know which situation I'd prefer.

I know which situation I'd prefer.

A quick fix is easier and more convenient than a properly fixed long-term solution, yes.

I'm all for proper long term solutions, but you have to balance the importance of a fix against the disruption it would cause.

Remember this issue is very rare, easy to fix by either a mod creator or server owner when encountered, and even when encountered doesn't do any actual damage beyond cosmetic changes to a single node. It doesn't seem to justify the level of work for the number of people your solution would call for.

Why does this need any change by mod owners? Surely you can add the correct mod name to the given node name if it doesn't contain a : and then add the alias. All in stairs, no mod update needed

Because other mods register nodes through stairs and if you simply add mod names to the stairs node name as suggested, those stairs would have a different name than before?

Edit: You could just automatically register an aliases for every stair node as well, but that seems messy, though at least would remove the work load for everyone else.

You could just automatically register an aliases for every stair node as well, but that seems messy

Even if that works (i assume it will) it results in a large number of aliases. I don't like aliases unless absolutely essential, they have to stay until every part of a world is updated and resaved to database, until then you effectively have nodes with 2 different names in the world.

I think stairs should omit a warning when redefining a stairs node.

I agree with that.

@paramat It would also mean you register aliases on every stair node regardless of whether it's needed. A warning would be much better (but would also likely get lost in the logs and not noticed by most people, so I don't know how much it would really help.).

Warnings show up in yellow in terminal, very noticeable. Or we could make it a big red error in chat.

Ohh, Forgot there were coloured errors now. That would be fine I think. :)

4w, I've reworked the stairs mod in my minetest_game fork, such a change was disapproved in minetest_game some time ago (l don't know which issue/pr it was).

[A warning] would also likely get lost in the logs and not noticed by most people

mod developers should usually care about the warnings related to their mods when developing them.

@HybridDog I agree fully that they should care, but a mod developer is unlikely to be testing with every other mod that might happen to cause a conflict. It's the server owner (or singleplayer) that needs to note the warning and then potentially pass it back to mod developers, since it's their mod choices that are likely to uncover it.

You're right. However, l'd prefer a warning, which might be seen by somebody at some point in time, over no warning. Silently overriding some stairs node may not be noticed at all.

Yes, I agree with that completely.

There is support for a warning, anyone feel like making a PR?

@HybridDog could you provide a PR for a warning?

There's the PR: #2429

Was this page helpful?
0 / 5 - 0 ratings

Related issues

MarkuBu picture MarkuBu  路  5Comments

paramat picture paramat  路  3Comments

cx384 picture cx384  路  6Comments

Desour picture Desour  路  6Comments

Fixer-007 picture Fixer-007  路  6Comments