BluebirdGreycoat / musttest_game

Other
1 stars 1 forks source link

Found another one (sorry!) #6

Closed hlqkj closed 4 months ago

hlqkj commented 5 months ago

This is actually not happening:

https://github.com/BluebirdGreycoat/musttest_game/blob/b8e3ebfa3d39176f4027ae066f71670b8a4b9e73/mods/welcome_msg/joinspec.lua#L58-L59

Happened to me yesterday: I somehow got disconnected while AFK, and dead. (Uh...) When I got back, the respawn form was correctly shown but hitting the button didn't help.

Seems the on_respawnplayer callback doesn't get called at all, with all the related consequences: no respawn in bed; no starting items in the Outback; permanent bones nohack state; anything else?

Official Minetest 5.8.0 (uncracked, of course. Me no like cracks...)

Edit: there actually is a way out of this, that is dying again, possibly without picking anything up so to avoid coffin deployment and thus preserve XP. However, I'm reporting because non-tech players wouldn't even realize what happened, and eventually try to get back on their tracks using the Outback gate.

hlqkj commented 5 months ago

I think I may have worked this out, but not sure at all.

  1. Player logs out while dead.
  2. Player logs back in and, to Mr. Server's surprise, they are still dead, so server conveniently sends them the death formspec.
  3. Soon after the XP mod does this: https://github.com/BluebirdGreycoat/musttest_game/blob/b8e3ebfa3d39176f4027ae066f71670b8a4b9e73/mods/xp/init.lua#L82
  4. Player clicks the "respawn" button.
  5. Client politely forwards the respawn request to server.
  6. Server readily ignores it because - according to his lazy definition of a dead player - the player is not actually dead anymore due to previous point 3.
  7. Player <hlqkj> respawns in the Outback and enters complaining mode.
Curiosity time. Why do we have the `cur_hp` stored in the player meta at all? Is this to cover for some hack able to circumvent the engine persisting the player's HP?
BluebirdGreycoat commented 5 months ago

Curiosity time. Why do we have the cur_hp stored in the player meta at all? Is this to cover for some hack able to circumvent the engine persisting the player's HP?

IIRC it's actually because the engine DOESN'T persist the player's HP. I don't know if this behavior is vanilla or is being triggered by something else in musttest_game, but it was certainly something I discovered and had to code around the last time the server had a major version update. OR, this could potentially be due to the engine refusing to persist an HP that's higher than the default 20.

BluebirdGreycoat commented 5 months ago

If you've got time and interest, could you do some more tests? I can't see how this can be happening unless the xp.update_players_max_hp function is somehow resurrecting you (hp > 0) before you can press the respawn button.

BluebirdGreycoat commented 5 months ago

cur_hp = max_hp on line 82 should only ever be happening for new players. Established players should be ignoring that line per the condition on line 80.

hlqkj commented 5 months ago

The condition on line 80 actually pass for everyone logging in with zero hp: when reaching there cur_hp is zero, both line 71 and 76 assign it like that (see also line 119).

I'll do another test later today, but I think I've already tried to comment out line 82 yesterday and the respawn button was then working as expected: that is, login in the outback with no hearts, then respawn in bed with full hearts when pressing the button. (Without commenting, when logging in dead I already had a full hp bar even before pressing the respawn button, instead.)

hlqkj commented 5 months ago

Sorry for waiting. I did another test and confirm all the above: line 82 do happen for any player (incl. non-new) if they join while dead, aka zero hp.

hlqkj commented 5 months ago

The problem commenting that line, is that makes things work for non-new players, but breaks them for new players joining.

Details... I always give too many, they tell me! As you'll probably figure, this is a new player joining for the first time: ``` 2024-05-17 17:19:42: ACTION[Server]: iNewPlayyr [::ffff:*.*.*.*] joins game. List of players: iNewPlayyr 2024-05-17 17:19:42: ACTION[Server]: player died @ (-9223,4570,5861) 2024-05-17 17:19:42: ACTION[Server]: Put 0 XP in bones @ (-9223,4570,5861). 2024-05-17 17:19:42: ACTION[Server]: Put default:pick_wood in bones @ (-9223,4570,5861). 2024-05-17 17:19:42: ACTION[Server]: Put mobs:meat_mutton 10 in bones @ (-9223,4570,5861). 2024-05-17 17:19:42: ACTION[Server]: Put torches:torch_floor 10 in bones @ (-9223,4570,5861). 2024-05-17 17:19:42: ACTION[Server]: Put tinderbox:tinderbox in bones @ (-9223,4570,5861). 2024-05-17 17:19:42: ACTION[Server]: Put clock:calendar in bones @ (-9223,4570,5861). 2024-05-17 17:19:42: ACTION[Server]: Put flint_and_steel:flint_and_steel in bones @ (-9223,4570,5861). 2024-05-17 17:19:42: ACTION[Server]: Player died at (-9223,4570,5861): stackcount=6 2024-05-17 17:19:42: ACTION[Server]: Successfully spawned bones @ (-9223,4570,5861)! ``` Here I pressed the respawn button. ``` 2024-05-17 17:22:49: ACTION[Server]: iNewPlayyr initiates teleport to (-9223,4569,5860) 2024-05-17 17:22:49: ACTION[Server]: iNewPlayyr respawns at (-9223,4569,5861) 2024-05-17 17:22:49: ACTION[Emerge-0]: executing teleport callback for iNewPlayyr! 2024-05-17 17:22:49: ACTION[Emerge-0]: iNewPlayyr actually teleports to (-9223,4569,5860) ```
hlqkj commented 5 months ago

I challenged my creativity and ended up with this (below): seems to work for both new, and dead non-new joins.

The trick seems to use the last_login parameter passed to the on_joinplayer() callback to know for sure if a join is new or not, ref. API docs.

More details!! ```diff diff --git a/mods/xp/init.lua b/mods/xp/init.lua index 85c75c90..03970ab0 100644 --- a/mods/xp/init.lua +++ b/mods/xp/init.lua @@ -60,7 +60,7 @@ end -function xp.update_players_max_hp(pname, login) +function xp.update_players_max_hp(pname, login, new_player) local pref = minetest.get_player_by_name(pname) if not pref then return @@ -77,7 +77,7 @@ function xp.update_players_max_hp(pname, login) -- Should only happen for new players (and existing that don't have 'hp_max' -- in their meta info yet). - if max_hp == 0 or cur_hp == 0 then + if new_player then max_hp = minetest.PLAYER_MAX_HP_DEFAULT cur_hp = max_hp end @@ -110,8 +110,9 @@ end -function xp.on_joinplayer(player) - minetest.after(0, xp.update_players_max_hp, player:get_player_name(), true) +function xp.on_joinplayer(player, last_login) + local is_new_playyr = last_login == nil + minetest.after(0, xp.update_players_max_hp, player:get_player_name(), true, is_new_playyr) end function xp.on_leaveplayer(player) ```
BluebirdGreycoat commented 5 months ago

D'oh! Somehow I misread if max_hp == 0 or cur_hp == 0 then as if max_hp == 0 and cur_hp == 0 then!

I don't think I knew about that bit in the API docs, could be useful.

Ok, I think I've got some stuff to work with. Thanks for the info!

BluebirdGreycoat commented 5 months ago

Can I get away with a one-word fix? 974ba5a2e498f8dbe4e60dbcc4440cba66180a5f

hlqkj commented 4 months ago

Apologies - I didn't see this. Just tested it but somehow this didn't work. (It should have, though?) Doing more testing...

hlqkj commented 4 months ago

I'm arrogant, they say. So let me give you a suggestion: when you pull new code, also restart the server!

Edit, translation for others: it actually works, my bad: I forgot to restart the server after pulling the new code before testing it. Thank you very much BluebirdGreycoat! :)