You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Possible Bug The function isArmyAlive is using a new parameter Battle which is not defined in the old context. Ensure that the new implementation aligns with the intended logic changes and that all necessary components are correctly passed and utilized.
Refactoring Needed The ArmyChip component has been significantly refactored with new logic for managing army details and interactions. It is crucial to ensure that all new methods and state management hooks are correctly implemented and that there are no regressions in functionality. The use of BattleManager and state hooks like useMemo for updatedArmy and updatedBattle need thorough testing to ensure they handle all edge cases correctly.
Logic Change The BattleActions component now includes changes in how battles are managed, particularly with the introduction of battleManager for updating battle and army states. This change affects how battles are initiated, updated, and concluded within the UI. Special attention should be given to how these changes impact the overall battle management workflow.
+if (!battle) throw new Error("Battle data is missing.");
const cloneArmy = structuredClone(army);
Suggestion importance[1-10]: 9
Why: Adding error handling to check for undefined battle before accessing its properties is crucial for preventing runtime errors, significantly improving the robustness of the method.
9
Enhancement
Add a check for battle_id before fetching a battle
Consider checking for the existence of battle_id before using it to fetch a battle. This prevents unnecessary calls and potential errors if battle_id is not set.
Why: This suggestion prevents potential errors and unnecessary calls by ensuring battle_id is set before fetching a battle, improving code robustness.
9
Simplify handling of undefined or null values using optional chaining and nullish coalescing
Use optional chaining (?.) and nullish coalescing (??) operators to simplify the handling of potentially undefined or null values in the playerStructures mapping function. This will make the code more concise and easier to understand.
Why: Using optional chaining and nullish coalescing operators simplifies the code and makes it more readable and concise, which is a significant improvement for handling undefined or null values.
9
Best practice
Enhance type safety by replacing direct type conversion with conditional checking
Replace the direct use of Number conversion in the getElapsedTime method with a safer type-checking approach to avoid runtime errors from invalid conversions.
Why: This suggestion improves type safety and prevents potential runtime errors from invalid conversions, making the code more robust and reliable.
8
Improve type safety by ensuring the currentTimestamp parameter type matches expected types in operations
Consider using a more precise type for the currentTimestamp parameter in the getElapsedTime method to ensure it aligns with the expected type of battle.last_updated. This will prevent potential type mismatches and runtime errors.
-public getElapsedTime(currentTimestamp: number): number {+public getElapsedTime(currentTimestamp: bigint): number {
Suggestion importance[1-10]: 7
Why: Changing the type of currentTimestamp to bigint can improve type safety, but it requires ensuring that all related operations and usages are compatible with bigint. This suggestion is beneficial but not critical.
7
Add type checks for id parameters to prevent type errors in getComponentValue calls
To ensure type safety and avoid potential runtime type errors, explicitly check the types of id parameters in getComponentValue calls. This can be done by ensuring id is of the expected type before passing it to the function.
-const realm = getComponentValue(Realm, id);-const structure = getComponentValue(Structure, id);-const position = getComponentValue(Position, id);+const realm = typeof id === 'bigint' ? getComponentValue(Realm, id) : undefined;+const structure = typeof id === 'bigint' ? getComponentValue(Structure, id) : undefined;+const position = typeof id === 'bigint' ? getComponentValue(Position, id) : undefined;
Suggestion importance[1-10]: 6
Why: Adding type checks for id parameters can prevent type errors and improve type safety. However, the suggestion could be more concise and might be over-cautious if the type of id is already guaranteed by the context in which it is used.
6
Performance
Use BigInt for calculations involving potentially large numbers to prevent precision loss
Optimize the calculation of total_knight_health, total_paladin_health, and total_crossbowman_health by directly using BigInt for multiplication to avoid precision loss in large numbers.
Why: Using BigInt for calculations involving large numbers ensures precision and prevents potential loss, optimizing performance and accuracy in the code.
8
Maintainability
Improve type safety by replacing 'as any' with specific types
Replace the direct use of as any with proper type casting or interfaces to ensure type safety and maintainability.
-const isInBattle = testedModule.protectorStillInBattle({} as any, {} as any, {} as any);+const isInBattle = testedModule.protectorStillInBattle({} as BattleInfo, {} as Component, {} as Entity);
Suggestion importance[1-10]: 8
Why: This suggestion improves type safety and maintainability by replacing 'as any' with specific types, which is crucial for catching type-related errors early.
8
Extract health conversion logic into a utility function
Refactor the repeated BigInt conversions and calculations into a separate utility function to improve code readability and reusability.
Why: Refactoring the repeated BigInt conversions into a utility function enhances code readability and reusability, making the code cleaner and easier to maintain.
8
Reduce redundancy by refactoring repeated mocking into a helper function
Refactor the repeated mocking of runQuery to a helper function to reduce redundancy and improve code clarity.
Why: Encapsulating the calculation of offset.y in a function improves maintainability and consistency, though it is a minor improvement.
7
Refactor repeated logic for mapping realms into a separate function to reduce code duplication
To improve code readability and maintainability, consider refactoring the repeated logic for mapping playerRealms and otherRealms into a separate function. This will reduce code duplication and make the codebase cleaner.
Why: Refactoring the repeated logic into a separate function improves maintainability and readability by reducing code duplication. However, the suggestion should ensure that the refactored function is properly integrated and tested.
7
Possible bug
Add null checks to prevent runtime errors when component values are missing
Consider adding null checks for realm and structure in the mapping functions to prevent runtime errors if getComponentValue returns undefined. This will ensure that the application handles cases where the component values are not found, improving the robustness of the code.
Why: Adding null checks for realm and structure is a good practice to prevent potential runtime errors, enhancing the robustness of the code. However, using empty objects as fallbacks might not be the best approach as it could lead to other issues if the code expects certain properties to be present.
8
Readability
Enhance code readability by using more descriptive variable names
Use a more descriptive variable name than protectors for the Set variable to enhance code readability.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
bot
added
enhancement
edisontim
force-pushed
the
ref/combat
branch
2 times, most recently
from
PR Type
Enhancement, Tests, Bug fix
Description
Changes walkthrough 📝
13 files
tailwind.config.js
Add utility class to hide scrollbarsclient/tailwind.config.js
.no-scrollbarto hide scrollbars acrossdifferent browsers.
useArmies.tsx
Refactor and enhance army-related hooks and utilitiesclient/src/hooks/helpers/useArmies.tsx
formatArmiesfunction to improve data handling.useArmiestouseMovableArmies.useBattles.tsx
Refactor and enhance battle-related hooks and utilitiesclient/src/hooks/helpers/battles/useBattles.tsx
getBattlefunction to fetch and process battle data.ArmyManagementCard.tsx
Update ArmyManagementCard component to use new army data structureclient/src/ui/components/military/ArmyManagementCard.tsx
entityprop toarmy.ViewOnMapIconcomponent.BattleManager.ts
Refactor BattleManager class for improved battle data handlingclient/src/dojo/modelManager/BattleManager.ts
BattleManagerclass to improve battle data handling.useStructures.tsx
Refactor and enhance structure-related hooks and utilitiesclient/src/hooks/helpers/useStructures.tsx
useEntities.tsx
Refactor and enhance entity-related hooks and utilitiesclient/src/hooks/helpers/useEntities.tsx
useEntitiesintouseEntitiesanduseEntitiesUtils.EntityDetails.tsx
Add EntityDetails component to display entity informationclient/src/ui/modules/entity-details/EntityDetails.tsx
EntityDetails.or clicked hex.
ArmyChip.tsx
Update ArmyChip component to use new army data structureclient/src/ui/components/military/ArmyChip.tsx
ArmyChipcomponent to use new army data structure.Entity.tsx
Update Entity component to use new entity and battle data structureclient/src/ui/components/entities/Entity.tsx
Entitycomponent to use new entity and battle data structure.BattleActions.tsx
Update BattleActions component to use new battle data structureclient/src/ui/modules/military/battle-view/BattleActions.tsx
BattleActionscomponent to use new battle data structure.ArmyInfoLabel.tsx
Update ArmyInfoLabel component to use new army data structureclient/src/ui/components/worldmap/armies/ArmyInfoLabel.tsx
ArmyInfoLabelcomponent to use new army data structure.Army.tsx
Update Army and ArmySelectionOverlay components to use new army datastructureclient/src/ui/components/worldmap/armies/Army.tsx
ArmyandArmySelectionOverlaycomponents to use new army datastructure.
2 files
useBattlesUtils.test.tsx
Add unit tests for useBattlesUtils functionsclient/src/hooks/helpers/battles/test/useBattlesUtils.test.tsx
useBattlesUtilsfunctions.useBattles.test.tsx
Add unit tests for useBattles functionsclient/src/hooks/helpers/battles/test/useBattles.test.tsx
useBattlesfunctions.66 files
ProductionManager.ts
...client/src/dojo/modelManager/ProductionManager.ts
...
BattleView.tsx
...client/src/ui/modules/military/battle-view/BattleView.tsx
...
Battle.tsx
...client/src/ui/modules/military/battle-view/Battle.tsx
...
useHyperstructures.tsx
...client/src/hooks/helpers/useHyperstructures.tsx
...
TroopChip.tsx
...client/src/ui/components/military/TroopChip.tsx
...
useGuilds.tsx
...client/src/hooks/helpers/useGuilds.tsx
...
TopMiddleNavigation.tsx
...client/src/ui/modules/navigation/TopMiddleNavigation.tsx
...
useTrade.tsx
...client/src/hooks/helpers/useTrade.tsx
...
Battles.tsx
...client/src/ui/components/models/buildings/worldmap/Battles.tsx
...
StructureCard.tsx
...client/src/ui/components/hyperstructures/StructureCard.tsx
...
GroundGrid.tsx
...client/src/ui/components/construction/GroundGrid.tsx
...
ShardsMines.tsx
...client/src/ui/components/models/buildings/worldmap/ShardsMines.tsx
...
useHexPosition.tsx
...client/src/hooks/helpers/useHexPosition.tsx
...
useTravel.tsx
...client/src/hooks/helpers/useTravel.tsx
...
ArmyViewCard.tsx
...client/src/ui/components/military/ArmyViewCard.tsx
...
MarketManager.ts
...client/src/dojo/modelManager/MarketManager.ts
...
RealmListItem.tsx
...client/src/ui/components/worldmap/realms/RealmListItem.tsx
...
Battle.tsx
...client/src/ui/components/military/Battle.tsx
...
InstancedCastles.tsx
...client/src/ui/components/models/buildings/worldmap/InstancedCastles.tsx
...
TradeHistoryEvent.tsx
...client/src/ui/components/trading/TradeHistoryEvent.tsx
...
Armies.tsx
...client/src/ui/components/worldmap/armies/Armies.tsx
...
LeftNavigationModule.tsx
...client/src/ui/modules/navigation/LeftNavigationModule.tsx
...
useResources.tsx
...client/src/hooks/helpers/useResources.tsx
...
HexceptionViewScene.tsx
...client/src/ui/modules/scenes/HexceptionViewScene.tsx
...
useBuildings.tsx
...client/src/hooks/helpers/useBuildings.tsx
...
ArmyHitBox.tsx
...client/src/ui/components/worldmap/armies/ArmyHitBox.tsx
...
utils.tsx
...client/src/ui/components/worldmap/armies/utils.tsx
...
useRoads.tsx
...client/src/hooks/helpers/useRoads.tsx
...
useBanks.tsx
...client/src/hooks/helpers/useBanks.tsx
...
RightNavigationModule.tsx
...client/src/ui/modules/navigation/RightNavigationModule.tsx
...
WorldStructuresMenu.tsx
...client/src/ui/modules/world-structures/WorldStructuresMenu.tsx
...
BattleDetails.tsx
...client/src/ui/modules/military/battle-view/BattleDetails.tsx
...
StructureListItem.tsx
...client/src/ui/components/worldmap/structures/StructureListItem.tsx
...
useBlockchainStore.tsx
...client/src/hooks/store/useBlockchainStore.tsx
...
HexGrid.tsx
...client/src/ui/components/models/biomes/HexGrid.tsx
...
GrasslandBiome.tsx
...client/src/ui/components/models/biomes/GrasslandBiome.tsx
...
DeepOceanBiome.tsx
...client/src/ui/components/models/biomes/DeepOceanBiome.tsx
...
DesertBiome.tsx
...client/src/ui/components/models/biomes/DesertBiome.tsx
...
OceanBiome.tsx
...client/src/ui/components/models/biomes/OceanBiome.tsx
...
useRealm.tsx
...client/src/hooks/helpers/useRealm.tsx
...
useQuestStore.tsx
...client/src/hooks/store/useQuestStore.tsx
...
vitest.config.ts
...client/vitest.config.ts
...
DeciduousForestBiome.tsx
...client/src/ui/components/models/biomes/DeciduousForestBiome.tsx
...
SubtropicalDesertBiome.tsx
...client/src/ui/components/models/biomes/SubtropicalDesertBiome.tsx
...
TemperateDesertBiome.tsx
...client/src/ui/components/models/biomes/TemperateDesertBiome.tsx
...
TemperateRainforestBiome.tsx
...client/src/ui/components/models/biomes/TemperateRainforestBiome.tsx
...
TropicalSeasonalForestBiome.tsx
...client/src/ui/components/models/biomes/TropicalSeasonalForestBiome.tsx
...
_mapStore.tsx
...client/src/hooks/store/_mapStore.tsx
...
ScorchedBiome.tsx
...client/src/ui/components/models/biomes/ScorchedBiome.tsx
...
SnowBiome.tsx
...client/src/ui/components/models/biomes/SnowBiome.tsx
...
TropicalRainforestBiome.tsx
...client/src/ui/components/models/biomes/TropicalRainforestBiome.tsx
...
BeachBiome.tsx
...client/src/ui/components/models/biomes/BeachBiome.tsx
...
ShrublandBiome.tsx
...client/src/ui/components/models/biomes/ShrublandBiome.tsx
...
TaigaBiome.tsx
...client/src/ui/components/models/biomes/TaigaBiome.tsx
...
TundraBiome.tsx
...client/src/ui/components/models/biomes/TundraBiome.tsx
...
BattleLabel.tsx
...client/src/ui/components/worldmap/armies/BattleLabel.tsx
...
useCaravans.tsx
...client/src/hooks/helpers/useCaravans.tsx
...
global.ts
...sdk/packages/eternum/src/constants/global.ts
...
ArmyPanel.tsx
...client/src/ui/components/military/ArmyPanel.tsx
...
BattlesArmyTable.tsx
...client/src/ui/components/military/BattlesArmyTable.tsx
...
DepositResources.tsx
...client/src/ui/components/resources/DepositResources.tsx
...
pnpm-lock.yaml
...pnpm-lock.yaml
...
tests.cairo
...contracts/src/systems/combat/tests.cairo
...
package.json
...client/package.json
...
test-contracts.yml
....github/workflows/test-contracts.yml
...
test-client.yml
....github/workflows/test-client.yml
...