From 610ddaba7ce23248ec898ac68c44ca62c0ad73b4 Mon Sep 17 00:00:00 2001 From: sfence Date: Fri, 27 Sep 2024 21:34:52 +0200 Subject: [PATCH] Allow detection of damage greater than HP (#15160) Co-authored-by: Gregor Parzefall --- doc/lua_api.md | 5 ++ games/devtest/mods/unittests/player.lua | 78 +++++++++++++++++++++---- src/server/player_sao.cpp | 9 +-- 3 files changed, 77 insertions(+), 15 deletions(-) diff --git a/doc/lua_api.md b/doc/lua_api.md index 7623c1c5d..9c888ff13 100644 --- a/doc/lua_api.md +++ b/doc/lua_api.md @@ -5850,8 +5850,13 @@ Call these functions only at load time! * `clicker`: ObjectRef - Object that acted upon `player`, may or may not be a player * `minetest.register_on_player_hpchange(function(player, hp_change, reason), modifier)` * Called when the player gets damaged or healed + * When `hp == 0`, damage doesn't trigger this callback. + * When `hp == hp_max`, healing does still trigger this callback. * `player`: ObjectRef of the player * `hp_change`: the amount of change. Negative when it is damage. + * Historically, the new HP value was clamped to [0, 65535] before + calculating the HP change. This clamping has been removed as of + Minetest 5.10.0 * `reason`: a PlayerHPChangeReason table. * The `type` field will have one of the following values: * `set_hp`: A mod or the engine called `set_hp` without diff --git a/games/devtest/mods/unittests/player.lua b/games/devtest/mods/unittests/player.lua index 7650d5f57..f8945f320 100644 --- a/games/devtest/mods/unittests/player.lua +++ b/games/devtest/mods/unittests/player.lua @@ -42,41 +42,97 @@ unittests.register("test_hpchangereason", run_hpchangereason_tests, {player=true -- local expected_diff = nil +local hpchange_counter = 0 +local die_counter = 0 core.register_on_player_hpchange(function(player, hp_change, reason) if expected_diff then assert(hp_change == expected_diff) + hpchange_counter = hpchange_counter + 1 end end) +core.register_on_dieplayer(function() + die_counter = die_counter + 1 +end) + +local function hp_diference_test(player, hp_max) + assert(hp_max >= 22) -local function run_hp_difference_tests(player) local old_hp = player:get_hp() local old_hp_max = player:get_properties().hp_max - expected_diff = nil - player:set_properties({hp_max = 30}) - player:set_hp(22) + hpchange_counter = 0 + die_counter = 0 - -- final HP value is clamped to >= 0 before difference calculation - expected_diff = -22 + expected_diff = nil + player:set_properties({hp_max = hp_max}) + player:set_hp(22) + assert(player:get_hp() == 22) + assert(hpchange_counter == 0) + assert(die_counter == 0) + + -- HP difference is not clamped + expected_diff = -25 player:set_hp(-3) - -- and actual final HP value is clamped to >= 0 too + -- actual final HP value is clamped to >= 0 assert(player:get_hp() == 0) + assert(hpchange_counter == 1) + assert(die_counter == 1) expected_diff = 22 player:set_hp(22) assert(player:get_hp() == 22) + assert(hpchange_counter == 2) + assert(die_counter == 1) - -- final HP value is clamped to <= U16_MAX before difference calculation - expected_diff = 65535 - 22 + -- Integer overflow is prevented + -- so result is S32_MIN, not S32_MIN - 22 + expected_diff = -2147483648 + player:set_hp(-2147483648) + -- actual final HP value is clamped to >= 0 + assert(player:get_hp() == 0) + assert(hpchange_counter == 3) + assert(die_counter == 2) + + -- Damage is ignored if player is already dead (hp == 0) + expected_diff = "never equal" + player:set_hp(-11) + assert(player:get_hp() == 0) + -- no on_player_hpchange or on_dieplayer call expected + assert(hpchange_counter == 3) + assert(die_counter == 2) + + expected_diff = 11 + player:set_hp(11) + assert(player:get_hp() == 11) + assert(hpchange_counter == 4) + assert(die_counter == 2) + + -- HP difference is not clamped + expected_diff = 1000000 - 11 player:set_hp(1000000) - -- and actual final HP value is clamped to <= hp_max - assert(player:get_hp() == 30) + -- actual final HP value is clamped to <= hp_max + assert(player:get_hp() == hp_max) + assert(hpchange_counter == 5) + assert(die_counter == 2) + + -- "Healing" is not ignored when hp == hp_max + expected_diff = 80000 - hp_max + player:set_hp(80000) + assert(player:get_hp() == hp_max) + -- on_player_hpchange_call expected + assert(hpchange_counter == 6) + assert(die_counter == 2) expected_diff = nil player:set_properties({hp_max = old_hp_max}) player:set_hp(old_hp) core.close_formspec(player:get_player_name(), "") -- hide death screen end +local function run_hp_difference_tests(player) + hp_diference_test(player, 22) + hp_diference_test(player, 30) + hp_diference_test(player, 65535) -- U16_MAX +end unittests.register("test_hp_difference", run_hp_difference_tests, {player=true}) -- diff --git a/src/server/player_sao.cpp b/src/server/player_sao.cpp index 30c41bb1e..61d328ca7 100644 --- a/src/server/player_sao.cpp +++ b/src/server/player_sao.cpp @@ -519,12 +519,13 @@ void PlayerSAO::rightClick(ServerActiveObject *clicker) void PlayerSAO::setHP(s32 target_hp, const PlayerHPChangeReason &reason, bool from_client) { - target_hp = rangelim(target_hp, 0, U16_MAX); - - if (target_hp == m_hp) + if (target_hp == m_hp || (m_hp == 0 && target_hp < 0)) return; // Nothing to do - s32 hp_change = m_env->getScriptIface()->on_player_hpchange(this, target_hp - (s32)m_hp, reason); + // Protect against overflow. + s32 hp_change = std::max((s64)target_hp - (s64)m_hp, S32_MIN); + + hp_change = m_env->getScriptIface()->on_player_hpchange(this, hp_change, reason); hp_change = std::min(hp_change, U16_MAX); // Protect against overflow s32 hp = (s32)m_hp + hp_change;