Battle reward box - Githubissues

User description

notes

everyone can now join and leave battle at any time irrespective of the outcome. i.e win or lose or draw
if you win or draw, you do not lose the resources you came to battle with. ie you dont lose the items you had when the battle started. (soon TM, more logic will to added to take items that the losers left. but that's it for now)
if you lose, you can still leave the battle but you lose all your items.
a structure can be pillaged most of the time. The only time it can't be pillaged is if it is in a battle AND the battle is ongoing
the pillager is advised to join the battle or wait for it to end
you can pillage a structure continuously (i.e without being sent back to your realm if it has no defence army or the army is dead)
you can only pillage a structure once before being returned to your realm if the pillaged structure has a standing and active army

PR Type

enhancement, bug fix, tests

Description

Improved styling and UI components across multiple files.
Refactored configuration and provider methods to support batch processing.
Simplified quest structure and added claimed status checks.
Added new TopLeftNavigation component with hints and settings buttons.
Enhanced MarketOrderPanel and MarketModal for better user experience.
Updated type definitions to support batch calls.

Changes walkthrough 📝

Relevant files

Enhancement

15 files

Headline.tsx `Improve styling and spacing in Headline component.` client/src/ui/elements/Headline.tsx Updated class names for better styling. Changed color classes for SVG elements. Adjusted spacing in the headline component.	+14/-9
index.ts `Refactor configuration functions to support batch processing.` sdk/packages/eternum/src/config/index.ts Refactored configuration functions to batch calls. Added arrays to collect calldata for batch processing. Updated logging for configuration status.	+53/-23
useQuests.tsx `Simplify quest structure and add claimed status check.` client/src/hooks/helpers/useQuests.tsx Simplified quest steps and prizes structure. Added logic to check if quests are claimed. Updated memoization for quests and claimable quests.	+31/-93
index.ts `Update provider methods to support batch calls.` sdk/packages/eternum/src/provider/index.ts Updated provider methods to handle batch calls. Refactored `set_weight_config`, `set_production_config`, and other methods.	+54/-29
TopMiddleNavigation.tsx `Optimize TopMiddleNavigation with memoization.` client/src/ui/modules/navigation/TopMiddleNavigation.tsx Added memoization for structures and hex view. Improved sorting logic for player structures.	+17/-28
HintBox.tsx `Simplify HintBox component by removing redundant logic.` client/src/ui/components/hints/HintBox.tsx Removed redundant `hasClaimed` logic. Added `claimed` status directly to quests. Updated UI to reflect claimed status.	+16/-22
ArmyChip.tsx `Enhance ArmyChip component with better styling and stamina display.` client/src/ui/components/military/ArmyChip.tsx Updated styling and class names for better UI. Added stamina resource display.	+16/-10
MarketOrderPanel.tsx `Improve MarketOrderPanel text and resource display.` client/src/ui/components/trading/MarketOrderPanel.tsx Updated text and labels for better clarity. Improved display of available resources and weights.	+13/-10
provider.ts `Update type definitions for batch call support.` sdk/packages/eternum/src/types/provider.ts Updated type definitions to support batch calls. Refactored `SetWeightConfigProps` and `SetProductionConfigProps`.	+16/-12
TroopChip.tsx `Add border styling to TroopChip component.` client/src/ui/components/military/TroopChip.tsx - Added border styling to troop menu rows.	+3/-3
MarketModal.tsx `Enhance MarketModal styling and add title.` client/src/ui/components/trading/MarketModal.tsx - Enhanced modal styling and added title.	+5/-2
TopLeftNavigation.tsx `Add TopLeftNavigation component with hints and settings buttons.` client/src/ui/modules/navigation/TopLeftNavigation.tsx Added new TopLeftNavigation component. Included buttons for hints and settings.	+36/-0
BottomNavigation.tsx `Remove settings button and adjust quest notifications in` `BottomNavigation.` client/src/ui/modules/navigation/BottomNavigation.tsx Removed settings button from BottomNavigation. Adjusted logic for displaying quest notifications.	+2/-14
ArmyManagementCard.tsx `Update grid layout and button text in ArmyManagementCard.` client/src/ui/components/military/ArmyManagementCard.tsx - Updated grid layout and button text.	+3/-3
StaminaResource.tsx `Update stamina display in StaminaResource component.` client/src/ui/elements/StaminaResource.tsx - Updated stamina display color and text.	+2/-3

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

Name	Status	Preview	Comments	Updated (UTC)
eternum	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 14, 2024 1:04pm

PR Code Suggestions ✨

Category	Suggestion	Score
Maintainability	Replace individual troop display elements with a loop to generate them dynamically ___ Consider using a loop to generate the troop display elements to reduce code duplication and improve maintainability. This will make it easier to add or modify troop types in the future without having to manually update each element. [client/src/ui/components/military/TroopChip.tsx [8-19]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-cbece6b978b65197edd5a82b070123d6ae7ad014017f363d3291a958e99803f5R8-R19) ```diff - - - {currencyFormat(army.troops.crossbowman_count, 0)} - - - - {currencyFormat(army.troops.knight_count, 0)} - - - - {currencyFormat(army.troops.paladin_count, 0)} - +{Object.entries(army.troops).map(([troopType, count]) => ( + + + {currencyFormat(count, 0)} + +))} ``` Suggestion importance[1-10]: 9 Why: This suggestion significantly improves maintainability by reducing code duplication and making it easier to add or modify troop types in the future. It directly addresses the new code introduced in the PR.	9
	Ensure consistent import style for `TopLeftContainer` ___ Ensure that `TopLeftContainer` is consistently imported with either default or named import across the project. If it's a named import elsewhere, change the import statement to maintain consistency. [client/src/ui/layouts/World.tsx [25]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-9a93bfa50f7ce28c5b800d7616c6951842cb9935ca80736ad99d90f64da6565aR25-R25) ```diff -import TopLeftContainer from "../containers/TopLeftContainer"; +import { TopLeftContainer } from "../containers/TopLeftContainer"; ``` Suggestion importance[1-10]: 7 Why: Ensuring consistent import style can improve code maintainability and readability. However, this is a minor issue and does not affect the functionality of the code.	7
	Review and adjust the padding and border styles for consistency with the design system ___ The padding and border styles have been modified. Ensure that these changes align with the design system's consistency, especially since the padding was increased and border color changed. If not intentional, consider reverting to the original styles or adjusting them according to the design guidelines. [client/src/ui/elements/ResourceCost.tsx [37-41]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-3a39b2a659bbd9184a5ea77d33fad2271c9f4141daa23f59f0a6c6eee54256b3R37-R41) ```diff className={clsx( - "relative flex items-center p-2 bg-gold/10 clip-angled-sm gap-1 border border-gold/10", + "relative flex items-center p-1 bg-gold/10 clip-angled-sm gap-1 border border-gold/5", type === "horizontal" ? "flex-row" : "flex-col justify-center", className, )} ``` Suggestion importance[1-10]: 6 Why: This suggestion addresses potential design inconsistencies by recommending a review of the padding and border styles to ensure they align with the design system. However, it is more of a design review than a critical code issue.	6
Possible bug	Add a check for undefined `entityId` before using it in a function call ___ Consider checking if `entityId` is undefined before using it in the `useEntityArmies` hook to avoid potential issues with invalid IDs. [client/src/hooks/helpers/useQuests.tsx [27]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-02207afc6e5bd13a56e092c44f234cbddae30dd5cf9704eb786a3b8f7da20d33R27-R27) ```diff -const { entityArmies } = useEntityArmies({ entity_id: entityId \|\| BigInt("0") }); +if (entityId === undefined) { + console.error("entityId is undefined"); + return; +} +const { entityArmies } = useEntityArmies({ entity_id: entityId }); ``` Suggestion importance[1-10]: 8 Why: This suggestion addresses a potential bug by ensuring that `entityId` is defined before using it, which can prevent runtime errors and improve code robustness.	8
Enhancement	Wrap new components in an error boundary for improved error handling ___ Consider adding error boundaries around new UI components to gracefully handle JavaScript errors in their child component tree. [client/src/ui/layouts/World.tsx [76-78]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-9a93bfa50f7ce28c5b800d7616c6951842cb9935ca80736ad99d90f64da6565aR76-R78) ```diff - - - + + + + + ``` Suggestion importance[1-10]: 8 Why: Adding error boundaries can significantly improve the robustness of the application by gracefully handling errors in the component tree. This is a valuable enhancement.	8
	Restore the background styling for consistency in UI appearance ___ Restore the background styling for the 'full' size modal to maintain consistent UI appearance. [client/src/ui/components/ModalContainer.tsx [15]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-a1513644e65e84da86182ce65ad29af34c4ece158341ed1f463f32ecde609985R15-R15) ```diff -? "w-full h-full bg-transparent p-4" +? "w-full h-full bg-battle-one bg-cover p-4" ``` Suggestion importance[1-10]: 7 Why: Restoring the background styling helps maintain a consistent UI appearance, which is important for user experience, but it is not a critical issue.	7
	Restore width and margin styles for consistent layout ___ Consider restoring the width and margin styles to the `ResourceCost` component to ensure consistent layout and spacing. [client/src/ui/components/entities/Entity.tsx [103]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-e41f7ca60992e606521c1c7a6cda0e416abefc1bc9ec7d0716bc4037a6cb1c43R103-R103) ```diff -className="!text-gold " +className="!text-gold !w-5 mt-0.5" ``` Suggestion importance[1-10]: 6 Why: Restoring the width and margin styles ensures consistent layout and spacing, which improves the visual consistency of the UI. However, it is a minor enhancement.	6
	Add back the `ornate-borders` class if its removal was unintentional to preserve the layout's design ___ The removal of the `ornate-borders` class might affect the visual layout of the onboarding screen. If the removal was unintentional, consider adding it back to maintain the intended stylistic design. [client/src/ui/layouts/Onboarding.tsx [32]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-df967e1ee188baa9cd08ac623cd22d7055f7dd3651538c78a351f825ae8203bcR32-R32) ```diff - + ``` Suggestion importance[1-10]: 5 Why: This suggestion addresses a potential visual layout issue by recommending the re-addition of a class that might have been removed unintentionally. It is a minor enhancement but important for maintaining the intended design.	5
Best practice	Add conditional rendering to ensure `className` is applied only when it is not null or undefined ___ Consider adding a conditional rendering for the `className` prop to ensure that it only appends additional classes when `className` is not null or undefined. This prevents potential issues with unwanted class names being applied. [client/src/ui/elements/Headline.tsx [15-17]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-58c6b068f95da4b941ade1f23a87a33ab911b54cbbdfa6e0942fa4c4b4778168R15-R17) ```diff className={clsx( "flex items-center justify-center select-none w-full clip-angled bg-gold/5 p-2 h6 font-bold", - className, + className && className, )} ``` Suggestion importance[1-10]: 8 Why: This suggestion improves code robustness by ensuring that `className` is only applied when it is not null or undefined, preventing potential issues with unwanted class names.	8
	Ensure loading state is reset correctly in asynchronous operations ___ Ensure that the loading state is reset in the `finally` block of `handleClaimResources` and `handleAllClaims` to handle UI state correctly across all scenarios. [client/src/ui/components/hints/HintBox.tsx [49-70]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-5cf22aac8f0607d764110992d7cb5eac12ced82af11478c86e79fd3659cefc6aR49-R70) ```diff -setIsLoading(false); +setIsLoading(false); // Ensure loading state is reset on completion ``` Suggestion importance[1-10]: 5 Why: Adding a comment to ensure the loading state is reset on completion is a good practice for code readability and maintainability, but it does not change the functionality.	5
	Add a key prop to `TopLeftContainer` for better React reconciliation ___ Add a key prop to the `TopLeftContainer` component when used in a list or iteration to help React identify which items have changed, are added, or are removed. [client/src/ui/layouts/World.tsx [76-78]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-9a93bfa50f7ce28c5b800d7616c6951842cb9935ca80736ad99d90f64da6565aR76-R78) ```diff - + ``` Suggestion importance[1-10]: 3 Why: Adding a key prop is important when rendering lists or iterating over components. In this case, `TopLeftContainer` is not part of a list, so this suggestion is not applicable.	3
Consistency	Ensure consistent unit notation for weights across the application ___ Use a consistent unit for displaying weights across the application. If 'kgs' is used elsewhere, ensure it's used here as well to maintain consistency and avoid confusion. [client/src/ui/components/trading/MarketOrderPanel.tsx [421]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR421-R421) ```diff - {divideByPrecision(orderWeight).toLocaleString()} kgs + {divideByPrecision(orderWeight).toLocaleString()} kg ``` Suggestion importance[1-10]: 8 Why: Consistency in unit notation is important for user understanding and avoiding confusion. This suggestion ensures that the weight unit is consistently displayed as 'kg' across the application.	8
Usability	Improve button text for clarity and user understanding ___ Consider using a more descriptive button text that includes the action and the item. This makes the interface more intuitive, especially for new users. [client/src/ui/components/trading/MarketOrderPanel.tsx [434]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR434-R434) ```diff -{isBuy ? "Buy" : `Sell `} {resource.toLocaleString()} {findResourceById(resourceId)?.trait} +{isBuy ? "Buy" : "Sell"} {resource.toLocaleString()} {findResourceById(resourceId)?.trait \|\| "Resource"} ``` Suggestion importance[1-10]: 7 Why: The suggestion enhances usability by making the button text more descriptive and intuitive, which is especially helpful for new users. However, the improvement is relatively minor.	7
Usability	Update the placeholder text in the 'Select' component for clarity and consistency ___ Ensure that the 'Select' component's placeholder text is consistent with the UI's terminology and context. If 'Select Realm' is not clear or consistent with other similar UI elements, consider a more descriptive placeholder. [client/src/ui/components/trading/MarketModal.tsx [50]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-60c160393d9cf2b5da3291c081dd0d823b49d1cb1dcdda811544686c0ddd347eR50-R50) ```diff - + ``` Suggestion importance[1-10]: 6 Why: While the suggestion improves usability by making the placeholder text more descriptive, the existing text is already fairly clear. This is a minor improvement.	6
Possible issue	Revert the stamina level high color to green unless the change was intentional based on new guidelines or feedback ___ The color for indicating stamina levels has been changed from green to yellow for high levels. This might confuse users accustomed to the previous color coding. If the change was not based on user feedback or a new design guideline, consider reverting to the original color to maintain user experience consistency. [client/src/ui/elements/StaminaResource.tsx [10]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-032a58b10e8dfd49cf812dbc1a0839edbff0d9696cea730d590aba2721135f23R10-R10) ```diff -(stamina?.amount \|\| 0n) < EternumGlobalConfig.stamina.travelCost ? "text-red/90" : "text-yellow/90"; +(stamina?.amount \|\| 0n) < EternumGlobalConfig.stamina.travelCost ? "text-red/90" : "text-green/90"; ``` Suggestion importance[1-10]: 7 Why: This suggestion highlights a potential user experience issue by pointing out a change in color coding that could confuse users. It is important for maintaining consistency in user interface design.	7
Possible issue	Verify dependency management for `TopLeftNavigation` ___ Verify that the added `TopLeftNavigation` module has its dependencies correctly managed in the project's state management system or context to ensure it functions correctly within the `TopLeftContainer`. [client/src/ui/layouts/World.tsx [76-78]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-9a93bfa50f7ce28c5b800d7616c6951842cb9935ca80736ad99d90f64da6565aR76-R78) ```diff - + // Ensure dependencies are managed ``` Suggestion importance[1-10]: 6 Why: Ensuring that the `TopLeftNavigation` module has its dependencies correctly managed is important for the functionality of the component. However, the suggestion is somewhat vague and does not provide a concrete improvement to the code.	6

⏱️ Estimated effort to review [1-5]	4
🧪 Relevant tests	No
🔒 Security concerns	- Resource Locking: The changes in `combat_systems.cairo` and `resource_systems.cairo` involve resource locking mechanisms that are critical for preventing unauthorized access or double-spending of resources during battles. It's essential to ensure that these locking mechanisms are foolproof and tested against potential race conditions or other security vulnerabilities.
⚡ Key issues to review	Possible Bug: The logic for handling battles and resource locking in `combat_systems.cairo` seems complex and could be prone to errors. Specifically, the changes around how battles are initiated and how resources are locked and unlocked during battles need thorough testing to ensure they behave as expected under various scenarios.
	Performance Concern: The modifications in `combat_systems.cairo` introduce additional complexity in the battle handling logic, which could impact performance, especially with multiple concurrent battles and resource transactions. It's crucial to assess if these changes could lead to increased gas costs or delays in transaction processing.
	Code Clarity: The new methods `balance_deposit` and `withdraw_deposit` in `combat_systems.cairo` could benefit from more detailed inline comments explaining the steps involved, especially how resources are managed during battles. This would improve maintainability and understandability of the code.

Category	Suggestion	Score
Maintainability	Replace individual troop display elements with a loop to generate them dynamically ___ Consider using a loop to generate the troop display elements to reduce code duplication and improve maintainability. This will make it easier to add or modify troop types in the future without having to manually update each element. [client/src/ui/components/military/TroopChip.tsx [8-19]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-cbece6b978b65197edd5a82b070123d6ae7ad014017f363d3291a958e99803f5R8-R19) ```diff - - - {currencyFormat(army.troops.crossbowman_count, 0)} - - - - {currencyFormat(army.troops.knight_count, 0)} - - - - {currencyFormat(army.troops.paladin_count, 0)} - +{Object.entries(army.troops).map(([troopType, count]) => ( + + + {currencyFormat(count, 0)} + +))} ``` Suggestion importance[1-10]: 9 Why: This suggestion significantly improves maintainability by reducing code duplication and making it easier to add or modify troop types in the future. It directly addresses the new code introduced in the PR.	9
	Ensure consistent import style for `TopLeftContainer` ___ Ensure that `TopLeftContainer` is consistently imported with either default or named import across the project. If it's a named import elsewhere, change the import statement to maintain consistency. [client/src/ui/layouts/World.tsx [25]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-9a93bfa50f7ce28c5b800d7616c6951842cb9935ca80736ad99d90f64da6565aR25-R25) ```diff -import TopLeftContainer from "../containers/TopLeftContainer"; +import { TopLeftContainer } from "../containers/TopLeftContainer"; ``` Suggestion importance[1-10]: 7 Why: Ensuring consistent import style can improve code maintainability and readability. However, this is a minor issue and does not affect the functionality of the code.	7
	Review and adjust the padding and border styles for consistency with the design system ___ The padding and border styles have been modified. Ensure that these changes align with the design system's consistency, especially since the padding was increased and border color changed. If not intentional, consider reverting to the original styles or adjusting them according to the design guidelines. [client/src/ui/elements/ResourceCost.tsx [37-41]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-3a39b2a659bbd9184a5ea77d33fad2271c9f4141daa23f59f0a6c6eee54256b3R37-R41) ```diff className={clsx( - "relative flex items-center p-2 bg-gold/10 clip-angled-sm gap-1 border border-gold/10", + "relative flex items-center p-1 bg-gold/10 clip-angled-sm gap-1 border border-gold/5", type === "horizontal" ? "flex-row" : "flex-col justify-center", className, )} ``` Suggestion importance[1-10]: 6 Why: This suggestion addresses potential design inconsistencies by recommending a review of the padding and border styles to ensure they align with the design system. However, it is more of a design review than a critical code issue.	6
Possible bug	Add a check for undefined `entityId` before using it in a function call ___ Consider checking if `entityId` is undefined before using it in the `useEntityArmies` hook to avoid potential issues with invalid IDs. [client/src/hooks/helpers/useQuests.tsx [27]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-02207afc6e5bd13a56e092c44f234cbddae30dd5cf9704eb786a3b8f7da20d33R27-R27) ```diff -const { entityArmies } = useEntityArmies({ entity_id: entityId \|\| BigInt("0") }); +if (entityId === undefined) { + console.error("entityId is undefined"); + return; +} +const { entityArmies } = useEntityArmies({ entity_id: entityId }); ``` Suggestion importance[1-10]: 8 Why: This suggestion addresses a potential bug by ensuring that `entityId` is defined before using it, which can prevent runtime errors and improve code robustness.	8
Enhancement	Wrap new components in an error boundary for improved error handling ___ Consider adding error boundaries around new UI components to gracefully handle JavaScript errors in their child component tree. [client/src/ui/layouts/World.tsx [76-78]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-9a93bfa50f7ce28c5b800d7616c6951842cb9935ca80736ad99d90f64da6565aR76-R78) ```diff - - - + + + + + ``` Suggestion importance[1-10]: 8 Why: Adding error boundaries can significantly improve the robustness of the application by gracefully handling errors in the component tree. This is a valuable enhancement.	8
	Restore the background styling for consistency in UI appearance ___ Restore the background styling for the 'full' size modal to maintain consistent UI appearance. [client/src/ui/components/ModalContainer.tsx [15]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-a1513644e65e84da86182ce65ad29af34c4ece158341ed1f463f32ecde609985R15-R15) ```diff -? "w-full h-full bg-transparent p-4" +? "w-full h-full bg-battle-one bg-cover p-4" ``` Suggestion importance[1-10]: 7 Why: Restoring the background styling helps maintain a consistent UI appearance, which is important for user experience, but it is not a critical issue.	7
	Restore width and margin styles for consistent layout ___ Consider restoring the width and margin styles to the `ResourceCost` component to ensure consistent layout and spacing. [client/src/ui/components/entities/Entity.tsx [103]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-e41f7ca60992e606521c1c7a6cda0e416abefc1bc9ec7d0716bc4037a6cb1c43R103-R103) ```diff -className="!text-gold " +className="!text-gold !w-5 mt-0.5" ``` Suggestion importance[1-10]: 6 Why: Restoring the width and margin styles ensures consistent layout and spacing, which improves the visual consistency of the UI. However, it is a minor enhancement.	6
	Add back the `ornate-borders` class if its removal was unintentional to preserve the layout's design ___ The removal of the `ornate-borders` class might affect the visual layout of the onboarding screen. If the removal was unintentional, consider adding it back to maintain the intended stylistic design. [client/src/ui/layouts/Onboarding.tsx [32]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-df967e1ee188baa9cd08ac623cd22d7055f7dd3651538c78a351f825ae8203bcR32-R32) ```diff - + ``` Suggestion importance[1-10]: 5 Why: This suggestion addresses a potential visual layout issue by recommending the re-addition of a class that might have been removed unintentionally. It is a minor enhancement but important for maintaining the intended design.	5
Best practice	Add conditional rendering to ensure `className` is applied only when it is not null or undefined ___ Consider adding a conditional rendering for the `className` prop to ensure that it only appends additional classes when `className` is not null or undefined. This prevents potential issues with unwanted class names being applied. [client/src/ui/elements/Headline.tsx [15-17]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-58c6b068f95da4b941ade1f23a87a33ab911b54cbbdfa6e0942fa4c4b4778168R15-R17) ```diff className={clsx( "flex items-center justify-center select-none w-full clip-angled bg-gold/5 p-2 h6 font-bold", - className, + className && className, )} ``` Suggestion importance[1-10]: 8 Why: This suggestion improves code robustness by ensuring that `className` is only applied when it is not null or undefined, preventing potential issues with unwanted class names.	8
	Ensure loading state is reset correctly in asynchronous operations ___ Ensure that the loading state is reset in the `finally` block of `handleClaimResources` and `handleAllClaims` to handle UI state correctly across all scenarios. [client/src/ui/components/hints/HintBox.tsx [49-70]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-5cf22aac8f0607d764110992d7cb5eac12ced82af11478c86e79fd3659cefc6aR49-R70) ```diff -setIsLoading(false); +setIsLoading(false); // Ensure loading state is reset on completion ``` Suggestion importance[1-10]: 5 Why: Adding a comment to ensure the loading state is reset on completion is a good practice for code readability and maintainability, but it does not change the functionality.	5
	Add a key prop to `TopLeftContainer` for better React reconciliation ___ Add a key prop to the `TopLeftContainer` component when used in a list or iteration to help React identify which items have changed, are added, or are removed. [client/src/ui/layouts/World.tsx [76-78]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-9a93bfa50f7ce28c5b800d7616c6951842cb9935ca80736ad99d90f64da6565aR76-R78) ```diff - + ``` Suggestion importance[1-10]: 3 Why: Adding a key prop is important when rendering lists or iterating over components. In this case, `TopLeftContainer` is not part of a list, so this suggestion is not applicable.	3
Consistency	Ensure consistent unit notation for weights across the application ___ Use a consistent unit for displaying weights across the application. If 'kgs' is used elsewhere, ensure it's used here as well to maintain consistency and avoid confusion. [client/src/ui/components/trading/MarketOrderPanel.tsx [421]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR421-R421) ```diff - {divideByPrecision(orderWeight).toLocaleString()} kgs + {divideByPrecision(orderWeight).toLocaleString()} kg ``` Suggestion importance[1-10]: 8 Why: Consistency in unit notation is important for user understanding and avoiding confusion. This suggestion ensures that the weight unit is consistently displayed as 'kg' across the application.	8
Usability	Improve button text for clarity and user understanding ___ Consider using a more descriptive button text that includes the action and the item. This makes the interface more intuitive, especially for new users. [client/src/ui/components/trading/MarketOrderPanel.tsx [434]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-00bad72a59745d3d904730035fcdeaddda30f53c9212d94f35f277ef202bd0bdR434-R434) ```diff -{isBuy ? "Buy" : `Sell `} {resource.toLocaleString()} {findResourceById(resourceId)?.trait} +{isBuy ? "Buy" : "Sell"} {resource.toLocaleString()} {findResourceById(resourceId)?.trait \|\| "Resource"} ``` Suggestion importance[1-10]: 7 Why: The suggestion enhances usability by making the button text more descriptive and intuitive, which is especially helpful for new users. However, the improvement is relatively minor.	7
Usability	Update the placeholder text in the 'Select' component for clarity and consistency ___ Ensure that the 'Select' component's placeholder text is consistent with the UI's terminology and context. If 'Select Realm' is not clear or consistent with other similar UI elements, consider a more descriptive placeholder. [client/src/ui/components/trading/MarketModal.tsx [50]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-60c160393d9cf2b5da3291c081dd0d823b49d1cb1dcdda811544686c0ddd347eR50-R50) ```diff - + ``` Suggestion importance[1-10]: 6 Why: While the suggestion improves usability by making the placeholder text more descriptive, the existing text is already fairly clear. This is a minor improvement.	6
Possible issue	Revert the stamina level high color to green unless the change was intentional based on new guidelines or feedback ___ The color for indicating stamina levels has been changed from green to yellow for high levels. This might confuse users accustomed to the previous color coding. If the change was not based on user feedback or a new design guideline, consider reverting to the original color to maintain user experience consistency. [client/src/ui/elements/StaminaResource.tsx [10]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-032a58b10e8dfd49cf812dbc1a0839edbff0d9696cea730d590aba2721135f23R10-R10) ```diff -(stamina?.amount \|\| 0n) < EternumGlobalConfig.stamina.travelCost ? "text-red/90" : "text-yellow/90"; +(stamina?.amount \|\| 0n) < EternumGlobalConfig.stamina.travelCost ? "text-red/90" : "text-green/90"; ``` Suggestion importance[1-10]: 7 Why: This suggestion highlights a potential user experience issue by pointing out a change in color coding that could confuse users. It is important for maintaining consistency in user interface design.	7
Possible issue	Verify dependency management for `TopLeftNavigation` ___ Verify that the added `TopLeftNavigation` module has its dependencies correctly managed in the project's state management system or context to ensure it functions correctly within the `TopLeftContainer`. [client/src/ui/layouts/World.tsx [76-78]](https://github.com/BibliothecaDAO/eternum/pull/927/files#diff-9a93bfa50f7ce28c5b800d7616c6951842cb9935ca80736ad99d90f64da6565aR76-R78) ```diff - + // Ensure dependencies are managed ``` Suggestion importance[1-10]: 6 Why: Ensuring that the `TopLeftNavigation` module has its dependencies correctly managed is important for the functionality of the component. However, the suggestion is somewhat vague and does not provide a concrete improvement to the code.	6

BibliothecaDAO / eternum

Battle reward box #927

User description

PR Type

Description

Changes walkthrough 📝

PR Reviewer Guide 🔍

PR Code Suggestions ✨