Fix and test OpenEnroth#1294 - use signed ints

pskelton · Nov 22, 2023 · 5270bed · 5270bed
1 parent d57729a
commit 5270bed
Show file tree

Hide file tree

Showing 4 changed files with 29 additions and 15 deletions.
diff --git a/src/Engine/Objects/Character.cpp b/src/Engine/Objects/Character.cpp
@@ -2003,7 +2003,7 @@ DamageType Character::GetSpellDamageType(SpellId uSpellID) const {
 //----- (0048E1B5) --------------------------------------------------------
 int Character::GetAttackRecoveryTime(bool attackUsesBow) const {
     const ItemGen *weapon = nullptr;
-    unsigned weapon_recovery = base_recovery_times_per_weapon_type[CHARACTER_SKILL_STAFF];
+    int weapon_recovery = base_recovery_times_per_weapon_type[CHARACTER_SKILL_STAFF];
     if (attackUsesBow) {
         assert(HasItemEquipped(ITEM_SLOT_BOW));
         weapon = GetBowItem();
@@ -2019,13 +2019,13 @@ int Character::GetAttackRecoveryTime(bool attackUsesBow) const {
         }
     }
 
-    unsigned shield_recovery = 0;
+    int shield_recovery = 0;
     if (HasItemEquipped(ITEM_SLOT_OFF_HAND)) {
         if (GetEquippedItemEquipType(ITEM_SLOT_OFF_HAND) == ITEM_TYPE_SHIELD) {
             CharacterSkillType skill_type = GetOffHandItem()->GetPlayerSkillType();
-            unsigned shield_base_recovery = base_recovery_times_per_weapon_type[skill_type];
+            int shield_base_recovery = base_recovery_times_per_weapon_type[skill_type];
             float multiplier = GetArmorRecoveryMultiplierFromSkillLevel(skill_type, 1.0f, 0, 0, 0);
-            shield_recovery = (unsigned)(shield_base_recovery * multiplier);
+            shield_recovery = (shield_base_recovery * multiplier);
         } else {
             if (base_recovery_times_per_weapon_type[GetOffHandItem()->GetPlayerSkillType()] > weapon_recovery) {
                 weapon = GetOffHandItem();
@@ -2034,10 +2034,10 @@ int Character::GetAttackRecoveryTime(bool attackUsesBow) const {
         }
     }
 
-    unsigned armour_recovery = 0;
+    int armour_recovery = 0;
     if (HasItemEquipped(ITEM_SLOT_ARMOUR)) {
         CharacterSkillType armour_skill_type = GetArmorItem()->GetPlayerSkillType();
-        unsigned base_armour_recovery = base_recovery_times_per_weapon_type[armour_skill_type];
+        int base_armour_recovery = base_recovery_times_per_weapon_type[armour_skill_type];
         float multiplier;
 
         if (armour_skill_type == CHARACTER_SKILL_LEATHER) {
@@ -2051,12 +2051,12 @@ int Character::GetAttackRecoveryTime(bool attackUsesBow) const {
             multiplier = GetArmorRecoveryMultiplierFromSkillLevel(armour_skill_type, 1.0f, 1.0f, 1.0f, 1.0f);
         }
 
-        armour_recovery = (unsigned)(base_armour_recovery * multiplier);
+        armour_recovery = base_armour_recovery * multiplier;
     }
 
-    unsigned player_speed_recovery_reduction = GetParameterBonus(GetActualSpeed());
+    int player_speed_recovery_reduction = GetParameterBonus(GetActualSpeed());
 
-    unsigned sword_axe_bow_recovery_reduction = 0;
+    int sword_axe_bow_recovery_reduction = 0;
     if (weapon != nullptr) {
         CombinedSkillValue weaponSkill = getActualSkillValue(weapon->GetPlayerSkillType());
         if (weaponSkill.level() > 0 &&
@@ -2072,7 +2072,7 @@ int Character::GetAttackRecoveryTime(bool attackUsesBow) const {
     bool shooting_laser = weapon && weapon->GetPlayerSkillType() == CHARACTER_SKILL_BLASTER;
     assert(!shooting_laser || !attackUsesBow); // For blasters we expect attackUsesBow == false.
 
-    unsigned armsmaster_recovery_reduction = 0;
+    int armsmaster_recovery_reduction = 0;
     if (!attackUsesBow && !shooting_laser) {
         CombinedSkillValue armsmasterSkill = getActualSkillValue(CHARACTER_SKILL_ARMSMASTER);
         if (armsmasterSkill.level() > 0) {
@@ -2082,11 +2082,11 @@ int Character::GetAttackRecoveryTime(bool attackUsesBow) const {
         }
     }
 
-    unsigned hasteRecoveryReduction = 0;
+    int hasteRecoveryReduction = 0;
     if (pCharacterBuffs[CHARACTER_BUFF_HASTE].Active()) hasteRecoveryReduction = 25;
     if (pParty->pPartyBuffs[PARTY_BUFF_HASTE].Active()) hasteRecoveryReduction = 25;
 
-    unsigned weapon_enchantment_recovery_reduction = 0;
+    int weapon_enchantment_recovery_reduction = 0;
     if (weapon != nullptr) {
         if (weapon->special_enchantment == ITEM_ENCHANTMENT_SWIFT ||
             weapon->special_enchantment == ITEM_ENCHANTMENT_OF_DARKNESS ||
@@ -2100,7 +2100,7 @@ int Character::GetAttackRecoveryTime(bool attackUsesBow) const {
                    hasteRecoveryReduction - sword_axe_bow_recovery_reduction -
                    player_speed_recovery_reduction;
 
-    unsigned minRecovery;
+    int minRecovery;
     if (shooting_laser) {
         minRecovery = engine->config->gameplay.MinRecoveryBlasters.value();
     } else if (attackUsesBow) {

diff --git a/src/Engine/Spells/CastSpellInfo.cpp b/src/Engine/Spells/CastSpellInfo.cpp
@@ -288,7 +288,7 @@ void CastSpellInfoHelpers::castSpell() {
                 pParty->bTurnBasedModeOn) {
                 ++pTurnEngine->pending_actions;
             }
-            
+
             setSpellRecovery(pCastSpell, pPlayer->GetAttackRecoveryTime(false));
         } else if (pCastSpell->uSpellID == SPELL_WATER_TOWN_PORTAL) {
             int success_chance_percent = 10 * spell_level;

diff --git a/test/Bin/GameTest/CMakeLists.txt b/test/Bin/GameTest/CMakeLists.txt
@@ -18,7 +18,7 @@ if(OE_BUILD_TESTS)
     ExternalProject_Add(OpenEnroth_TestData
             PREFIX ${CMAKE_CURRENT_BINARY_DIR}/test_data_tmp
             GIT_REPOSITORY https://github.com/OpenEnroth/OpenEnroth_TestData.git
-            GIT_TAG 65a1b9e8208338844993d35d00ed86f2d91ec051
+            GIT_TAG ee67386f0a475fc81fcd61bf6873abc706a5aeea
             SOURCE_DIR ${CMAKE_CURRENT_BINARY_DIR}/test_data
             CONFIGURE_COMMAND ""
             BUILD_COMMAND ""

diff --git a/test/Bin/GameTest/GameTests.cpp b/test/Bin/GameTest/GameTests.cpp
@@ -1733,6 +1733,20 @@ GAME_TEST(Issues, Issue1282) {
     EXPECT_EQ(totalObjectsTape.delta(), -1);
 }
 
+GAME_TEST(Issues, Issue1294_1389) {
+    // Bow and Blaster recovery times
+    // Character::GetAttackRecoveryTime assert when character is using blaster
+    auto windowTape = tapes.custom([] { return current_character_screen_window; });
+    test.playTraceFromTestData("issue_1294.mm7", "issue_1294.json");
+
+    // Check that we get back to stats screen without asserting
+    EXPECT_TRUE(windowTape.contains(WindowType::WINDOW_CharacterWindow_Inventory));
+    EXPECT_EQ(windowTape.back(), WindowType::WINDOW_CharacterWindow_Stats);
+    // Check min values are used
+    EXPECT_EQ(pParty->pCharacters[0].GetAttackRecoveryTime(false), engine->config->gameplay.MinRecoveryBlasters.value());
+    EXPECT_EQ(pParty->pCharacters[2].GetAttackRecoveryTime(true), engine->config->gameplay.MinRecoveryRanged.value());
+}
+
 // 1300
 
 GAME_TEST(Issues, Issue1315) {