[client] fixes and optimizations

[r0man1337]

User description

1. Removed auto-close of side bars #917
2. Fixed bug with shadows map bounds
3. Castles and Banks refactored to use Instanced Mesh (big performance/fps boost) #580
4. Added support for Low Graphics setting #939
5. Added Castles labels (on hover) (partial #928)
6. Added distance factor to Army labels
7. Selected unit highlight component rename
8. Added lite models for Bank and Castle (using 2 meshes and 2 materials, for simpler usage with instances)
9. Removed Anger order reference from Bank model

---

PR Type

Enhancement, Bug fix

---

Description

• Added new InstancedBanks and InstancedCastles components for optimized rendering of banks and castles using instanced meshes.
• Integrated the new instanced components into the main structure rendering logic.
• Renamed SelectedUnit component to UnitHighlight and updated relevant references.
• Added support for low graphics settings, including conditional rendering based on LOW_GRAPHICS_FLAG.
• Removed auto-close functionality from left and right navigation modules.
• Fixed shadow camera bounds for directional light in WorldMapScene.
• Added UI elements for toggling between low and high graphics settings.
• Improved tooltip positioning and rendering logic for flags by adding hitbox instances.

---

Changes walkthrough 📝

	Relevant files
Enhancement	8 files InstancedBanks.tsx Add `InstancedBanks` component for optimized bank rendering. client/src/ui/components/models/buildings/worldmap/InstancedBanks.tsx Added new component InstancedBanks for rendering banks using instanced meshes. Utilized useGLTF for loading bank models. Applied transformations and optimizations for performance. +63/-0    InstancedCastles.tsx Add `InstancedCastles` component for optimized castle rendering. client/src/ui/components/models/buildings/worldmap/InstancedCastles.tsx Added new component InstancedCastles for rendering castles using instanced meshes. Utilized useGLTF for loading castle models. Applied transformations and optimizations for performance. +63/-0    Structures.tsx Integrate instanced components and refactor structure rendering. client/src/ui/components/models/buildings/worldmap/Structures.tsx Integrated InstancedCastles and InstancedBanks components. Refactored structure rendering logic for better performance. +20/-3    ArmyInfoLabel.tsx Add distance factor to army info label.                                    client/src/ui/components/worldmap/armies/ArmyInfoLabel.tsx - Added `distanceFactor` property to `BaseThreeTooltip`. +1/-0      WorldHexagon.tsx Remove unused `ShardsMines` import.                                            client/src/ui/components/worldmap/hexagon/WorldHexagon.tsx - Removed `ShardsMines` import. +0/-2      MainScene.tsx Add support for low graphics settings.                                      client/src/ui/modules/scenes/MainScene.tsx Added support for low graphics settings. Conditional rendering based on LOW_GRAPHICS_FLAG. +70/-59  Settings.tsx Add UI for graphics settings toggle.                                          client/src/ui/modules/settings/Settings.tsx - Added UI for toggling low and high graphics settings. +28/-0    Flags.jsx Add hitbox instances and improve tooltip rendering.            client/src/ui/components/worldmap/Flags.jsx Added hitbox instances for flags. Improved tooltip positioning and rendering logic. +36/-17
Bug fix	5 files Army.tsx Rename `SelectedUnit` to `UnitHighlight`.                                client/src/ui/components/worldmap/armies/Army.tsx - Renamed `SelectedUnit` component to `UnitHighlight`. +2/-2      UnitHighlight.tsx Rename `SelectedUnit` component and props.                              client/src/ui/components/worldmap/hexagon/UnitHighlight.tsx Renamed SelectedUnitProps to UnitHighlightProps. Renamed SelectedUnit component to UnitHighlight. +2/-2      LeftNavigationModule.tsx Remove auto-close functionality from left navigation.        client/src/ui/modules/navigation/LeftNavigationModule.tsx - Removed auto-close functionality for left navigation. +0/-18    RightNavigationModule.tsx Remove auto-close functionality from right navigation.      client/src/ui/modules/navigation/RightNavigationModule.tsx - Removed auto-close functionality for right navigation. +0/-9      WorldMapScene.tsx Fix shadow camera bounds for directional light.                    client/src/ui/modules/scenes/WorldMapScene.tsx - Fixed shadow camera bounds for directional light. +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]]

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 20, 2024 8:51am

[github-actions[bot]]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	4
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Performance Concern: The use of instanced meshes and the manipulation of matrix transformations in InstancedBanks and InstancedCastles are performance-critical areas. Reviewers should ensure that these operations are optimized and do not introduce memory leaks or excessive re-computation.
	Code Duplication: There is a noticeable amount of duplicated logic between InstancedBanks and InstancedCastles, particularly in how instances are created and transformed. Consider abstracting common functionality into a shared utility or base component.
	Hardcoded Values: Several hardcoded values (e.g., matrix transformations, geometry manipulations) are used in the new components. It's advisable to define these as constants or retrieve them from a configuration to enhance maintainability and flexibility.

[github-actions[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Add cleanup for hitBoxMeshes to prevent memory leaks ___ **To ensure that the hitBoxMeshes are correctly cleaned up to prevent memory leaks, consider adding a cleanup function in the useEffect where hitBoxInstances are set.** [client/src/ui/components/worldmap/Flags.jsx [197]](https://github.com/BibliothecaDAO/eternum/pull/961/files#diff-363ae1ff924f5e9c661aafb937b8548d4f7f0b0a563a5bbaa209c010cf5ffbe9R197-R197) ```diff -setHitBoxInstances(hitBoxMeshes); +useEffect(() => { + setHitBoxInstances(hitBoxMeshes); + return () => hitBoxMeshes.forEach(mesh => mesh.dispose()); +}, [hitBoxMeshes]); ``` Suggestion importance[1-10]: 10 Why: Adding a cleanup function in the useEffect hook is crucial for preventing memory leaks, ensuring that hitBoxMeshes are correctly disposed of when the component unmounts.	10
	Improve type safety by specifying a more accurate type than any ___ **Consider using a more specific type for the structure parameter in the forEach loop to avoid using any. This will help in catching potential bugs during compile time and improve code maintainability.** [client/src/ui/components/models/buildings/worldmap/InstancedBanks.tsx [40]](https://github.com/BibliothecaDAO/eternum/pull/961/files#diff-c3a035ff9c59c52207de2288ffb48fefdc6bbcc1ce3db59a830ead9a10240147R40-R40) ```diff -structures.forEach((structure: any) => { +structures.forEach((structure: StructureType) => { ``` Suggestion importance[1-10]: 7 Why: Using a more specific type than `any` helps catch potential bugs during compile time and improves code maintainability. However, the suggested type `StructureType` is incorrect; it should be the type that represents the structure object, not just its type property.	7
Performance	Improve performance and maintainability by managing graphics settings through state or context ___ **Instead of directly checking localStorage in the render method, use a state or context to manage the graphics settings. This approach avoids potential performance issues and ensures better reactivity and encapsulation.** [client/src/ui/modules/scenes/MainScene.tsx [130]](https://github.com/BibliothecaDAO/eternum/pull/961/files#diff-fc21828fcac93c01c5fc8d97c5f4206af9199daaf1230bc6a1b1a24d20bd50c8R130-R130) ```diff -{!localStorage.getItem("LOW_GRAPHICS_FLAG") && ( +{!isLowGraphics && ( ``` Suggestion importance[1-10]: 9 Why: Using state or context to manage graphics settings instead of directly checking `localStorage` in the render method improves performance and ensures better reactivity and encapsulation.	9
	Improve performance by using useMemo for initializing hitBoxInstances ___ **Consider initializing hitBoxInstances with a useMemo hook to ensure that the instances are only recalculated when ordersRealms changes, which is more efficient than recalculating on every render.** [client/src/ui/components/worldmap/Flags.jsx [57]](https://github.com/BibliothecaDAO/eternum/pull/961/files#diff-363ae1ff924f5e9c661aafb937b8548d4f7f0b0a563a5bbaa209c010cf5ffbe9R57-R57) ```diff -const [hitBoxInstances, setHitBoxInstances] = useState([]); +const hitBoxInstances = useMemo(() => ordersRealms.map(orderRealm => new THREE.InstancedMesh(hitBoxGeometry, hitBoxMaterial, orderRealm.length)), [ordersRealms]); ``` Suggestion importance[1-10]: 9 Why: Using useMemo for initializing hitBoxInstances ensures that the instances are only recalculated when ordersRealms changes, which improves performance by avoiding unnecessary recalculations on every render.	9
	Optimize resource usage and performance by memoizing the useGLTF hook call ___ **To prevent potential memory leaks or unnecessary re-renders, consider wrapping the useGLTF hook call inside a useMemo or useEffect to ensure it only re-executes when necessary.** [client/src/ui/components/models/buildings/worldmap/InstancedBanks.tsx [23]](https://github.com/BibliothecaDAO/eternum/pull/961/files#diff-c3a035ff9c59c52207de2288ffb48fefdc6bbcc1ce3db59a830ead9a10240147R23-R23) ```diff -const { nodes, materials } = useGLTF("/models/buildings/bank_lite.glb") as GLTFResult; +const { nodes, materials } = useMemo(() => useGLTF("/models/buildings/bank_lite.glb") as GLTFResult, []); ``` Suggestion importance[1-10]: 8 Why: Memoizing the `useGLTF` hook call can prevent unnecessary re-renders and optimize performance. This is a good practice for resource-intensive operations.	8
Enhancement	Enhance user experience and application responsiveness by using state management for graphics settings ___ **Use a more reactive approach to handle changes in graphics settings by using state management instead of directly manipulating localStorage and reloading the page.** [client/src/ui/modules/settings/Settings.tsx [86-87]](https://github.com/BibliothecaDAO/eternum/pull/961/files#diff-7fa22706f986ed33da69c4d0952bac0d4f9404336cbe235f93c09014a75a0cddR86-R87) ```diff -localStorage.setItem("LOW_GRAPHICS_FLAG", "true"); -window.location.reload(); +setGraphicsSetting("low"); // Assume setGraphicsSetting is a function that updates the state and is properly handled in the application. ``` Suggestion importance[1-10]: 9 Why: Using state management for graphics settings instead of directly manipulating `localStorage` and reloading the page provides a more reactive and user-friendly approach, enhancing the overall user experience.	9
Possible bug	Add error handling for undefined instanceId to avoid runtime errors ___ **It's recommended to add error handling for instanceId retrieval to avoid potential runtime errors if instanceId is undefined.** [client/src/ui/components/worldmap/Flags.jsx [227]](https://github.com/BibliothecaDAO/eternum/pull/961/files#diff-363ae1ff924f5e9c661aafb937b8548d4f7f0b0a563a5bbaa209c010cf5ffbe9R227-R227) ```diff -const instanceId = e.instanceId; +const instanceId = e.instanceId || 0; // Default to 0 or handle appropriately if undefined ``` Suggestion importance[1-10]: 8 Why: Adding error handling for instanceId retrieval helps avoid potential runtime errors if instanceId is undefined, improving the robustness of the code.	8
Maintainability	Extract repeated JSX groups into a separate React component for better maintainability ___ **To improve the readability and maintainability of the JSX structure, consider extracting the repeated group component into a separate React component.** [client/src/ui/components/worldmap/Flags.jsx [250-262]](https://github.com/BibliothecaDAO/eternum/pull/961/files#diff-363ae1ff924f5e9c661aafb937b8548d4f7f0b0a563a5bbaa209c010cf5ffbe9R250-R262) ```diff -<> - hoverHandler(e, index)} - onPointerLeave={() => { - posVector.set(0, 0, 0); - setTooltipPosition(posVector); - }} - > - - - clickHandler(e, index)}> - - - - + ``` Suggestion importance[1-10]: 7 Why: Extracting the repeated JSX groups into a separate React component improves the readability and maintainability of the code, making it easier to manage and understand.	7