add bank model back on world map + change coords

aymericdelab commented 3 months ago

PR Type

enhancement

Description

Added the Banks component back to the WorldMap JSX structure in WorldHexagon.tsx.
Adjusted the X coordinate for the bank model in index.ts.

Changes walkthrough 📝

Relevant files

Enhancement

WorldHexagon.tsx

Add Banks component to WorldMap JSX structure

client/src/ui/components/worldmap/hexagon/WorldHexagon.tsx

Imported Banks from the appropriate module.

Added Banks component to the JSX structure of WorldMap.

+2/-0

Configuration changes

index.ts

Change bank model X coordinate

config/bank/index.ts - Adjusted the X coordinate for the bank model.

+1/-1

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

vercel[bot] commented 3 months 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	Jun 28, 2024 2:41pm

github-actions[bot] commented 3 months ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	2
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Possible Bug: The addition of the `Banks` component in `WorldHexagon.tsx` should be accompanied by a check to ensure that the `Banks` data is available before rendering. This is to prevent potential runtime errors if the data is not initialized or fetched yet.

github-actions[bot] commented 3 months ago

PR Code Suggestions ✨

Category	Suggestion	Score
Maintainability	Add the '.js' extension to the 'Banks' import statement to maintain consistency with other imports ___ Ensure that the import statement for 'Banks' matches the convention used by other imports in the file. Specifically, consider adding the '.js' extension to maintain consistency. [client/src/ui/components/worldmap/hexagon/WorldHexagon.tsx [12]](https://github.com/BibliothecaDAO/eternum/pull/1042/files#diff-eab0cbdc1c33d4db9d2fda4be1bb8f506aff500e566c896d59d061882758345cR12-R12) ```diff -import { Banks } from "../../models/buildings/worldmap/Banks"; +import { Banks } from "../../models/buildings/worldmap/Banks.js"; ``` Suggestion importance[1-10]: 9 Why: This suggestion improves maintainability by ensuring consistency in import statements, which can help prevent potential issues with module resolution.	9
Maintainability	Document the reason for the change in 'COORD_X' value ___ Add a comment explaining why the value of 'COORD_X' was changed, as this helps in understanding the context and reasoning behind such changes. [config/bank/index.ts [34]](https://github.com/BibliothecaDAO/eternum/pull/1042/files#diff-4d2730971b1c5850f9ba84960a8c2015dd30635cca6f8fc0b5b4137ff395c9fcR34-R34) ```diff +// Adjusted COORD_X to align with new mapping requirements const COORD_X = 2147483899; ``` Suggestion importance[1-10]: 6 Why: Adding a comment for the change in 'COORD_X' value improves maintainability by providing context and reasoning, which is helpful for future developers.	6
Possible issue	Implement a check to ensure 'COORD_X' does not exceed JavaScript's safe integer limit ___ Consider using a safer data type or validation for 'COORD_X' to prevent potential overflow issues, as the value is very close to the maximum safe integer in JavaScript. [config/bank/index.ts [34]](https://github.com/BibliothecaDAO/eternum/pull/1042/files#diff-4d2730971b1c5850f9ba84960a8c2015dd30635cca6f8fc0b5b4137ff395c9fcR34-R34) ```diff -const COORD_X = 2147483899; +const COORD_X = Number.isSafeInteger(2147483899) ? 2147483899 : throw new Error('Value exceeds safe integer limit'); ``` Suggestion importance[1-10]: 8 Why: This suggestion addresses a possible issue by adding a safety check for integer overflow, which is crucial for preventing potential runtime errors.	8
Enhancement	Conditionally render the 'Banks' component based on relevant state or props ___ Consider wrapping the '' component in a conditional rendering to ensure it is only displayed when appropriate, based on the state or props. [client/src/ui/components/worldmap/hexagon/WorldHexagon.tsx [112]](https://github.com/BibliothecaDAO/eternum/pull/1042/files#diff-eab0cbdc1c33d4db9d2fda4be1bb8f506aff500e566c896d59d061882758345cR112-R112) ```diff - +{shouldDisplayBanks && } ``` Suggestion importance[1-10]: 7 Why: This enhancement improves the component's efficiency by ensuring that 'Banks' is only rendered when necessary, based on the state or props.	7

BibliothecaDAO / eternum