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
Performance Concern The WorldmapScene class is very large and complex, which could lead to maintenance challenges and performance issues. Consider refactoring into smaller components or using more efficient data structures and algorithms.
Code Complexity The GameRenderer class handles multiple responsibilities such as scene management, event handling, and rendering logic. This could be refactored to separate concerns more clearly, improving readability and maintainability.
Possible Bug The ArmyManager class uses a complex system of managing armies that might be prone to errors, especially in the methods handling movement and updates. Ensure thorough testing and consider simplifying the logic.
Code Smell The TileManager class has methods with deeply nested logic and multiple responsibilities, which could be simplified or broken down into smaller methods to improve readability and testability.
Improve type safety by specifying more accurate types for parameters
Consider using a more descriptive type than any for the set and get parameters in the store slice functions to improve type safety and code readability.
Why: This suggestion significantly improves type safety and code readability by specifying more accurate types for the set and get parameters. This is a best practice in TypeScript to avoid the pitfalls of using any.
10
Use optional chaining to safely access deeply nested properties
Consider using optional chaining (?.) when accessing deeply nested properties to avoid potential runtime errors if any part of the chain is undefined. This is particularly useful in complex destructuring assignments where the structure of the object might not be guaranteed.
Why: This suggestion improves code safety by preventing potential runtime errors when accessing deeply nested properties. It is particularly useful in complex destructuring assignments where the structure of the object might not be guaranteed.
8
Improve type safety by specifying a more accurate type than any
Consider using a more specific type than any for the stats property to enhance type safety and code clarity.
Why: Specifying a more accurate type than any enhances type safety and code clarity, which is a good practice in TypeScript.
8
Integrate DOM updates with React lifecycle using React portals
Replace the direct manipulation of document.body with a more React-friendly approach, such as using React portals or state management, to integrate DOM updates within the React lifecycle.
-document.body.appendChild(this.stats.dom);-document.body.appendChild(this.renderer.domElement);+// Use React portals or state to manage these elements+ReactDOM.createPortal(this.stats.dom, document.body);+ReactDOM.createPortal(this.renderer.domElement, document.body);
Suggestion importance[1-10]: 7
Why: Using React portals for DOM manipulation aligns with React best practices, but the current implementation might not be straightforward to convert without additional context.
7
Use explicit React fragment tags for clarity
Replace the fragment shorthand <> with <React.Fragment> for better readability and consistency.
Why: While using <React.Fragment> instead of <> can improve readability and consistency, it is a minor improvement and does not significantly impact functionality or performance.
6
Security
Remove debug logging to enhance security and cleanliness of production logs
Remove the console.log statement to avoid exposing potentially sensitive information in production logs.
Why: Removing console.log statements in production code is a good practice to avoid exposing potentially sensitive information and to keep logs clean. This suggestion directly addresses a security concern.
9
Enhancement
Ensure uniqueness in the openedPopups list by using a Set
Refactor the openAllPopups method to avoid potential duplicates in the openedPopups array by using a Set to ensure uniqueness before converting it back to an array.
Why: This suggestion effectively prevents duplicates in the openedPopups array, enhancing the robustness of the code. Using a Set to ensure uniqueness is a simple yet powerful improvement.
9
Improve robustness of localStorage data retrieval and parsing
Consider using a more robust type checking for localStorage operations to handle cases where the item does not exist or is not a valid number. This will prevent potential runtime errors or incorrect values being used in the state.
Why: This suggestion improves the robustness of the code by handling cases where localStorage items might not exist or be invalid, preventing potential runtime errors and ensuring default values are used correctly.
9
Maintainability
Add comprehensive error handling for system calls
Implement error handling for the asynchronous explore and travel_hex system calls to manage exceptions and maintain application stability.
Why: Adding comprehensive error handling improves maintainability and stability of the application. This suggestion enhances the robustness of the code by ensuring errors are logged and managed appropriately.
8
Reduce code duplication by refactoring repeated camera update logic into a function
Refactor the repeated logic for setting camera position and target into a separate function to reduce code duplication and improve maintainability.
Why: Refactoring the repeated logic into a function improves maintainability and readability by reducing code duplication.
8
Use a function to dynamically calculate position offsets
Replace the hardcoded addition of FELT_CENTER with a dynamic calculation based on the hex position to ensure the position calculation remains accurate even if the configuration changes.
Why: This suggestion enhances maintainability by making the position calculation more flexible and adaptable to configuration changes. However, it introduces a minor complexity by requiring the definition of the getFeltCenterOffset function.
7
Performance
Optimize pathfinding with a proper priority queue
Replace the manual sort operation with a priority queue implementation for better performance in pathfinding.
-priorityQueue.sort((a, b) => a.distance - b.distance); // This makes the queue work as a priority queue+// Implement a proper priority queue for better performance+// Example placeholder for priority queue usage
Suggestion importance[1-10]: 7
Why: Replacing the manual sort operation with a proper priority queue can improve performance, especially in pathfinding algorithms. However, the suggestion lacks a concrete implementation, making it less actionable.
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
PR Type
Enhancement, Tests, Bug fix
Description
WorldmapSceneandHexceptionSceneclasses extendingHexagonScenefor managing world map and hexception scenes.ArmyMovementManagerandArmyManagerclasses for managing army movements and interactions.useUIStoreto include new slices for three.js state management.GameRendererclass for managing three.js rendering and scene transitions.TileManagerclass for managing tile and building data.ActionInfocomponent to use new state management for army actions.useExplorehook to remove unused functions and simplify logic.InteractiveHexManagerclass for managing interactive hexagons.Biomeclass for biome calculations and management.RightNavigationModuleto use new modal store.Changes walkthrough 📝
13 files
Worldmap.ts
Introduce `WorldmapScene` class with army and hexagon management.client/src/three/scenes/Worldmap.ts
WorldmapSceneclass extendingHexagonScene.interactions.
ArmyMovementManager.ts
Add `ArmyMovementManager` for army movements and pathfinding.client/src/dojo/modelManager/ArmyMovementManager.ts
ArmyMovementManagerclass for managing army movements.Hexception.ts
IntroduceHexceptionSceneclass with building and hexagon management.client/src/three/scenes/Hexception.ts
HexceptionSceneclass extendingHexagonScene.HexagonScene.ts
Add abstract `HexagonScene` class for common functionality.client/src/three/scenes/HexagonScene.ts
HexagonSceneclass for common hexagon scenefunctionality.
useUIStore.tsx
Refactor `useUIStore` with new slices and methods.client/src/hooks/store/useUIStore.tsx
useUIStoreto include new slices for three.js statemanagement.
GameRenderer.ts
Add `GameRenderer` class for rendering and scene management.client/src/three/GameRenderer.ts
GameRendererclass for managing three.js rendering and scenetransitions.
resize.
ArmyManager.ts
Add `ArmyManager` for managing army instances and movements.client/src/three/components/ArmyManager.ts
ArmyManagerclass for managing army instances and movements.TileManager.ts
Add `TileManager` for managing tile and building data.client/src/dojo/modelManager/TileManager.ts
TileManagerclass for managing tile and building data.ActionInfo.tsx
Update `ActionInfo` component with new state management.client/src/ui/components/worldmap/hexagon/ActionInfo.tsx
ActionInfocomponent to use new state management for armyactions.
useExplore.tsx
Refactor `useExplore` hook for simplification.client/src/hooks/helpers/useExplore.tsx
useExplorehook to remove unused functions and simplifylogic.
InteractiveHexManager.ts
Add `InteractiveHexManager` for interactive hexagon management.client/src/three/components/InteractiveHexManager.ts
InteractiveHexManagerclass for managing interactive hexagons.hexes.
Biome.ts
Add `Biome` class for biome calculations and management.client/src/three/components/Biome.ts
Biomeclass for biome calculations and management.moisture.
RightNavigationModule.tsx
Update `RightNavigationModule` to use new modal store.client/src/ui/modules/navigation/RightNavigationModule.tsx
RightNavigationModuleto use new modal store.46 files
InstancedModel.tsx
...client/src/three/components/InstancedModel.tsx
...
BaseThreeTooltip.tsx
...client/src/ui/elements/BaseThreeTooltip.tsx
...
BuildingPreview.ts
...client/src/three/components/BuildingPreview.ts
...
StructureManager.ts
...client/src/three/components/StructureManager.ts
...
SystemManager.ts
...client/src/three/systems/SystemManager.ts
...
useHexPosition.tsx
...client/src/hooks/helpers/useHexPosition.tsx
...
EntityDetails.tsx
...client/src/ui/modules/entity-details/EntityDetails.tsx
...
InputManager.ts
...client/src/three/components/InputManager.ts
...
World.tsx
...client/src/ui/layouts/World.tsx
...
_threeStore.tsx
...client/src/hooks/store/_threeStore.tsx
...
InstancedBuilding.tsx
...client/src/three/components/InstancedBuilding.tsx
...
utils.tsx
...client/src/ui/utils/utils.tsx
...
BattleView.tsx
...client/src/ui/modules/military/battle-view/BattleView.tsx
...
ArmyInfoLabel.tsx
...client/src/ui/components/worldmap/armies/ArmyInfoLabel.tsx
...
HighlightHexManager.ts
...client/src/three/components/HighlightHexManager.ts
...
_popupsStore.tsx
...client/src/hooks/store/_popupsStore.tsx
...
LabelManager.ts
...client/src/three/components/LabelManager.ts
...
FogManager.ts
...client/src/three/components/FogManager.ts
...
TransitionManager.tsx
...client/src/three/components/TransitionManager.tsx
...
TopMiddleNavigation.tsx
...client/src/ui/modules/navigation/TopMiddleNavigation.tsx
...
SceneManager.ts
...client/src/three/SceneManager.ts
...
Steps.tsx
...client/src/ui/modules/onboarding/Steps.tsx
...
LocationManager.ts
...client/src/three/helpers/LocationManager.ts
...
HooksComponent.tsx
...client/src/ui/components/HooksComponent.tsx
...
highlightHexMaterial.ts
...client/src/three/shaders/highlightHexMaterial.ts
...
MarketModal.tsx
...client/src/ui/components/trading/MarketModal.tsx
...
BattleActions.tsx
...client/src/ui/modules/military/battle-view/BattleActions.tsx
...
TopLeftNavigation.tsx
...client/src/ui/modules/navigation/TopLeftNavigation.tsx
...
HintModalButton.tsx
...client/src/ui/elements/HintModalButton.tsx
...
main.tsx
...client/src/main.tsx
...
types.ts
...client/src/three/systems/types.ts
...
BaseContainer.tsx
...client/src/ui/containers/BaseContainer.tsx
...
ModalContainer.tsx
...client/src/ui/components/ModalContainer.tsx
...
ticks.ts
...client/src/three/helpers/ticks.ts
...
Onboarding.tsx
...client/src/ui/layouts/Onboarding.tsx
...
DojoHtml.tsx
...client/src/ui/elements/DojoHtml.tsx
...
useModalStore.tsx
...client/src/hooks/store/useModalStore.tsx
...
global.ts
...sdk/packages/eternum/src/constants/global.ts
...
borderHexMaterial.ts
...client/src/three/shaders/borderHexMaterial.ts
...
transparentHexMaterial.ts
...client/src/three/shaders/transparentHexMaterial.ts
...
createClientComponents.ts
...client/src/dojo/createClientComponents.ts
...
App.tsx
...client/src/App.tsx
...
createSystemCalls.ts
...client/src/dojo/createSystemCalls.ts
...
GUIManager.ts
...client/src/three/helpers/GUIManager.ts
...
pnpm-lock.yaml
...pnpm-lock.yaml
...
package.json
...client/package.json
...