fix: army deposits battle bugs + hyperstructure points calculations

edisontim commented 5 days ago

User description

fixes armies appearing in the arrivals list even though they're dead
add possibility to leave a battle with a defending army that lost, this unlocks a realm from a state where it's in a lost battle so it can't transfer and also can't leave because the battle is lost so it doesn't show up on the map
fixes points calculations and points layout in hyperstructure panel

PR Type

Bug fix, Enhancement

Description

Fixed armies appearing in the arrivals list even though they're dead.
Added possibility to leave a battle with a defending army that lost.
Fixed points calculations and points layout in hyperstructure panel.
Improved army filtering and renamed hook to useArmiesByEntityOwner.
Enhanced battle information retrieval and filtering.
Fixed resource cost calculation in hyperstructures.
Updated various components to use new army and battle hooks.
Simplified hyperstructure panel and leaderboard.
Added function to mint all resources in script.

Changes walkthrough 📝

Relevant files

Formatting

1 files

BattleManager.ts

Minor formatting improvement.

client/src/dojo/modelManager/BattleManager.ts - Added a newline for better readability.

+1/-0

Enhancement

18 files

useArmies.tsx `Improved army filtering and renamed hook.` client/src/hooks/helpers/useArmies.tsx Added `getExtraBattleInformation` import. Replaced `checkIfArmyLostAFinishedBattle` with `filterArmy` in multiple functions. Renamed `useEntityArmies` to `useArmiesByEntityOwner`. Updated filtering logic for armies.	+22/-18
useBattles.tsx `Enhanced battle information retrieval and filtering.` client/src/hooks/helpers/useBattles.tsx Added `getExtraBattleInformation` and `BattleInfo` type. Refactored battle-related functions to use `getExtraBattleInformation`. Added `filterBattles` function. Added `battleIsEmpty` function.	+114/-89
useResources.tsx `Filter armies in resource arrivals.` client/src/hooks/helpers/useResources.tsx Added `filterArmy` and `getArmyByEntityId` imports. Updated `getAllArrivalsWithResources` to filter armies.	+24/-2
useLeaderBoardStore.tsx `Refactor leaderboard points calculation.` client/src/hooks/store/useLeaderBoardStore.tsx Refactored `TOTAL_CONTRIBUTABLE_AMOUNT` calculation. Removed redundant `playerPointsLeaderboards` parameter.	+9/-15
useQuestStore.tsx `Update quest store to use new army hook.` client/src/hooks/store/useQuestStore.tsx - Replaced `useEntityArmies` with `useArmiesByEntityOwner`.	+5/-5
Entity.tsx `Add army existence check in entity component.` client/src/ui/components/entities/Entity.tsx Added `getArmyByEntityId` import. Added army existence check before rendering.	+12/-9
HyperstructurePanel.tsx `Simplify hyperstructure panel and leaderboard.` client/src/ui/components/hyperstructures/HyperstructurePanel.tsx Updated resource elements to use `HYPERSTRUCTURE_TOTAL_COSTS_SCALED`. Simplified `HyperstructureLeaderboard` component.	+17/-35
HyperstructureResourceChip.tsx `Adjust max contributable amount and improve layout.` client/src/ui/components/hyperstructures/HyperstructureResourceChip.tsx Adjusted max contributable amount calculation. Improved layout of resource chip component.	+11/-4
ArmyList.tsx `Update army list to use new army hook.` client/src/ui/components/military/ArmyList.tsx - Replaced `useEntityArmies` with `useArmiesByEntityOwner`.	+3/-3
BattlesArmyTable.tsx `Update battles army table to use new battle hook.` client/src/ui/components/military/BattlesArmyTable.tsx Replaced `useBattles` with `usePlayerBattles`. Updated `BattleChip` to use `BattleInfo`.	+15/-16
EntitiesArmyTable.tsx `Simplify entities army table with new hook.` client/src/ui/components/military/EntitiesArmyTable.tsx Replaced `useEntityArmies` with `useArmiesByEntityOwner`. Simplified army elements rendering.	+9/-12
Battles.tsx `Remove redundant battle health check.` client/src/ui/components/models/buildings/worldmap/Battles.tsx - Removed redundant battle health check.	+1/-3
ResourceArrivals.tsx `Remove unused resource arrivals component.` client/src/ui/components/trading/ResourceArrivals.tsx Removed unused `useResources` import and `ResourceArrivals` component.	+0/-15
Army.tsx `Update BattleLabel to use battle entity ID.` client/src/ui/components/worldmap/armies/Army.tsx - Updated `BattleLabel` to use battle entity ID.	+1/-1
HexagonInformationPanel.tsx `Update hexagon information panel to use BattleInfo.` client/src/ui/components/worldmap/hexagon/HexagonInformationPanel.tsx - Updated `checkWhatToShow` to use `BattleInfo`.	+2/-3
LeftNavigationModule.tsx `Update left navigation module to use new army hook.` client/src/ui/modules/navigation/LeftNavigationModule.tsx - Replaced `useEntityArmies` with `useArmiesByEntityOwner`.	+8/-9
RightNavigationModule.tsx `Remove unused togglePopup import.` client/src/ui/modules/navigation/RightNavigationModule.tsx - Removed unused `togglePopup` import.	+0/-2
mint_resource.sh `Add function to mint all resources in script.` contracts/scripts/testing/mint_resource.sh Added function to mint all resources. Updated script to handle minting all resources.	+29/-7

Bug fix

3 files

useHyperstructures.tsx `Fix resource cost calculation in hyperstructures.` client/src/hooks/helpers/useHyperstructures.tsx Fixed resource cost calculation in `getAllProgressesAndTotalPercentage`.	+1/-4
BattleActions.tsx `Fix potential undefined value and adjust select width.` client/src/ui/modules/military/battle-view/BattleActions.tsx Fixed potential undefined value in `setBattleView`. Adjusted `SelectContent` width.	+2/-2
BattleView.tsx `Add non-null assertions for battle starter armies.` client/src/ui/modules/military/battle-view/BattleView.tsx - Added non-null assertions for battle starter armies.	+3/-3

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

vercel[bot] commented 5 days ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
eternum	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 5, 2024 2:48pm

github-actions[bot] commented 5 days ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review

**Possible Bug:** The `filterArmy` function in `useArmies.tsx` uses a logical OR to combine checks for army life and battle loss. This could unintentionally include armies that are dead but have a `protectee_id`. The logic might need revising to ensure it accurately filters armies based on the intended conditions. **Refactoring Needed:** The `useBattles.tsx` and `useArmies.tsx` files have significant changes with new functions and logic that could be split into smaller, more manageable functions. This would improve readability and maintainability. **Consistency Issue:** The renaming of hooks and functions (e.g., `useEntityArmies` to `useArmiesByEntityOwner`) should be consistently reflected across all files to avoid confusion and potential bugs.

github-actions[bot] commented 5 days ago

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add a null check for `battle` to prevent potential runtime errors ___ Consider adding a null check for `battle` in the `checkIfArmyLostAFinishedBattle` function to prevent potential runtime errors when `battle` is undefined. [client/src/hooks/helpers/useArmies.tsx [573-575]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-6d68c14fbe3262def9a16f121115d2006c7c34bf942b3e5d8ed7c076d2b89027R573-R575) ```diff const battle = getExtraBattleInformation([getEntityIdFromKeys([BigInt(army.battle_id)])], Battle, Position, Realm)[0]; -if (battle && armyIsLosingSide(army, battle!) && battleIsFinished(Army, battle)) { +if (!battle) return false; +if (armyIsLosingSide(army, battle) && battleIsFinished(Army, battle)) { return true; } ``` Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies a potential runtime error if `battle` is undefined and provides a robust solution by adding a null check. This is crucial for preventing unexpected crashes.	9
	Add null checks for variables before using them to prevent runtime errors ___ Consider checking for the existence of `ownArmyBattleStarter` and `defenderArmyBattleStarter` before using them to prevent runtime errors if they are undefined. [client/src/ui/modules/military/battle-view/BattleView.tsx [46-51]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-9926efdce78d03bb2703f0b52f301b5125b0e15fc3676eeae4645d8f90e6146fR46-R51) ```diff -: [ownArmyBattleStarter!]; -: [defenderArmyBattleStarter!]; +: ownArmyBattleStarter ? [ownArmyBattleStarter] : []; +: defenderArmyBattleStarter ? [defenderArmyBattleStarter] : []; ``` Suggestion importance[1-10]: 9 Why: Adding null checks for `ownArmyBattleStarter` and `defenderArmyBattleStarter` is crucial to prevent potential runtime errors if these variables are undefined. This is a significant improvement for code robustness.	9
	Ensure `filterArmy` handles null or undefined `army` values safely ___ Refactor the `filterArmy` function to ensure it correctly handles cases where `army` might be undefined or null, to avoid runtime errors. [client/src/hooks/helpers/useArmies.tsx [585-589]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-6d68c14fbe3262def9a16f121115d2006c7c34bf942b3e5d8ed7c076d2b89027R585-R589) ```diff +if (!army) return false; return ( (checkIfArmyAlive(army) && checkIfArmyLostAFinishedBattle(Battle, Army, army, Position, Realm) === false) \|\| - BigInt(army?.protectee_id \|\| 0) !== 0n + BigInt(army.protectee_id \|\| 0) !== 0n ); ``` Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential runtime error by adding a check for undefined or null `army` values, which is important for maintaining code stability.	8
	Add checks to handle potential undefined returns from `getArmy` ___ Ensure that the `getArmy` function is always returning a value or handle potential undefined returns where it's used to avoid runtime errors. [client/src/hooks/helpers/useResources.tsx [123-124]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-da868a5a06b096d28f8b0e970a0260a8d26d34f5acfafa3dcbd0156da8f15749R123-R124) ```diff const { getArmy } = getArmyByEntityId(); const army = getArmy(position?.entity_id \|\| BigInt(0)); -if (!army \|\| !filterArmy(army, Battle, Army, Position, Realm)) return undefined; +if (!army) return undefined; +if (!filterArmy(army, Battle, Army, Position, Realm)) return undefined; ``` Suggestion importance[1-10]: 8 Why: The suggestion improves the robustness of the code by ensuring that `getArmy` returns a defined value before proceeding, thus preventing possible runtime errors.	8
	Add a fallback value for `attackerTroops` to handle undefined cases safely ___ Ensure that `attackerTroops` is defined before using it to avoid potential runtime errors. [client/src/ui/modules/military/battle-view/BattleView.tsx [99]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-9926efdce78d03bb2703f0b52f301b5125b0e15fc3676eeae4645d8f90e6146fR99-R99) ```diff -attackerTroops={attackerTroops!} +attackerTroops={attackerTroops ?? []} ``` Suggestion importance[1-10]: 8 Why: Ensuring `attackerTroops` is defined before usage is important to avoid runtime errors. Providing a fallback value improves code safety and reliability.	8
Enhancement	Use `reduce` method for a more functional and concise calculation of `TOTAL_CONTRIBUTABLE_AMOUNT` ___ Consider using the `reduce` method directly on `HYPERSTRUCTURE_TOTAL_COSTS_SCALED` to calculate `TOTAL_CONTRIBUTABLE_AMOUNT` instead of using a separate `forEach` loop. This approach is more functional and concise. [client/src/hooks/store/useLeaderBoardStore.tsx [44-49]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-540f755145619d641c732e0f62d7d62491556a2546e505509d2fe24bcd9dfa04R44-R49) ```diff -export const TOTAL_CONTRIBUTABLE_AMOUNT: number = 0; -HYPERSTRUCTURE_TOTAL_COSTS_SCALED.forEach(({ resource, amount }) => { - TOTAL_CONTRIBUTABLE_AMOUNT += ResourceMultipliers[resource as keyof typeof ResourceMultipliers]! * amount; -}); +export const TOTAL_CONTRIBUTABLE_AMOUNT: number = HYPERSTRUCTURE_TOTAL_COSTS_SCALED.reduce( + (total, { resource, amount }) => { + return total + (ResourceMultipliers[resource as keyof typeof ResourceMultipliers] ?? 0) * amount; + }, + 0, +); ``` Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies a more functional and concise way to calculate `TOTAL_CONTRIBUTABLE_AMOUNT` using `reduce`, which improves readability and maintainability.	9
Error handling	Add error handling for `getContributions` to manage potential failures gracefully ___ Ensure that the `getContributions` function handles potential errors or exceptions, especially if it involves asynchronous operations or fetching data from external sources. [client/src/hooks/store/useLeaderBoardStore.tsx [138]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-540f755145619d641c732e0f62d7d62491556a2546e505509d2fe24bcd9dfa04R138-R138) ```diff -const contributions = getContributions(hyperstructureEntityId); +let contributions; +try { + contributions = getContributions(hyperstructureEntityId); +} catch (error) { + console.error("Failed to get contributions:", error); + return []; // Return an empty array or handle as needed +} ``` Suggestion importance[1-10]: 8 Why: Adding error handling for `getContributions` is a good practice to ensure robustness, especially if the function involves asynchronous operations or external data fetching.	8
Possible issue	Add a null check for `realmEntityId` to prevent passing undefined to `useArmiesByEntityOwner` ___ Replace the direct usage of `realmEntityId` with a null check to ensure it is defined before using it in `useArmiesByEntityOwner`. [client/src/ui/modules/navigation/LeftNavigationModule.tsx [71]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-be9e8648ba688d13b3b68afca51d87c31cca8d9412f8c7d77edee0d71737a287R71-R71) ```diff -const { entityArmies } = useArmiesByEntityOwner({ entity_owner_entity_id: realmEntityId }); +const { entityArmies } = useArmiesByEntityOwner({ entity_owner_entity_id: realmEntityId ?? '' }); ``` Suggestion importance[1-10]: 8 Why: Adding a null check for `realmEntityId` ensures that `useArmiesByEntityOwner` does not receive an undefined value, which could lead to unexpected behavior or errors.	8
Best practice	Use `const` for variables that are not reassigned to emphasize immutability ___ Replace the `let` declaration with `const` for `totalHyperstructurePoints` since it is not reassigned after its initial calculation. This change enhances code clarity and immutability. [client/src/hooks/store/useLeaderBoardStore.tsx [142]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-540f755145619d641c732e0f62d7d62491556a2546e505509d2fe24bcd9dfa04R142-R142) ```diff -let totalHyperstructurePoints = HYPERSTRUCTURE_POINTS_PER_CYCLE * nbOfCycles; +const totalHyperstructurePoints = HYPERSTRUCTURE_POINTS_PER_CYCLE * nbOfCycles; ``` Suggestion importance[1-10]: 7 Why: Replacing `let` with `const` for `totalHyperstructurePoints` enhances code clarity and emphasizes immutability, which is a best practice.	7
Best practice	Use nullish coalescing to handle potential undefined values more explicitly ___ Use nullish coalescing operator to provide default values for `selectedArmy?.x` and `selectedArmy?.y` to ensure that the coordinates are always valid numbers. [client/src/ui/modules/military/battle-view/BattleActions.tsx [110]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-a2b38e5f9c040c3591adb945a143c3fe234beb433a77a677a8d069ea47a6a462R110-R110) ```diff -setBattleView({ battle: { x: selectedArmy?.x \|\| 0, y: selectedArmy?.y \|\| 0 }, target: undefined }); +setBattleView({ battle: { x: selectedArmy?.x ?? 0, y: selectedArmy?.y ?? 0 }, target: undefined }); ``` Suggestion importance[1-10]: 7 Why: Using the nullish coalescing operator (`??`) is a best practice for handling potential undefined values, making the code more explicit and safer.	7
Maintainability	Simplify the calculation of `maxContributableAmount` to enhance code readability ___ Refactor the calculation of `maxContributableAmount` to a single expression to improve readability and reduce complexity. [client/src/ui/components/hyperstructures/HyperstructureResourceChip.tsx [37-39]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-7e16180f0b985ff45a36af6b89c3452bb5216cda722df951c420f1dd3ad87610R37-R39) ```diff -let maxContributableAmount = Math.min(progress.costNeeded! - progress.amount, balance); -maxContributableAmount = progress.costNeeded - progress.amount > balance ? safetyMargin : 1; -maxContributableAmount = Math.floor(maxContributableAmount); +const maxContributableAmount = Math.floor(Math.min(progress.costNeeded! - progress.amount, balance) (progress.costNeeded - progress.amount > balance ? safetyMargin : 1)); ``` Suggestion importance[1-10]: 6 Why: The refactoring of `maxContributableAmount` into a single expression improves readability and reduces complexity, making the code easier to understand.	6

Category	Suggestion	Score
Possible bug	Add a null check for `battle` to prevent potential runtime errors ___ Consider adding a null check for `battle` in the `checkIfArmyLostAFinishedBattle` function to prevent potential runtime errors when `battle` is undefined. [client/src/hooks/helpers/useArmies.tsx [573-575]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-6d68c14fbe3262def9a16f121115d2006c7c34bf942b3e5d8ed7c076d2b89027R573-R575) ```diff const battle = getExtraBattleInformation([getEntityIdFromKeys([BigInt(army.battle_id)])], Battle, Position, Realm)[0]; -if (battle && armyIsLosingSide(army, battle!) && battleIsFinished(Army, battle)) { +if (!battle) return false; +if (armyIsLosingSide(army, battle) && battleIsFinished(Army, battle)) { return true; } ``` Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies a potential runtime error if `battle` is undefined and provides a robust solution by adding a null check. This is crucial for preventing unexpected crashes.	9
	Add null checks for variables before using them to prevent runtime errors ___ Consider checking for the existence of `ownArmyBattleStarter` and `defenderArmyBattleStarter` before using them to prevent runtime errors if they are undefined. [client/src/ui/modules/military/battle-view/BattleView.tsx [46-51]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-9926efdce78d03bb2703f0b52f301b5125b0e15fc3676eeae4645d8f90e6146fR46-R51) ```diff -: [ownArmyBattleStarter!]; -: [defenderArmyBattleStarter!]; +: ownArmyBattleStarter ? [ownArmyBattleStarter] : []; +: defenderArmyBattleStarter ? [defenderArmyBattleStarter] : []; ``` Suggestion importance[1-10]: 9 Why: Adding null checks for `ownArmyBattleStarter` and `defenderArmyBattleStarter` is crucial to prevent potential runtime errors if these variables are undefined. This is a significant improvement for code robustness.	9
	Ensure `filterArmy` handles null or undefined `army` values safely ___ Refactor the `filterArmy` function to ensure it correctly handles cases where `army` might be undefined or null, to avoid runtime errors. [client/src/hooks/helpers/useArmies.tsx [585-589]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-6d68c14fbe3262def9a16f121115d2006c7c34bf942b3e5d8ed7c076d2b89027R585-R589) ```diff +if (!army) return false; return ( (checkIfArmyAlive(army) && checkIfArmyLostAFinishedBattle(Battle, Army, army, Position, Realm) === false) \|\| - BigInt(army?.protectee_id \|\| 0) !== 0n + BigInt(army.protectee_id \|\| 0) !== 0n ); ``` Suggestion importance[1-10]: 8 Why: The suggestion addresses a potential runtime error by adding a check for undefined or null `army` values, which is important for maintaining code stability.	8
	Add checks to handle potential undefined returns from `getArmy` ___ Ensure that the `getArmy` function is always returning a value or handle potential undefined returns where it's used to avoid runtime errors. [client/src/hooks/helpers/useResources.tsx [123-124]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-da868a5a06b096d28f8b0e970a0260a8d26d34f5acfafa3dcbd0156da8f15749R123-R124) ```diff const { getArmy } = getArmyByEntityId(); const army = getArmy(position?.entity_id \|\| BigInt(0)); -if (!army \|\| !filterArmy(army, Battle, Army, Position, Realm)) return undefined; +if (!army) return undefined; +if (!filterArmy(army, Battle, Army, Position, Realm)) return undefined; ``` Suggestion importance[1-10]: 8 Why: The suggestion improves the robustness of the code by ensuring that `getArmy` returns a defined value before proceeding, thus preventing possible runtime errors.	8
	Add a fallback value for `attackerTroops` to handle undefined cases safely ___ Ensure that `attackerTroops` is defined before using it to avoid potential runtime errors. [client/src/ui/modules/military/battle-view/BattleView.tsx [99]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-9926efdce78d03bb2703f0b52f301b5125b0e15fc3676eeae4645d8f90e6146fR99-R99) ```diff -attackerTroops={attackerTroops!} +attackerTroops={attackerTroops ?? []} ``` Suggestion importance[1-10]: 8 Why: Ensuring `attackerTroops` is defined before usage is important to avoid runtime errors. Providing a fallback value improves code safety and reliability.	8
Enhancement	Use `reduce` method for a more functional and concise calculation of `TOTAL_CONTRIBUTABLE_AMOUNT` ___ Consider using the `reduce` method directly on `HYPERSTRUCTURE_TOTAL_COSTS_SCALED` to calculate `TOTAL_CONTRIBUTABLE_AMOUNT` instead of using a separate `forEach` loop. This approach is more functional and concise. [client/src/hooks/store/useLeaderBoardStore.tsx [44-49]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-540f755145619d641c732e0f62d7d62491556a2546e505509d2fe24bcd9dfa04R44-R49) ```diff -export const TOTAL_CONTRIBUTABLE_AMOUNT: number = 0; -HYPERSTRUCTURE_TOTAL_COSTS_SCALED.forEach(({ resource, amount }) => { - TOTAL_CONTRIBUTABLE_AMOUNT += ResourceMultipliers[resource as keyof typeof ResourceMultipliers]! * amount; -}); +export const TOTAL_CONTRIBUTABLE_AMOUNT: number = HYPERSTRUCTURE_TOTAL_COSTS_SCALED.reduce( + (total, { resource, amount }) => { + return total + (ResourceMultipliers[resource as keyof typeof ResourceMultipliers] ?? 0) * amount; + }, + 0, +); ``` Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies a more functional and concise way to calculate `TOTAL_CONTRIBUTABLE_AMOUNT` using `reduce`, which improves readability and maintainability.	9
Error handling	Add error handling for `getContributions` to manage potential failures gracefully ___ Ensure that the `getContributions` function handles potential errors or exceptions, especially if it involves asynchronous operations or fetching data from external sources. [client/src/hooks/store/useLeaderBoardStore.tsx [138]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-540f755145619d641c732e0f62d7d62491556a2546e505509d2fe24bcd9dfa04R138-R138) ```diff -const contributions = getContributions(hyperstructureEntityId); +let contributions; +try { + contributions = getContributions(hyperstructureEntityId); +} catch (error) { + console.error("Failed to get contributions:", error); + return []; // Return an empty array or handle as needed +} ``` Suggestion importance[1-10]: 8 Why: Adding error handling for `getContributions` is a good practice to ensure robustness, especially if the function involves asynchronous operations or external data fetching.	8
Possible issue	Add a null check for `realmEntityId` to prevent passing undefined to `useArmiesByEntityOwner` ___ Replace the direct usage of `realmEntityId` with a null check to ensure it is defined before using it in `useArmiesByEntityOwner`. [client/src/ui/modules/navigation/LeftNavigationModule.tsx [71]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-be9e8648ba688d13b3b68afca51d87c31cca8d9412f8c7d77edee0d71737a287R71-R71) ```diff -const { entityArmies } = useArmiesByEntityOwner({ entity_owner_entity_id: realmEntityId }); +const { entityArmies } = useArmiesByEntityOwner({ entity_owner_entity_id: realmEntityId ?? '' }); ``` Suggestion importance[1-10]: 8 Why: Adding a null check for `realmEntityId` ensures that `useArmiesByEntityOwner` does not receive an undefined value, which could lead to unexpected behavior or errors.	8
Best practice	Use `const` for variables that are not reassigned to emphasize immutability ___ Replace the `let` declaration with `const` for `totalHyperstructurePoints` since it is not reassigned after its initial calculation. This change enhances code clarity and immutability. [client/src/hooks/store/useLeaderBoardStore.tsx [142]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-540f755145619d641c732e0f62d7d62491556a2546e505509d2fe24bcd9dfa04R142-R142) ```diff -let totalHyperstructurePoints = HYPERSTRUCTURE_POINTS_PER_CYCLE * nbOfCycles; +const totalHyperstructurePoints = HYPERSTRUCTURE_POINTS_PER_CYCLE * nbOfCycles; ``` Suggestion importance[1-10]: 7 Why: Replacing `let` with `const` for `totalHyperstructurePoints` enhances code clarity and emphasizes immutability, which is a best practice.	7
Best practice	Use nullish coalescing to handle potential undefined values more explicitly ___ Use nullish coalescing operator to provide default values for `selectedArmy?.x` and `selectedArmy?.y` to ensure that the coordinates are always valid numbers. [client/src/ui/modules/military/battle-view/BattleActions.tsx [110]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-a2b38e5f9c040c3591adb945a143c3fe234beb433a77a677a8d069ea47a6a462R110-R110) ```diff -setBattleView({ battle: { x: selectedArmy?.x \|\| 0, y: selectedArmy?.y \|\| 0 }, target: undefined }); +setBattleView({ battle: { x: selectedArmy?.x ?? 0, y: selectedArmy?.y ?? 0 }, target: undefined }); ``` Suggestion importance[1-10]: 7 Why: Using the nullish coalescing operator (`??`) is a best practice for handling potential undefined values, making the code more explicit and safer.	7
Maintainability	Simplify the calculation of `maxContributableAmount` to enhance code readability ___ Refactor the calculation of `maxContributableAmount` to a single expression to improve readability and reduce complexity. [client/src/ui/components/hyperstructures/HyperstructureResourceChip.tsx [37-39]](https://github.com/BibliothecaDAO/eternum/pull/1086/files#diff-7e16180f0b985ff45a36af6b89c3452bb5216cda722df951c420f1dd3ad87610R37-R39) ```diff -let maxContributableAmount = Math.min(progress.costNeeded! - progress.amount, balance); -maxContributableAmount = progress.costNeeded - progress.amount > balance ? safetyMargin : 1; -maxContributableAmount = Math.floor(maxContributableAmount); +const maxContributableAmount = Math.floor(Math.min(progress.costNeeded! - progress.amount, balance) (progress.costNeeded - progress.amount > balance ? safetyMargin : 1)); ``` Suggestion importance[1-10]: 6 Why: The refactoring of `maxContributableAmount` into a single expression improves readability and reduces complexity, making the code easier to understand.	6

BibliothecaDAO / eternum