From 71254baf55a8d820eb7d032329421b72056f203c Mon Sep 17 00:00:00 2001 From: 0xNotMe Date: Fri, 4 Jul 2025 06:51:30 +0200 Subject: [PATCH] fix: load equipment and inventory data in ability handler (fixes #20) - Add equipment loading (loadEquipment()) in _handleAbility function - Add inventory loading from storage in _handleAbility function - Load defender equipment when processing offensive abilities - Fix variable shadowing warning in Storage.sol _getKiller function - Add comprehensive tests for equipment/inventory loading during abilities This ensures ability usage returns complete BattleNad structs with populated weapon, armor, and inventory data instead of empty values. --- src/battle-nads/Handler.sol | 7 + src/battle-nads/Storage.sol | 5 +- test/battle-nads/BattleNadsAbilityTest.t.sol | 128 +++++++++++++++++++ 3 files changed, 138 insertions(+), 2 deletions(-) diff --git a/src/battle-nads/Handler.sol b/src/battle-nads/Handler.sol index 026bd73..a35b1db 100644 --- a/src/battle-nads/Handler.sol +++ b/src/battle-nads/Handler.sol @@ -568,6 +568,10 @@ abstract contract Handler is Balances { return (attacker, false, 0); } + // Load equipment and inventory + attacker = attacker.loadEquipment(); + attacker.inventory = inventories[attacker.id]; + // Attempt to load a defender, exit if no defenders remain BattleNad memory defender; bool loadedDefender = attacker.activeAbility.targetIndex != 0 @@ -587,6 +591,9 @@ abstract contract Handler is Balances { // Return early if target cant be found - process their death in regular combat task. return (attacker, false, 0); } + + // Load defender equipment too + defender = defender.loadEquipment(); } // Make sure the characters are in combat if appropriate diff --git a/src/battle-nads/Storage.sol b/src/battle-nads/Storage.sol index 7407ddc..8547316 100644 --- a/src/battle-nads/Storage.sol +++ b/src/battle-nads/Storage.sol @@ -210,11 +210,12 @@ abstract contract Storage { } function _getKiller(bytes32 deceasedID) internal view returns (bytes32 killerID, bool valid) { - bytes32 killerID = killMap[deceasedID]; - if (killerID == _UNKILLED || killerID == _KILL_PROCESSED || !_isValidID(killerID)) { + bytes32 storedKillerID = killMap[deceasedID]; + if (storedKillerID == _UNKILLED || storedKillerID == _KILL_PROCESSED || !_isValidID(storedKillerID)) { killerID = _NULL_ID; return (killerID, false); } + killerID = storedKillerID; return (killerID, true); } diff --git a/test/battle-nads/BattleNadsAbilityTest.t.sol b/test/battle-nads/BattleNadsAbilityTest.t.sol index 5ccc31a..4401aeb 100644 --- a/test/battle-nads/BattleNadsAbilityTest.t.sol +++ b/test/battle-nads/BattleNadsAbilityTest.t.sol @@ -483,4 +483,132 @@ contract BattleNadsAbilityTest is BattleNadsBaseTest { } } } + + // ============================================================================= + // EQUIPMENT AND INVENTORY LOADING TESTS (Issue #20) + // ============================================================================= + + /** + * @dev Tests that equipment and inventory data are properly loaded when using abilities + * This test verifies the fix for issue #20 where ability usage returned empty equipment data + */ + function test_Ability_EquipmentAndInventoryLoading() public { + bytes32 character = character1; + + // First equip some items to the character by setting weapon and armor IDs + _modifyCharacterStat(character, "weaponID", 4); // Set weapon ID + _modifyCharacterStat(character, "armorID", 4); // Set armor ID + + // Get character data before ability to verify equipment is set + BattleNad memory beforeAbility = battleNads.getBattleNad(character); + console.log("=== Before Ability Usage ==="); + console.log("Weapon ID:", beforeAbility.stats.weaponID); + console.log("Armor ID:", beforeAbility.stats.armorID); + console.log("Weapon Name:", beforeAbility.weapon.name); + console.log("Armor Name:", beforeAbility.armor.name); + + // Verify character has equipment IDs + assertTrue(beforeAbility.stats.weaponID > 0, "Character should have weapon equipped"); + assertTrue(beforeAbility.stats.armorID > 0, "Character should have armor equipped"); + + // Enter combat to use abilities + bool combatStarted = _triggerRandomCombat(character); + assertTrue(combatStarted, "Should enter combat"); + + // Use a non-offensive ability (doesn't require target) + vm.prank(userSessionKey1); + battleNads.useAbility(character, 0, 1); + + // Get character data after ability usage + BattleNad memory afterAbility = battleNads.getBattleNad(character); + console.log("=== After Ability Usage ==="); + console.log("Weapon ID:", afterAbility.stats.weaponID); + console.log("Armor ID:", afterAbility.stats.armorID); + console.log("Weapon Name:", afterAbility.weapon.name); + console.log("Weapon Base Damage:", afterAbility.weapon.baseDamage); + console.log("Armor Name:", afterAbility.armor.name); + console.log("Armor Factor:", afterAbility.armor.armorFactor); + console.log("Inventory Weapon Bitmap:", afterAbility.inventory.weaponBitmap); + console.log("Inventory Armor Bitmap:", afterAbility.inventory.armorBitmap); + + // Verify equipment data is still loaded (not empty) after ability usage + if (afterAbility.stats.weaponID > 0) { + assertTrue( + bytes(afterAbility.weapon.name).length > 0 || afterAbility.weapon.baseDamage > 0, + "Weapon data should be loaded when weapon is equipped" + ); + } + + if (afterAbility.stats.armorID > 0) { + assertTrue( + bytes(afterAbility.armor.name).length > 0 || afterAbility.armor.armorFactor > 0, + "Armor data should be loaded when armor is equipped" + ); + } + + // Verify inventory data is loaded + assertTrue( + afterAbility.inventory.weaponBitmap > 0 || afterAbility.inventory.armorBitmap > 0 || afterAbility.inventory.balance > 0, + "Inventory data should be loaded" + ); + + console.log("Equipment and inventory data properly loaded during ability usage!"); + } + + /** + * @dev Tests equipment loading for offensive abilities with targets + */ + function test_Ability_OffensiveWithEquipmentLoading() public { + bytes32 attacker = character1; + bytes32 defender = character2; + + // Equip items to both characters by setting weapon and armor IDs + _modifyCharacterStat(attacker, "weaponID", 3); // Set weapon ID + _modifyCharacterStat(attacker, "armorID", 3); // Set armor ID + _modifyCharacterStat(defender, "weaponID", 5); // Set weapon ID + _modifyCharacterStat(defender, "armorID", 5); // Set armor ID + + // Enter combat + bool combatStarted = _triggerRandomCombat(attacker); + assertTrue(combatStarted, "Should enter combat"); + + // Find target for offensive ability + uint256 targetIndex = _findCombatTarget(attacker); + assertTrue(targetIndex > 0, "Should find valid target"); + + // Determine offensive ability index based on class + BattleNad memory attackerData = battleNads.getBattleNad(attacker); + uint256 offensiveAbilityIndex = attackerData.stats.class == CharacterClass.Warrior ? 1 : 2; + + // Use offensive ability + vm.prank(userSessionKey1); + battleNads.useAbility(attacker, targetIndex, offensiveAbilityIndex); + + // Get updated attacker data + BattleNad memory afterAbility = battleNads.getBattleNad(attacker); + + // Verify both attacker's equipment is loaded + console.log("=== Attacker Equipment After Offensive Ability ==="); + console.log("Weapon ID:", afterAbility.stats.weaponID); + console.log("Weapon Name:", afterAbility.weapon.name); + console.log("Armor ID:", afterAbility.stats.armorID); + console.log("Armor Name:", afterAbility.armor.name); + + if (afterAbility.stats.weaponID > 0) { + assertTrue( + bytes(afterAbility.weapon.name).length > 0 || afterAbility.weapon.baseDamage > 0, + "Attacker weapon data should be loaded" + ); + } + + if (afterAbility.stats.armorID > 0) { + assertTrue( + bytes(afterAbility.armor.name).length > 0 || afterAbility.armor.armorFactor > 0, + "Attacker armor data should be loaded" + ); + } + + // The defender's equipment should also be loaded in the handler + console.log("Offensive ability with equipment loading test passed!"); + } } \ No newline at end of file