Mitigate crash if metadata is not valid (disconnected player?)

[S-S-X]

This is fairly uncommon crash, similar has happened with other mods and similar fixes were pushed around. For me it seems to be some fairly recent change on engine itself (5.3 or 5.4 possibly?).

Basically what happens is that player is gone but instance of player object is still partially valid, it will return some thing but a lot is missing. For example get_meta() can return nil. It seems to be most likely if we're running entity step (and player disappears / disconnected just before).

(similar seems to possibly happen if client crashes at the end of connection / joining process, not this bug necessarily but similar incomplete player instance)

[fluxionary]

You should probably do something similar for set_player_attribute

[S-S-X]

You should probably do something similar for set_player_attribute

That's not probably important or relevant becaus it seems to be only with recent engine versions with get_meta

[fluxionary]

You should probably do something similar for set_player_attribute

That's not probably important or relevant becaus it seems to be only with recent engine versions with get_meta

set_player_attribute calls player:get_meta() as well, without a nil check

[S-S-X]

You should probably do something similar for set_player_attribute

That's not probably important or relevant becaus it seems to be only with recent engine versions with get_meta

set_player_attribute calls player:get_meta() as well, without a nil check

What I mean get_meta was added here: https://github.com/minetest/minetest/commit/91615f9588420fd716978552fdacf1323b8df11c And this problem started happening only after fairly recent engine updates, it almost sure does not affect ancient minetest engines that calls through player:get_attribute.

[S-S-X]

Well, now I see what you meant. I was looking just at changes and mixed up set_player_attribute, get_player_attribute and player:get_attribute 🤣

Sure, could add similar check there 👍

[fluxionary]

i think i personally like the latter option, but i'll leave it up to you. btw, i don't have access to accept the PR, i'm just subscribed here cuz i've done several updates for this mod.

[SmallJoker]

get_meta only returns nil when the player is already gone: https://github.com/minetest/minetest/blob/2628316842e9c3fc1c54c6d57505c7851fa83490/src/script/lua_api/l_object.cpp#L1288-L1297

Please reproduce this error on 5.4.0 or newer and post the log here. The set_attribute backwards compat is fine by me, but the meta ~= nil check is not. This needs to be fixed not only there, but also its caller function.

[S-S-X]

get_meta only returns nil when the player is already gone: https://github.com/minetest/minetest/blob/2628316842e9c3fc1c54c6d57505c7851fa83490/src/script/lua_api/l_object.cpp#L1288-L1297

Please reproduce this error on 5.4.0 or newer and post the log here. The set_attribute backwards compat is fine by me, but the meta ~= nil check is not. This needs to be fixed not only there, but also its caller function.

Yes, that's exact issue and seems to happen only when player is gone but player object is still available. I can try to find (uninteresting) logs but yes it is 5.4.1 that will crash when player's gone but player instance is still accessible.

I think I've seen it happen only when entity step is running and never outside that.

What I mean by this: calling player:get_meta() will succeed but returns nil. So far it seems it happens if player is lost while entity step is running.