fix model manager in landing #2632
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Hi @aymericdelab! You need to be added as a user to interact with me. Please ask @ponderingdemocritus to add you on the settings page. You are receiving this comment because I am set to review all PRs. That is configurable here. |
WalkthroughThis pull request introduces a comprehensive refactoring of the game's client-side architecture, primarily focusing on consolidating imports, restructuring manager classes, and migrating utility functions and constants to a centralized SDK package called Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
… model-managers
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (5)
client/apps/landing/src/dojo/setup.ts (2)
Restore commented out model filters to ensure proper synchronization
The commented out models are actively used across the codebase in critical game systems, UI components, and core functionality. Keeping them commented out may prevent proper model synchronization. Please restore:
- Core identity models:
AddressName,Realm,Owner- Game progression models:
Hyperstructure,Epoch,Season,Contribution- Configuration models:
HyperstructureResourceConfig,HyperstructureConfig,TickConfig- Social models:
GuildMember,EntityName,Structure🔗 Analysis chain
Line range hint
18-33: Verify if filter configuration is intentionally minimal.Most model and event filters are commented out, leaving only "CapacityConfig" active. This seems unusually restrictive.
Let's check if other parts of the codebase reference these models:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to the commented out models for model in "AddressName" "Realm" "Owner" "Hyperstructure" "Epoch" "Season" "Contribution" "HyperstructureResourceConfig" "HyperstructureConfig" "TickConfig" "GuildMember" "EntityName" "Structure"; do echo "Checking references to $model..." rg -l "$model" --type ts doneLength of output: 37220
Remove commented code as it's obsolete
The commented code in
landing/src/dojo/setup.tscan be safely removed as it has been replaced by newer implementations:
- The game app uses
syncEntitiesDebounced- The landing app uses
syncEntitiesEternum🔗 Analysis chain
Line range hint
58-89: Clean up or document commented code sections.Large sections of entity fetching and syncing code are commented out. If these are temporarily disabled for testing, consider adding a TODO comment explaining when they should be re-enabled.
Let's check if these functions are used elsewhere:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if getEntities and syncEntities are used in other files rg -l "getEntities|syncEntities" --type tsLength of output: 412
Script:
#!/bin/bash # Check the actual usage context of these functions in the files rg "getEntities|syncEntities" --type ts -C 2Length of output: 12459
client/apps/landing/src/components/ui/utils/utils.ts (2)
Line range hint
96-108: Fix Inconsistent Return Object Keys incalculateOffsetFunctionThe
calculateOffsetfunction returns{ x: 0, y: 0 }whentotal === 1and{ x: ..., z: ... }otherwise. This inconsistency in the object keys (yvsz) may lead to bugs when consuming this function.Apply this diff to fix the inconsistency:
export const calculateOffset = (index: number, total: number, radius: number) => { if (total === 1) { - return { x: 0, y: 0 }; + return { x: 0, z: 0 }; } const angleIncrement = (2 * Math.PI) / 6; // Maximum 6 points on the circumference for the first layer let angle = angleIncrement * (index % 6); let offsetRadius = radius;
Line range hint
281-289: Add Error Handling ingetJSONFileFunctionThe
getJSONFilefunction does not handle errors that may occur during the fetch or JSON parsing operations. To prevent unhandled exceptions, consider adding error handling to gracefully handle failures.Apply this diff to enhance error handling:
export const getJSONFile = async (filePath: string) => { + try { const response = await fetch(filePath); + if (!response.ok) { + throw new Error(`Network response was not ok: ${response.statusText}`); + } const data = await response.json(); return data; + } catch (error) { + console.error("Error fetching JSON file:", error); + throw error; + } };This ensures that any network or parsing errors are caught and can be handled appropriately in the calling code.
client/apps/game/src/ui/components/military/ArmyManagementCard.tsx (1)
Line range hint
122-130: Await the asynchronousaddTroopsmethod and handle errors.The
armyManager.addTroopsmethod is asynchronous but is not being awaited. Additionally,isLoadingis set tofalseimmediately after the call, which could lead to UI inconsistencies if the operation hasn't completed or fails.Apply this diff to properly await the method and add error handling:
const handleBuyArmy = async () => { setIsLoading(true); - armyManager.addTroops(account, { + try { + await armyManager.addTroops(account, { [ResourcesIds.Knight]: multiplyByPrecision(troopCounts[ResourcesIds.Knight]), [ResourcesIds.Crossbowman]: multiplyByPrecision(troopCounts[ResourcesIds.Crossbowman]), [ResourcesIds.Paladin]: multiplyByPrecision(troopCounts[ResourcesIds.Paladin]), - }); + }); + + setTroopCounts({ + [ResourcesIds.Knight]: 0, + [ResourcesIds.Crossbowman]: 0, + [ResourcesIds.Paladin]: 0, + }); + } catch (e) { + console.error(e); + } finally { + setIsLoading(false); + } - - setIsLoading(false); };
🧹 Nitpick comments (18)
client/apps/landing/dojoConfig.ts (4)
2-4: Align manifest file names with their usageThere's a naming inconsistency between the file name and its imported variable name:
manifest_prod.jsonis imported assepoliaManifest- This mismatch could cause confusion for other developers
Consider renaming the file to match its usage:
-import sepoliaManifest from "../../common/manifests/manifest_prod.json"; +import sepoliaManifest from "../../common/manifests/manifest_sepolia.json";
Line range hint
19-20: Remove commented-out codeThese commented lines are no longer relevant as the
VITE_PUBLIC_DEVenvironment variable has been removed.-// const isLocal = VITE_PUBLIC_CHAIN === "local" || VITE_PUBLIC_CHAIN === "testnet"; -// const manifest = VITE_PUBLIC_DEV === true && isLocal ? devManifest : productionManifest;
Line range hint
22-28: Make the fallback chain selection more explicitThe current fallback logic using the nullish coalescing operator is implicit. Consider making it more explicit for better maintainability.
-const manifest = manifestMap[VITE_PUBLIC_CHAIN as keyof typeof manifestMap] ?? sepoliaManifest; +const DEFAULT_CHAIN = 'sepolia'; +const manifest = manifestMap[VITE_PUBLIC_CHAIN as keyof typeof manifestMap] || manifestMap[DEFAULT_CHAIN];
Line range hint
35-43: Extract default values to named constantsThe hardcoded default values for
accountClassHashandfeeTokenAddressshould be extracted to named constants for better maintainability.+const DEFAULT_ACCOUNT_CLASS_HASH = "0x07dc7899aa655b0aae51eadff6d801a58e97dd99cf4666ee59e704249e51adf2"; +const DEFAULT_FEE_TOKEN_ADDRESS = "0x49d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7"; export const dojoConfig = createDojoConfig({ rpcUrl: VITE_PUBLIC_NODE_URL, toriiUrl: VITE_PUBLIC_TORII, relayUrl: VITE_PUBLIC_TORII_RELAY, masterAddress: VITE_PUBLIC_MASTER_ADDRESS, masterPrivateKey: VITE_PUBLIC_MASTER_PRIVATE_KEY, - accountClassHash: VITE_PUBLIC_ACCOUNT_CLASS_HASH || "0x07dc7899aa655b0aae51eadff6d801a58e97dd99cf4666ee59e704249e51adf2", - feeTokenAddress: VITE_PUBLIC_FEE_TOKEN_ADDRESS || "0x49d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7", + accountClassHash: VITE_PUBLIC_ACCOUNT_CLASS_HASH || DEFAULT_ACCOUNT_CLASS_HASH, + feeTokenAddress: VITE_PUBLIC_FEE_TOKEN_ADDRESS || DEFAULT_FEE_TOKEN_ADDRESS, manifest, });client/apps/game/src/ui/components/worldmap/armies/ArmyWarning.tsx (1)
16-18: Consider subscribing to tick updates instead of accessing store state directly.The direct access to store state using
useUIStore.getState()withinuseMemodependencies might miss updates. Consider using the store's subscribe mechanism or moving the state access outside the memoized calculations.Example refactor:
- const food = armyManager.getFood(useUIStore.getState().currentDefaultTick); + const currentDefaultTick = useUIStore(state => state.currentDefaultTick); + const food = armyManager.getFood(currentDefaultTick); // Later in the code... - const staminaManager = new StaminaManager(dojo.setup.components, army.entity_id); - return staminaManager.getStamina(useUIStore.getState().currentArmiesTick); + const currentArmiesTick = useUIStore(state => state.currentArmiesTick); + const staminaManager = new StaminaManager(dojo.setup.components, army.entity_id); + return staminaManager.getStamina(currentArmiesTick);Also applies to: 30-32
client/apps/landing/src/dojo/setup.ts (4)
Line range hint
13-16: Consider documenting expected config parameters.While spreading config is flexible, explicit documentation of expected parameters would improve maintainability.
Add JSDoc to clarify expected config parameters:
+/** + * Sets up the game client with network, components, and system calls + * @param config - Configuration object containing network settings + * @returns Setup result including network, components, system calls, and event sync + */ export async function setup({ ...config }: DojoConfig) {
Line range hint
35-57: Consider validating undefined keys.The clauses use undefined keys which might need validation to prevent runtime issues.
Consider adding validation:
const clauses: Clause[] = [ { Keys: { - keys: [undefined], + keys: [undefined].filter((key): key is string => key !== undefined), pattern_matching: "FixedLen", models: [], }, }, // ... other clauses ];
Line range hint
91-103: Document the boolean flags in getEvents call.The purpose of the two false flags in the getEvents call is unclear.
Consider adding comments or using named parameters:
const eventSync = getEvents( network.toriiClient, network.contractComponents.events as any, undefined, { Keys: { keys: [undefined], pattern_matching: "VariableLen", models: filteredEvents.map((event) => `s0_eternum-${event}`), }, }, - false, - false + false, // shouldSync + false // shouldStartFromLatest );
Line range hint
107-113: Maintain consistency in return statement.The commented out sync property in the return statement should be removed if it's no longer needed.
return { network, components, systemCalls, - //sync, eventSync, };client/apps/game/src/ui/components/worldmap/structures/StructureListItem.tsx (1)
Line range hint
89-157: Consider extracting duplicate Sword component logic.There's duplicate code for the Sword component with similar props and click handlers. Consider extracting this into a reusable component to improve maintainability.
Here's a suggested refactor:
const BattleSwordButton = ({ isImmune, onClick, className }: { isImmune: boolean; onClick: () => void; className?: string; }) => ( <Sword className={clsx( "fill-gold h-6 w-6 my-auto animate-slow transition-all hover:fill-gold/50 hover:scale-125", { "opacity-50": isImmune }, className )} onClick={() => { if (!isImmune) { onClick(); } }} onMouseEnter={() => { if (isImmune) { setTooltip({ content: immuneTooltipContent, position: "top", }); } }} onMouseLeave={() => setTooltip(null)} /> );Then use it in both places:
// For the first usage const swordButton = ( <BattleSwordButton key="sword-2" isImmune={isImmune} onClick={() => setBattleView({ battleEntityId: updatedBattle?.entity_id, targetArmy: structure.protector?.entity_id, ownArmyEntityId: ownArmySelected?.entity_id, }) } /> ); // For the second usage return [ <BattleSwordButton key="sword-2" isImmune={isImmune} onClick={() => setBattleView({ engage: true, battleEntityId: undefined, ownArmyEntityId: ownArmySelected?.entity_id, targetArmy: structure.protector?.entity_id, }) } /> ];client/apps/game/src/ui/modules/military/battle-view/BattleView.tsx (2)
Line range hint
156-168: Consider safer type handling for the structure prop.While casting
structure as Structureensures type compatibility, it might mask potential runtime issues ifstructureis undefined. Consider using a more explicit approach:- structure={structure as Structure} + structure={structure || undefined}This would make it clearer to the Battle component when it needs to handle an undefined structure.
Line range hint
22-124: Consider extracting battle state logic into custom hooks.The component contains complex battle state management with multiple memoized computations. Consider extracting these into custom hooks (e.g.,
useBattleState,useBattleHealth) to:
- Improve code organization
- Enhance testability
- Reduce component complexity
For example:
// hooks/useBattleState.ts export function useBattleState(battleManager: BattleManager, armies: Army[]) { const attackerArmies = useMemo(...) const defenderArmies = useMemo(...) return { attackerArmies, defenderArmies } } // hooks/useBattleHealth.ts export function useBattleHealth(battleAdjusted: Battle, armies: Army[]) { const attackerHealth = useMemo(...) const defenderHealth = useMemo(...) return { attackerHealth, defenderHealth } }client/apps/landing/src/components/ui/utils/utils.ts (1)
Line range hint
152-160: Replacealertwith User-Friendly Notification MethodUsing
alert()for user notifications can be disruptive to the user experience. Consider replacingalert()with a non-blocking notification method, such as a toast or snackbar, to provide a smoother interaction.Apply this diff to update the notification method:
export const copyPlayerAddressToClipboard = (address: ContractAddress, name: string) => { navigator.clipboard .writeText(address.toString()) .then(() => { - alert(`Address of ${name} copied to clipboard`); + // Replace alert with a non-blocking notification + showToast(`Address of ${name} copied to clipboard`); }) .catch((err) => { console.error("Failed to copy: ", err); }); };Ensure
showToastis defined and implemented in your UI components.client/apps/game/src/ui/modules/military/battle-view/BattleActions.tsx (1)
199-199: Encapsulate battle deletion withinBattleManagerDeleting the battle entity directly via
world.deleteEntitymay bypass important cleanup logic encapsulated withinBattleManager. Consider adding adeleteBattle()method toBattleManagerto handle the deletion process and ensure all necessary cleanups are performed.client/sdk/packages/eternum/src/dojo/components/contractComponents.ts (1)
1-2213: Consider optimizing component definitions in code generatorAs this is an autogenerated file, any manual changes will be overwritten. If the unnecessary Immediately Invoked Function Expressions (IIFEs) in component definitions are not required, consider updating the code generator to produce more streamlined code without them. This will simplify the code and improve readability.
client/apps/game/src/ui/modules/entity-details/EnemyArmies.tsx (1)
Line range hint
76-81: Consider extracting battle state checkThe battle state check logic could be extracted into a separate method for better readability and reusability.
+const isBattleInactive = (battleManager: BattleManager, army: ArmyInfo, timestamp: number) => { + return ( + battleManager.isBattleOngoing(timestamp) || + battleManager.getUpdatedArmy(army, battleManager.getUpdatedBattle(timestamp))!.health.current <= 0 + ); +}; const getArmyChip = useCallback( (army: ArmyInfo, index: number) => { // ... const battleManager = new BattleManager(dojo.setup.components, dojo.network.provider, army.battle_id); - if ( - battleManager.isBattleOngoing(nextBlockTimestamp!) || - battleManager.getUpdatedArmy(army, battleManager.getUpdatedBattle(nextBlockTimestamp!))!.health.current <= 0 - ) { + if (isBattleInactive(battleManager, army, nextBlockTimestamp!)) { return null; } // ... }, // ... );client/apps/game/src/ui/components/list/EntityList.tsx (1)
Line range hint
19-19: Consider adding JSDoc for the new propThe new
filterEntityIdsprop would benefit from JSDoc documentation explaining its purpose and usage.+/** + * Optional array of entity IDs to filter the list. + * If provided, only entities with matching IDs will be displayed. + */ filterEntityIds?: ID[];client/apps/game/src/ui/components/bank/AddLiquidity.tsx (1)
42-42: Consider optimizing useMemo dependencies.The current implementation correctly uses
setup.components, but consider destructuringsetup.componentsin the dependencies array to avoid unnecessary re-renders when other properties ofsetupchange.const marketManager = useMemo( () => new MarketManager(setup.components, bankEntityId, ContractAddress(account.address), resourceId), - [setup, bankEntityId, resourceId, account.address], + [setup.components, bankEntityId, resourceId, account.address], );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (82)
client/apps/game/src/dojo/createSystemCalls.ts(1 hunks)client/apps/game/src/dojo/modelManager/utils/ArmyMovementUtils.ts(0 hunks)client/apps/game/src/dojo/setup.ts(1 hunks)client/apps/game/src/hooks/helpers/battles/useBattles.tsx(2 hunks)client/apps/game/src/hooks/helpers/useArmies.tsx(1 hunks)client/apps/game/src/hooks/helpers/useHyperstructures.tsx(1 hunks)client/apps/game/src/hooks/helpers/useQuests.tsx(2 hunks)client/apps/game/src/hooks/helpers/useRealm.tsx(1 hunks)client/apps/game/src/hooks/helpers/useResources.tsx(8 hunks)client/apps/game/src/hooks/helpers/useStamina.tsx(2 hunks)client/apps/game/src/hooks/helpers/useStructures.tsx(2 hunks)client/apps/game/src/hooks/store/_buildModeStore.tsx(1 hunks)client/apps/game/src/hooks/store/_threeStore.tsx(1 hunks)client/apps/game/src/hooks/store/useLeaderBoardStore.tsx(2 hunks)client/apps/game/src/three/components/HighlightHexManager.ts(1 hunks)client/apps/game/src/three/components/Navigator.ts(1 hunks)client/apps/game/src/three/helpers/pathfinding.ts(1 hunks)client/apps/game/src/three/scenes/HexagonScene.ts(1 hunks)client/apps/game/src/three/scenes/Hexception.tsx(6 hunks)client/apps/game/src/three/scenes/Worldmap.ts(6 hunks)client/apps/game/src/three/scenes/constants.ts(0 hunks)client/apps/game/src/three/systems/SystemManager.ts(1 hunks)client/apps/game/src/three/systems/types.ts(1 hunks)client/apps/game/src/types/index.ts(0 hunks)client/apps/game/src/ui/components/bank/AddLiquidity.tsx(2 hunks)client/apps/game/src/ui/components/bank/LiquidityResourceRow.tsx(3 hunks)client/apps/game/src/ui/components/bank/Swap.tsx(2 hunks)client/apps/game/src/ui/components/battles/BattleListItem.tsx(2 hunks)client/apps/game/src/ui/components/hyperstructures/HyperstructureDetails.tsx(2 hunks)client/apps/game/src/ui/components/hyperstructures/HyperstructurePanel.tsx(2 hunks)client/apps/game/src/ui/components/hyperstructures/Leaderboard.tsx(2 hunks)client/apps/game/src/ui/components/hyperstructures/ResourceExchange.tsx(1 hunks)client/apps/game/src/ui/components/list/EntityList.tsx(1 hunks)client/apps/game/src/ui/components/military/ArmyChip.tsx(2 hunks)client/apps/game/src/ui/components/military/ArmyList.tsx(2 hunks)client/apps/game/src/ui/components/military/ArmyManagementCard.tsx(4 hunks)client/apps/game/src/ui/components/military/EntitiesArmyTable.tsx(2 hunks)client/apps/game/src/ui/components/quest/steps/pauseProductionSteps.tsx(1 hunks)client/apps/game/src/ui/components/resources/DepositResources.tsx(3 hunks)client/apps/game/src/ui/components/structures/worldmap/StructureCard.tsx(2 hunks)client/apps/game/src/ui/components/trading/MarketModal.tsx(2 hunks)client/apps/game/src/ui/components/trading/MarketOrderPanel.tsx(2 hunks)client/apps/game/src/ui/components/trading/MarketResourceSideBar.tsx(5 hunks)client/apps/game/src/ui/components/worldmap/armies/ActionInfo.tsx(1 hunks)client/apps/game/src/ui/components/worldmap/armies/ArmyInfoLabel.tsx(1 hunks)client/apps/game/src/ui/components/worldmap/armies/ArmyWarning.tsx(2 hunks)client/apps/game/src/ui/components/worldmap/battles/BattleLabel.tsx(2 hunks)client/apps/game/src/ui/components/worldmap/structures/StructureLabel.tsx(1 hunks)client/apps/game/src/ui/components/worldmap/structures/StructureListItem.tsx(2 hunks)client/apps/game/src/ui/elements/ArmyCapacity.tsx(1 hunks)client/apps/game/src/ui/modules/entity-details/Battles.tsx(1 hunks)client/apps/game/src/ui/modules/entity-details/BuildingEntityDetails.tsx(3 hunks)client/apps/game/src/ui/modules/entity-details/CombatEntityDetails.tsx(1 hunks)client/apps/game/src/ui/modules/entity-details/EnemyArmies.tsx(2 hunks)client/apps/game/src/ui/modules/entity-details/Entities.tsx(1 hunks)client/apps/game/src/ui/modules/entity-details/realm/Buildings.tsx(2 hunks)client/apps/game/src/ui/modules/entity-details/realm/Castle.tsx(1 hunks)client/apps/game/src/ui/modules/entity-details/realm/RealmDetails.tsx(1 hunks)client/apps/game/src/ui/modules/military/battle-view/Battle.tsx(1 hunks)client/apps/game/src/ui/modules/military/battle-view/BattleActions.tsx(5 hunks)client/apps/game/src/ui/modules/military/battle-view/BattleProgress.tsx(1 hunks)client/apps/game/src/ui/modules/military/battle-view/BattleSideView.tsx(1 hunks)client/apps/game/src/ui/modules/military/battle-view/BattleView.tsx(2 hunks)client/apps/game/src/ui/modules/military/battle-view/EntityAvatar.tsx(1 hunks)client/apps/game/src/ui/modules/military/battle-view/battle-twitter-share-button.tsx(1 hunks)client/apps/game/src/ui/modules/military/battle-view/utils.tsx(2 hunks)client/apps/game/src/ui/modules/world-structures/WorldStructuresMenu.tsx(2 hunks)client/apps/game/src/ui/utils/realms.tsx(1 hunks)client/apps/game/src/ui/utils/utils.tsx(1 hunks)client/apps/landing/dojoConfig.ts(1 hunks)client/apps/landing/src/components/ui/utils/utils.ts(1 hunks)client/apps/landing/src/dojo/modelManager/ConfigManager.ts(0 hunks)client/apps/landing/src/dojo/modelManager/ResourceManager.ts(0 hunks)client/apps/landing/src/dojo/modelManager/leaderboard/LeaderboardManager.ts(0 hunks)client/apps/landing/src/dojo/modelManager/leaderboard/LeaderboardUtils.ts(0 hunks)client/apps/landing/src/dojo/setup.ts(1 hunks)client/apps/landing/src/hooks/helpers/useRealms.tsx(1 hunks)client/apps/landing/tailwind.config.js(1 hunks)client/sdk/packages/eternum/package.json(1 hunks)client/sdk/packages/eternum/src/constants/buildings.ts(1 hunks)client/sdk/packages/eternum/src/dojo/components/contractComponents.ts(1 hunks)client/sdk/packages/eternum/src/dojo/components/createClientComponents.ts(1 hunks)
⛔ Files not processed due to max files limit (19)
- client/sdk/packages/eternum/src/index.ts
- client/sdk/packages/eternum/src/modelManager/ArmyManager.ts
- client/sdk/packages/eternum/src/modelManager/ArmyMovementManager.ts
- client/sdk/packages/eternum/src/modelManager/BattleManager.ts
- client/sdk/packages/eternum/src/modelManager/ConfigManager.ts
- client/sdk/packages/eternum/src/modelManager/LeaderboardManager.ts
- client/sdk/packages/eternum/src/modelManager/MarketManager.ts
- client/sdk/packages/eternum/src/modelManager/ResourceInventoryManager.ts
- client/sdk/packages/eternum/src/modelManager/ResourceManager.ts
- client/sdk/packages/eternum/src/modelManager/StaminaManager.ts
- client/sdk/packages/eternum/src/modelManager/TileManager.ts
- client/sdk/packages/eternum/src/modelManager/index.ts
- client/sdk/packages/eternum/src/modelManager/types/index.ts
- client/sdk/packages/eternum/src/modelManager/utils/ArmyMovementUtils.ts
- client/sdk/packages/eternum/src/modelManager/utils/LeaderboardUtils.ts
- client/sdk/packages/eternum/src/modelManager/utils/index.ts
- client/sdk/packages/eternum/src/types/common.ts
- client/sdk/packages/eternum/src/utils/battleSimulation.ts
- client/sdk/packages/eternum/src/utils/index.ts
💤 Files with no reviewable changes (7)
- client/apps/landing/src/dojo/modelManager/ResourceManager.ts
- client/apps/game/src/types/index.ts
- client/apps/game/src/three/scenes/constants.ts
- client/apps/game/src/dojo/modelManager/utils/ArmyMovementUtils.ts
- client/apps/landing/src/dojo/modelManager/ConfigManager.ts
- client/apps/landing/src/dojo/modelManager/leaderboard/LeaderboardUtils.ts
- client/apps/landing/src/dojo/modelManager/leaderboard/LeaderboardManager.ts
✅ Files skipped from review due to trivial changes (14)
- client/apps/game/src/three/systems/SystemManager.ts
- client/apps/game/src/ui/components/worldmap/armies/ActionInfo.tsx
- client/apps/game/src/hooks/helpers/useRealm.tsx
- client/apps/game/src/hooks/helpers/useHyperstructures.tsx
- client/apps/game/src/ui/utils/realms.tsx
- client/apps/game/src/ui/modules/military/battle-view/Battle.tsx
- client/apps/game/src/ui/components/hyperstructures/ResourceExchange.tsx
- client/apps/landing/src/hooks/helpers/useRealms.tsx
- client/sdk/packages/eternum/src/constants/buildings.ts
- client/apps/game/src/ui/modules/military/battle-view/battle-twitter-share-button.tsx
- client/apps/game/src/ui/modules/military/battle-view/BattleProgress.tsx
- client/apps/game/src/ui/modules/military/battle-view/EntityAvatar.tsx
- client/apps/game/src/ui/modules/entity-details/realm/RealmDetails.tsx
- client/apps/game/src/three/helpers/pathfinding.ts
🧰 Additional context used
📓 Learnings (1)
client/apps/game/src/ui/components/worldmap/structures/StructureLabel.tsx (1)
Learnt from: aymericdelab
PR: BibliothecaDAO/eternum#1818
File: client/src/ui/components/worldmap/structures/StructureLabel.tsx:47-0
Timestamp: 2024-11-12T03:28:50.421Z
Learning: In `StructureLabel.tsx`, when `timer` is negative, the immunity message is not shown, so handling negative `timer` values isn't necessary.
🔇 Additional comments (86)
client/apps/game/src/ui/components/quest/steps/pauseProductionSteps.tsx (1)
4-4: LGTM! Verify SDK constant value.The import change aligns with the SDK migration. However, let's verify that the constant is properly exported from the SDK with the expected value to ensure the tutorial flow remains intact.
✅ Verification successful
Import change verified successfully
The constant
BUILDINGS_CENTERis correctly defined in the SDK with the expected value of[10, 10], ensuring the tutorial positioning remains unchanged.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify BUILDINGS_CENTER constant exists in SDK with correct value # Expected: Should find the constant with value [10, 10] # Search for BUILDINGS_CENTER definition in SDK rg -A 1 "BUILDINGS_CENTER.*=.*\[10,\s*10\]" --type tsLength of output: 264
client/apps/game/src/ui/components/worldmap/armies/ArmyWarning.tsx (2)
5-5: LGTM! Good consolidation of imports.The consolidation of imports from
@bibliothecadao/eternumaligns well with the architectural refactoring goals, improving code organization and maintainability.
13-17: Verify consistent manager instantiation pattern across the codebase.The new pattern of using
dojo.setup.componentsanddojo.network.providerfor manager instantiation looks good. Let's verify this pattern is used consistently across other components.Also applies to: 30-32
✅ Verification successful
Manager instantiation patterns are correctly implemented
The patterns are consistent within their respective layers:
- UI components correctly use
dojo.setup.componentsanddojo.network.provider- SDK internal managers use scoped component references
- Three.js scene uses class-scoped
this.dojoreferences🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of ArmyMovementManager and StaminaManager instantiation # to verify consistent usage of the new pattern echo "Checking ArmyMovementManager instantiation pattern:" rg "new ArmyMovementManager\(" -A 2 echo -e "\nChecking StaminaManager instantiation pattern:" rg "new StaminaManager\(" -A 2Length of output: 2535
client/apps/landing/src/dojo/setup.ts (1)
1-1: LGTM! Clean import consolidation.The refactoring to use
@bibliothecadao/eternumpackage improves code organization. The singleton pattern forConfigManageris appropriate here.Also applies to: 10-11
client/apps/game/src/ui/components/worldmap/structures/StructureListItem.tsx (2)
5-5: LGTM! Well-organized imports.The imports are properly organized and aligned with the architectural changes to use the centralized
@bibliothecadao/eternummodule.Also applies to: 7-7, 10-10
54-57: Verify consistent BattleManager initialization across components.The updated initialization pattern looks correct. Let's verify that this pattern is consistently applied across other components.
✅ Verification successful
BattleManager initialization is correct and consistent.
The different initialization patterns are intentional as they refer to two separate BattleManager classes:
- SDK BattleManager (from @bibliothecadao/eternum) for battle logic
- Three.js BattleManager for 3D visualization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other BattleManager initializations to ensure consistency ast-grep --pattern 'new BattleManager($_)'Length of output: 150
Script:
#!/bin/bash # Find BattleManager class definitions ast-grep --pattern 'class BattleManager { $$$ }' # Find BattleManager imports rg "import.*BattleManager" -A 1Length of output: 60534
client/apps/game/src/ui/modules/military/battle-view/BattleView.tsx (2)
4-4: LGTM! Import consolidation looks good.The consolidation of imports from local paths to
@bibliothecadao/eternumaligns well with the architectural refactoring objectives.Also applies to: 7-7
30-34: Verify BattleManager initialization consistency across the codebase.The BattleManager initialization now requires additional dependencies. Let's verify this pattern is consistently applied across the codebase.
✅ Verification successful
BattleManager initialization is consistent across the codebase
The codebase has two separate BattleManager implementations:
- Game logic BattleManager in the SDK consistently uses (components, provider, battleEntityId)
- Three.js BattleManager for 3D visualization uses (scene) and is only used in Worldmap.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all BattleManager instantiations ast-grep --pattern 'new BattleManager($$$)' # Also search for any potential missed instances rg "new BattleManager" -A 3Length of output: 29332
Script:
#!/bin/bash # Find BattleManager class definitions fd "BattleManager.ts" --type f --exclude "*.test.ts" # Search for class definitions ast-grep --pattern 'class BattleManager { $$$ }'Length of output: 56472
client/apps/landing/src/components/ui/utils/utils.ts (1)
1-1: Import Statement UpdateThe import statement for
ClientConfigManagerhas been correctly updated to use the new module@bibliothecadao/eternum, aligning with the refactored architecture.client/apps/game/src/ui/components/worldmap/battles/BattleLabel.tsx (3)
4-4: Import statement is appropriate.The addition of
useStructureByPositionis correctly imported and is used within the component.
10-10: Imports from@bibliothecadao/eternumare correctly updated.The import of
BattleManagerandStructurefrom@bibliothecadao/eternumaligns with the refactoring and consolidates dependencies.
61-64: Initialization ofBattleManageris correct.The
BattleManageris instantiated with the updated parametersdojo.setup.components,dojo.network.provider, andbattleEntityId, which is consistent with the new constructor signature.client/apps/game/src/ui/modules/entity-details/BuildingEntityDetails.tsx (1)
10-10: Import statement updated appropriately.The import of
TileManagerand other entities from@bibliothecadao/eternumstreamlines dependencies and improves code organization.client/apps/game/src/ui/components/military/ArmyManagementCard.tsx (1)
27-27: Import statement updated correctly.The import of
ArmyInfoandArmyManagerfrom@bibliothecadao/eternumis appropriate and reflects the refactoring efforts.client/apps/game/src/hooks/helpers/useArmies.tsx (1)
3-7: Ensure compatibility with the newArmyInfotype definitionWith
ArmyInfonow imported from@bibliothecadao/eternum, please verify that all usages ofArmyInfoproperties in this file are compatible with the updated type definition.client/apps/game/src/ui/modules/entity-details/Battles.tsx (1)
3-3: LGTM! Import consolidation looks good.The change to import
ArmyInfofrom@bibliothecadao/eternumaligns with the broader refactoring effort to consolidate types into the SDK package.client/apps/game/src/hooks/helpers/useStamina.tsx (1)
1-1: LGTM! Manager instantiation updated correctly.The changes align with the broader refactoring pattern:
- Consolidated imports from
@bibliothecadao/eternum- Updated
StaminaManagerinstantiation to usesetup.componentsAlso applies to: 13-13
client/apps/game/src/three/systems/types.ts (1)
2-2: LGTM! Type imports consolidated correctly.The change to import
HexPosition,ID, andStructureTypefrom@bibliothecadao/eternumaligns with the type consolidation effort.client/apps/game/src/hooks/store/_buildModeStore.tsx (1)
1-1: LGTM! Constants and types consolidated correctly.The change to import
BUILDINGS_CENTER,BuildingType, andResourcesIdsfrom@bibliothecadao/eternumaligns with the effort to consolidate game constants and types into the SDK.client/sdk/packages/eternum/src/dojo/components/createClientComponents.ts (1)
4-4: LGTM! Well-structured component creation pattern.The implementation follows a clean pattern for creating overridable client components. The type definition using
ReturnTypeensures type safety.Let's verify that all components being wrapped exist in the contract components:
Also applies to: 6-22
✅ Verification successful
Implementation verified: Components are properly inherited from contracts
The client components correctly extend from contract components by spreading the entire
contractComponentsobject, ensuring all contract components are available. The type definition usingReturnTypemaintains type safety.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all components referenced in createClientComponents exist in contractComponents # Look for the ContractComponents interface definition ast-grep --pattern 'interface ContractComponents { $$$ }'Length of output: 61
Script:
#!/bin/bash # First, let's find where ContractComponents is defined rg "ContractComponents" -A 2 # Then look for component imports and definitions rg "import.*Component" client/sdk/packages/eternum/src/dojo/components/Length of output: 4431
client/apps/game/src/three/components/HighlightHexManager.ts (1)
3-3: LGTM! Import consolidation.The change aligns with the broader refactoring effort to consolidate types into the
@bibliothecadao/eternumpackage.client/apps/game/src/hooks/store/useLeaderBoardStore.tsx (2)
1-1: LGTM! Import consolidation.Consolidating imports from
@bibliothecadao/eternumimproves code organization.
35-35: Verify consistent LeaderboardManager instantiation across codebase.The change in LeaderboardManager instantiation to use
dojo.setup.componentssuggests an API change.Let's verify this change is consistent across all usages:
client/apps/game/src/ui/modules/entity-details/Entities.tsx (1)
2-2: LGTM! Clean import organization.Good separation of concerns by moving the type import to
@bibliothecadao/eternumwhile keeping the hook local.Also applies to: 7-7
client/apps/game/src/ui/components/hyperstructures/HyperstructureDetails.tsx (2)
3-3: LGTM! Import consolidation from SDK package.The import consolidation from
@bibliothecadao/eternumaligns with the broader refactoring effort.
12-14: Verify LeaderboardManager instantiation pattern across codebase.The instantiation pattern has been updated to use
dojo.setup.components. Let's verify this pattern is consistently applied across the codebase.✅ Verification successful
LeaderboardManager instantiation pattern is consistent across codebase
The pattern variations (
dojo.setup.components, destructuredcomponents, anddojofrom hook) are equivalent ways to access the same object, representing valid coding styles rather than inconsistencies.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for LeaderboardManager instantiations to verify consistent pattern rg -A 2 "LeaderboardManager\.instance\("Length of output: 2710
Script:
#!/bin/bash # Check the context of 'components' and 'dojo' variables in the inconsistent files rg -B 5 "const.*components.*=|const.*dojo.*=" client/apps/game/src/ui/modules/world-structures/WorldStructuresMenu.tsx client/apps/landing/src/hooks/use-get-all-players.tsxLength of output: 717
Script:
#!/bin/bash # Get more context about WorldStructuresMenu.tsx rg -l "WorldStructuresMenu" | xargs catLength of output: 22380
client/apps/game/src/ui/components/worldmap/structures/StructureLabel.tsx (1)
3-3: LGTM! Type imports consolidated from SDK package.The import consolidation from
@bibliothecadao/eternumaligns with the broader refactoring effort.Also applies to: 8-8
client/apps/game/src/ui/elements/ArmyCapacity.tsx (1)
4-4: Verify function signature compatibility.The
getArmyNumberOfTroopsfunction has been moved from local utils to the SDK package. Let's verify the function signature remains compatible.✅ Verification successful
Function signature is compatible ✅
The implementation in the SDK package maintains the same signature and behavior as expected by the consuming code in ArmyCapacity.tsx.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all usages of getArmyNumberOfTroops to verify compatibility ast-grep --pattern 'getArmyNumberOfTroops($args)'Length of output: 51
Script:
#!/bin/bash # Search for original implementation in local utils echo "=== Local Implementation ===" rg "getArmyNumberOfTroops" -A 5 -B 2 # Search for new implementation in node_modules echo -e "\n=== SDK Implementation ===" fd "eternum" --type f --exec rg "getArmyNumberOfTroops" -A 5 -B 2 {} \;Length of output: 3579
client/apps/game/src/ui/components/trading/MarketResourceSideBar.tsx (3)
2-2: LGTM! Import consolidation from SDK package.The import consolidation from
@bibliothecadao/eternumaligns with the broader refactoring effort.
23-23: LGTM! Updated MarketManager instantiation pattern.The instantiation pattern has been updated to use
dojo.setup.components, consistent with other manager classes.Also applies to: 35-37
62-71: LGTM! Proper dependency updates in useMemo.The useMemo dependencies have been correctly updated to include
dojo.setupinstead of the previoussetup.client/apps/game/src/ui/components/resources/DepositResources.tsx (4)
8-8: LGTM! Import consolidation looks good.The imports have been consolidated from separate paths into a single import from @bibliothecadao/eternum.
32-36: LGTM! BattleManager instantiation updated correctly.The BattleManager instantiation has been updated to use the new signature with components and provider parameters, which aligns with the model manager fixes.
48-48: LGTM! ResourceInventoryManager instantiation updated correctly.The ResourceInventoryManager instantiation follows the same pattern of using components and provider parameters.
55-55: LGTM! Account parameter added to onOffloadAll.The onOffloadAll method now includes the account parameter, improving account integration.
client/apps/game/src/three/components/Navigator.ts (1)
2-2: LGTM! Type import moved to SDK package.The HexPosition type import has been moved to @bibliothecadao/eternum, consistent with the centralization of types.
client/apps/game/src/hooks/store/_threeStore.tsx (2)
1-2: LGTM! Imports organized correctly.Types are properly imported from their respective sources, with game-specific types from local paths and shared types from @bibliothecadao/eternum.
Line range hint
71-72: LGTM! Building entity ID management added.New properties and methods for managing selected building entity IDs have been added, enhancing the store's capabilities.
client/apps/game/src/ui/components/hyperstructures/Leaderboard.tsx (2)
9-9: LGTM! LeaderboardManager import moved to SDK package.The LeaderboardManager and related types have been moved to @bibliothecadao/eternum.
33-36: LGTM! LeaderboardManager instance creation updated.The LeaderboardManager instance creation now uses dojo.setup.components, consistent with other manager updates in the codebase.
client/apps/landing/tailwind.config.js (1)
5-5: Verify the preset path exists.The preset path has been updated to point to a different location. Let's verify that the file exists at the new location.
✅ Verification successful
Preset path verification successful
The file exists at
client/default.jsand the relative path../../default.jscorrectly points to it from the tailwind config location.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the default.js file at the new location. fd --type f --full-path "default.js" | grep -v "node_modules"Length of output: 77
client/apps/game/src/ui/components/military/EntitiesArmyTable.tsx (1)
1-1: LGTM! Import consolidation looks good.The imports have been properly consolidated, moving
ArmyInfoto be imported from@bibliothecadao/eternumwhile maintaining the existing functionality.Also applies to: 10-11
client/apps/game/src/hooks/helpers/battles/useBattles.tsx (1)
2-2: LGTM! Import consolidation and manager instantiation look good.The changes properly:
- Consolidate imports from
@bibliothecadao/eternum.- Update the
BattleManagerinstantiation to use the correct parameters.Also applies to: 64-64
client/apps/game/src/ui/modules/entity-details/EnemyArmies.tsx (2)
9-9: LGTM! Import consolidationThe import statement has been updated to source
ArmyInfoandBattleManagerfrom@bibliothecadao/eternum, aligning with the SDK consolidation effort.
76-76: Verify BattleManager instantiationThe
BattleManagerconstructor now requires three parameters instead of one. While the change looks correct, ensure that all instances ofBattleManageracross the codebase have been updated to match this new signature.✅ Verification successful
✓ BattleManager instantiations are correctly updated
All production code instances of BattleManager have been properly updated to use the new three-parameter signature (components, provider, battle_id). The single instance in
Worldmap.tsusing a different signature appears to be a separate class serving a different purpose in the 3D scene rendering context.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining old BattleManager instantiations rg -A 2 "new BattleManager\(" --type ts --type tsxLength of output: 80
Script:
#!/bin/bash # Search for BattleManager instantiations using ast-grep ast-grep --pattern 'new BattleManager($_)' # Backup search using ripgrep without file type flags rg "new BattleManager" -g "*.{ts,tsx}"Length of output: 8601
client/apps/game/src/ui/components/list/EntityList.tsx (1)
2-2: LGTM! Import consolidationThe import statement has been updated to source
IDfrom@bibliothecadao/eternum, aligning with the SDK consolidation effort.client/apps/game/src/ui/components/worldmap/armies/ArmyInfoLabel.tsx (1)
4-6: LGTM! Import organizationThe imports have been consolidated and organized, improving code maintainability:
- Army-related imports from the SDK
- Structure-related hooks consolidated into a single line
Also applies to: 13-13
client/apps/game/src/ui/modules/entity-details/CombatEntityDetails.tsx (1)
2-4: LGTM! Import organizationThe imports have been properly organized, separating local hooks from SDK imports.
client/apps/game/src/ui/modules/military/battle-view/BattleSideView.tsx (1)
7-7: LGTM! Clean import consolidation.The consolidation of imports from
@bibliothecadao/eternumimproves code organization and maintainability.client/apps/game/src/ui/components/military/ArmyList.tsx (2)
9-9: LGTM! Clean import consolidation.The consolidation of imports from
@bibliothecadao/eternumimproves code organization.
26-29: LGTM! Improved dependency injection.The updated TileManager initialization with explicit dependencies (components and provider) is cleaner and more maintainable than passing the entire setup object.
client/apps/game/src/ui/components/battles/BattleListItem.tsx (2)
9-9: LGTM! Clean import consolidation.The consolidation of imports from
@bibliothecadao/eternumaligns with the codebase-wide refactoring.
34-37: LGTM! Improved dependency injection.The updated BattleManager initialization follows the new pattern of explicit dependency injection, making the code more maintainable.
client/apps/game/src/ui/components/bank/AddLiquidity.tsx (1)
8-8: LGTM! Clean import consolidation.The consolidation of imports from
@bibliothecadao/eternummaintains consistency with the rest of the codebase.client/apps/game/src/hooks/helpers/useResources.tsx (1)
39-39: Consistent update of ResourceManager instantiationThe changes consistently update all instances of
ResourceManagerinitialization to usesetup.componentsinstead ofsetup. This aligns with the broader refactoring effort to standardize manager instantiation across the codebase.Let's verify that all instances have been updated consistently:
Also applies to: 59-59, 86-86, 119-124, 133-133, 139-139, 158-158, 182-182
✅ Verification successful
ResourceManager instantiation is consistently updated across the codebase
All instances of ResourceManager initialization correctly use the components property, either as
setup.components,dojo.setup.components, orthis.componentsin class contexts. No inconsistencies found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining instances of ResourceManager instantiation that might have been missed # Test: Search for ResourceManager instantiation patterns. Expect: All instances should use setup.components # Look for any ResourceManager instantiations rg "new ResourceManager\(" # Look for specific imports of ResourceManager rg "import.*ResourceManager.*from"Length of output: 2659
client/apps/game/src/ui/modules/military/battle-view/utils.tsx (1)
4-11: Verify battle simulation accuracy with HealthSimulatorThe change from
HealthtoHealthSimulatorappears to be part of the refactoring to use the SDK's implementation. While the change is straightforward, we should ensure that battle calculations remain accurate.Let's verify the integration:
Also applies to: 102-103
✅ Verification successful
HealthSimulator integration verified successfully
The codebase shows consistent usage of HealthSimulator from the SDK, with proper health calculations maintained for both attackers and defenders in battle simulations. No traces of the old implementation remain.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old Health implementation # Test: Search for Health-related imports and usage. Expect: All instances should use HealthSimulator # Look for any remaining Health imports rg "import.*Health.*from" --type ts --type tsx # Look for Health instantiations rg "new Health\(" --type ts --type tsxLength of output: 145
Script:
#!/bin/bash # Search with correct file types and broader patterns rg "Health" --type typescript --type tsx -A 2 # Look for battle calculation patterns rg "battle.*simulation|simulation.*battle" --type typescript --type tsx -A 2 # Use ast-grep to find Health-related class/type usage ast-grep --pattern 'class $_ extends Health' ast-grep --pattern 'new HealthSimulator($$$)'Length of output: 1626
client/apps/game/src/dojo/setup.ts (1)
4-8: Verify ClientConfigManager compatibilityThe import change aligns with the centralization of managers to the SDK. The configuration logic remains unchanged, maintaining backward compatibility.
Let's verify the integration:
✅ Verification successful
ClientConfigManager integration verified successfully
The centralization of ClientConfigManager to the SDK is complete and consistent across the codebase. All imports and usage patterns are compatible with the SDK version.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old ConfigManager # Test: Search for ConfigManager imports and usage. Expect: All instances should use the SDK version # Look for any remaining local ConfigManager imports rg "import.*ConfigManager.*from.*modelManager" --type ts --type tsx # Look for ConfigManager usage patterns rg "ConfigManager\." --type ts --type tsxLength of output: 169
Script:
#!/bin/bash # Search for any remaining imports from old location and current usage patterns rg "import.*ClientConfigManager.*from" --type typescript rg "ClientConfigManager\." --type typescript -A 2 # Search for any references to the old path rg "modelManager.*ClientConfigManager" --type typescript # Look for configuration-related patterns rg "new ClientConfigManager\(" --type typescript -A 2Length of output: 365
Script:
#!/bin/bash # Search for ConfigManager imports rg "import.*\{.*ClientConfigManager.*\}.*from" "\.ts$" rg "import.*\{.*ConfigManager.*\}.*from.*modelManager" "\.ts$" # Look for usage patterns rg "ClientConfigManager\." "\.ts$" -A 2 rg "new ClientConfigManager\(" "\.ts$" -A 2 # Search for any old references rg "modelManager.*ConfigManager" "\.ts$"Length of output: 482
Script:
#!/bin/bash # Search for ConfigManager imports and usage rg -g "*.ts" "import.*ClientConfigManager.*from" rg -g "*.ts" "import.*ConfigManager.*from.*modelManager" rg -g "*.ts" "ClientConfigManager\." -A 2 rg -g "*.ts" "new ClientConfigManager"Length of output: 2860
client/apps/game/src/ui/modules/entity-details/realm/Buildings.tsx (1)
10-10: LGTM: Import consolidationThe import from
@bibliothecadao/eternumcorrectly includes all required types and managers.client/apps/game/src/dojo/createSystemCalls.ts (1)
3-3: LGTM! The function signature is now more explicit.The updated parameter type makes the function's requirements clearer and more maintainable.
client/apps/game/src/hooks/helpers/useStructures.tsx (1)
250-254: LGTM! The BattleManager instantiation is properly updated.The changes align with the new SDK package's requirements, providing better type safety and maintainability.
client/apps/game/src/hooks/helpers/useQuests.tsx (2)
2-2: LGTM! Imports are properly consolidated.The imports are now centralized from the SDK package, improving maintainability.
84-87: LGTM! The TileManager instantiation is properly updated.The changes align with the new SDK package's requirements, providing better type safety and maintainability.
client/apps/game/src/ui/components/military/ArmyChip.tsx (2)
14-14: LGTM! Imports are properly consolidated.The imports are now centralized from the SDK package, improving maintainability.
78-81: LGTM! The BattleManager instantiation is properly updated.The changes align with the new SDK package's requirements, providing better type safety and maintainability.
client/apps/game/src/ui/modules/world-structures/WorldStructuresMenu.tsx (1)
19-26: LGTM! Consolidated imports enhance code organization.The consolidation of imports from
@bibliothecadao/eternumimproves code organization and maintainability.client/apps/game/src/ui/components/hyperstructures/HyperstructurePanel.tsx (2)
17-23: LGTM! Consistent import consolidation.The imports have been properly consolidated from
@bibliothecadao/eternum, maintaining consistency with other files.
147-150: LGTM! Consistent manager instantiation pattern.The LeaderboardManager instantiation follows the standardized pattern using
setup.components.client/apps/game/src/ui/components/bank/LiquidityResourceRow.tsx (2)
8-8: LGTM! Consistent import pattern.The MarketManager import has been properly moved to
@bibliothecadao/eternum.
51-51: LGTM! Consistent manager instantiation pattern.The MarketManager instantiations follow the standardized pattern using
setup.components.Let's verify this pattern is consistently applied for MarketManager:
Also applies to: 354-354
✅ Verification successful
Verification successful: MarketManager instantiation pattern is consistent
All instances of MarketManager consistently use the setup.components pattern, with expected variations in variable naming (dojo.setup, dojoContext.setup, setup.components).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for inconsistent MarketManager instantiations # Expect: All instantiations should use setup.components rg -A 2 "new MarketManager\("Length of output: 1587
client/apps/game/src/ui/components/bank/Swap.tsx (2)
16-16: LGTM! Consistent import pattern.The MarketManager import has been properly moved to
@bibliothecadao/eternum.
62-62: LGTM! Consistent manager instantiation pattern.The MarketManager instantiation follows the standardized pattern using
setup.components.client/apps/game/src/ui/components/trading/MarketModal.tsx (2)
25-25: LGTM! Import consolidation looks good.The import statement has been updated to use the centralized
@bibliothecadao/eternummodule, which aligns with the broader refactoring effort to improve code organization.
74-77: Verify the dependency array in useMemo.The
dojoobject has been removed from the dependency array while still being used in the memoized function. This could lead to stale values ifdojochanges.Consider adding the required properties to the dependency array:
const battleManager = useMemo( () => new BattleManager(dojo.setup.components, dojo.network.provider, battle?.entity_id || 0), - [battle?.entity_id], + [battle?.entity_id, dojo.setup.components, dojo.network.provider], );client/apps/game/src/ui/utils/utils.tsx (2)
2-16: LGTM! Import consolidation looks good.The imports have been updated to use the centralized
@bibliothecadao/eternummodule, improving code organization.
2-16: Verify the impact of removed troop food cost calculations.The removal of
computeTravelFoodCostsandcomputeExploreFoodCostsfunctions suggests a significant change in game mechanics.Run the following script to check for any remaining references to these functions:
✅ Verification successful
Troop food cost calculations are preserved and properly refactored
The functions were not removed but rather moved to
@bibliothecadao/eternumSDK (ArmyMovementUtils.ts), where they are properly implemented and still being used by the UI components. This is a positive architectural change that centralizes these calculations in the SDK.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the removed functions rg -l "computeTravelFoodCosts|computeExploreFoodCosts"Length of output: 329
Script:
#!/bin/bash # Check the actual implementation and usage echo "=== Checking ArmyMovementUtils.ts ===" rg "computeTravelFoodCosts|computeExploreFoodCosts" client/sdk/packages/eternum/src/modelManager/utils/ArmyMovementUtils.ts -A 5 echo -e "\n=== Checking UI Components ===" rg "computeTravelFoodCosts|computeExploreFoodCosts" client/apps/game/src/ui/components/worldmap/armies/*.tsx -A 5Length of output: 4348
client/apps/game/src/three/scenes/HexagonScene.ts (1)
8-8: LGTM! Type import consolidation looks good.The
HexPositiontype import has been updated to use the centralized@bibliothecadao/eternummodule, which aligns with the type consolidation effort.client/apps/game/src/ui/components/structures/worldmap/StructureCard.tsx (1)
18-18: LGTM! Type import consolidation looks good.The import statement has been updated to use the centralized
@bibliothecadao/eternummodule for types likeArmyInfo, improving code organization.client/apps/game/src/ui/components/trading/MarketOrderPanel.tsx (1)
258-258: LGTM! The ResourceManager initialization has been updated correctly.The change aligns with the broader refactoring effort to pass
dojo.setup.componentsinstead ofdojo.setupwhen initializing managers.client/apps/game/src/three/scenes/Hexception.tsx (4)
149-149: LGTM! The TileManager initialization has been updated correctly.The change aligns with the broader refactoring effort to pass
dojo.setup.componentsanddojo.network.providerinstead ofdojo.setupwhen initializing managers.
313-318: LGTM! The placeBuilding call has been updated to include account information.The change correctly includes the account from
useAccountStoreas the first parameter, which is necessary for authorization.
414-414: LGTM! The getRealmLevel call has been updated to include structureEntityId.The change correctly passes
this.state.structureEntityIdto determine the realm level for the specific structure.
481-481: Verify the structureEntityId usage.The code uses
this.state.structureEntityIdto check for wonders, but this might be undefined if the state hasn't been initialized.Run the following script to verify the state initialization:
client/apps/game/src/three/scenes/Worldmap.ts (4)
91-91: LGTM! The TileManager initialization has been updated correctly.The change aligns with the broader refactoring effort to pass
dojo.setup.componentsanddojo.network.providerinstead ofdojo.setupwhen initializing managers.
306-318: LGTM! The placeStructure call has been updated to include account information.The change correctly includes the account from
useAccountStoreas the first parameter, which is necessary for authorization. The error handling has also been properly implemented to clean up the structure on failure.
351-355: LGTM! The ArmyMovementManager initialization and usage have been updated correctly.The changes properly:
- Initialize
ArmyMovementManagerwithdojo.componentsanddojo.network.provider- Include account information in the
moveArmycall- Pass necessary parameters for movement calculation
Also applies to: 357-363
376-380: LGTM! The ArmyMovementManager initialization has been updated correctly.The change aligns with the previous instance of
ArmyMovementManagerinitialization.
| const tileManager = new TileManager(dojo.setup.components, dojo.network.provider, { | ||
| col: selectedBuildingHex.outerCol, | ||
| row: selectedBuildingHex.outerRow, | ||
| }); | ||
| tileManager.destroyBuilding(selectedBuildingHex.innerCol, selectedBuildingHex.innerRow); | ||
| tileManager.destroyBuilding(dojo.account.account, selectedBuildingHex.innerCol, selectedBuildingHex.innerRow); |
There was a problem hiding this comment.
Ensure error handling for destroyBuilding method.
The destroyBuilding method is likely asynchronous and should be handled appropriately to catch errors and confirm completion. Currently, there's no handling of the returned promise.
Apply this diff to handle the promise:
tileManager.destroyBuilding(dojo.account.account, selectedBuildingHex.innerCol, selectedBuildingHex.innerRow);
+ .then(() => {
+ // Success handling if needed
+ })
+ .catch((error) => {
+ console.error(error);
+ // Error handling if needed
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const tileManager = new TileManager(dojo.setup.components, dojo.network.provider, { | |
| col: selectedBuildingHex.outerCol, | |
| row: selectedBuildingHex.outerRow, | |
| }); | |
| tileManager.destroyBuilding(selectedBuildingHex.innerCol, selectedBuildingHex.innerRow); | |
| tileManager.destroyBuilding(dojo.account.account, selectedBuildingHex.innerCol, selectedBuildingHex.innerRow); | |
| const tileManager = new TileManager(dojo.setup.components, dojo.network.provider, { | |
| col: selectedBuildingHex.outerCol, | |
| row: selectedBuildingHex.outerRow, | |
| }); | |
| tileManager.destroyBuilding(dojo.account.account, selectedBuildingHex.innerCol, selectedBuildingHex.innerRow) | |
| .then(() => { | |
| // Success handling if needed | |
| }) | |
| .catch((error) => { | |
| console.error(error); | |
| // Error handling if needed | |
| }); |
| const tileManager = new TileManager(dojo.setup.components, dojo.network.provider, { | ||
| col: selectedBuildingHex.outerCol, | ||
| row: selectedBuildingHex.outerRow, | ||
| }); | ||
| const action = !isPaused ? tileManager.pauseProduction : tileManager.resumeProduction; | ||
| action(selectedBuildingHex.innerCol, selectedBuildingHex.innerRow).then(() => { | ||
| action(dojo.account.account, selectedBuildingHex.innerCol, selectedBuildingHex.innerRow).then(() => { |
There was a problem hiding this comment.
Add error handling for the asynchronous action.
The call to action() returns a promise, but there's no error handling in case the promise is rejected. This could lead to unhandled promise rejections and isLoading might remain true indefinitely if an error occurs.
Apply this diff to add error handling:
action(dojo.account.account, selectedBuildingHex.innerCol, selectedBuildingHex.innerRow)
.then(() => {
setIsLoading(false);
})
+ .catch((error) => {
+ console.error(error);
+ setIsLoading(false);
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const tileManager = new TileManager(dojo.setup.components, dojo.network.provider, { | |
| col: selectedBuildingHex.outerCol, | |
| row: selectedBuildingHex.outerRow, | |
| }); | |
| const action = !isPaused ? tileManager.pauseProduction : tileManager.resumeProduction; | |
| action(selectedBuildingHex.innerCol, selectedBuildingHex.innerRow).then(() => { | |
| action(dojo.account.account, selectedBuildingHex.innerCol, selectedBuildingHex.innerRow).then(() => { | |
| const tileManager = new TileManager(dojo.setup.components, dojo.network.provider, { | |
| col: selectedBuildingHex.outerCol, | |
| row: selectedBuildingHex.outerRow, | |
| }); | |
| const action = !isPaused ? tileManager.pauseProduction : tileManager.resumeProduction; | |
| action(dojo.account.account, selectedBuildingHex.innerCol, selectedBuildingHex.innerRow) | |
| .then(() => { | |
| setIsLoading(false); | |
| }) | |
| .catch((error) => { | |
| console.error(error); | |
| setIsLoading(false); | |
| }); |
| @@ -48,7 +47,7 @@ export const ArmyManagementCard = ({ owner_entity, army, setSelectedEntity }: Ar | |||
|
|
|||
| const maxTroopCountPerArmy = configManager.getTroopConfig().maxTroopCount; | |||
|
|
|||
| const armyManager = new ArmyManager(dojo, army?.entity_id || 0); | |||
| const armyManager = new ArmyManager(dojo.setup.network.provider, dojo.setup.components, army?.entity_id || 0); | |||
There was a problem hiding this comment.
Correct parameter order in ArmyManager instantiation.
The parameters in the ArmyManager constructor are in the wrong order. The correct order should be components, provider, and then entityId. Also, dojo.setup.network.provider should be dojo.network.provider to match the structure used elsewhere in the code.
Apply this diff to fix the parameter order and provider reference:
-const armyManager = new ArmyManager(dojo.setup.network.provider, dojo.setup.components, army?.entity_id || 0);
+const armyManager = new ArmyManager(dojo.setup.components, dojo.network.provider, army?.entity_id || 0);Committable suggestion skipped: line range outside the PR's diff.
| const battleManager = new BattleManager( | ||
| dojo.setup.components, | ||
| dojo.network.provider, | ||
| defender?.battle_id || battleAdjusted?.entity_id || 0, | ||
| ); |
There was a problem hiding this comment.
Avoid variable shadowing of battleManager
The variable battleManager is redeclared inside the defenderArmy useMemo, which shadows the battleManager prop passed to the BattleActions component. This can lead to confusion and potential bugs. Consider renaming the local variable or reusing the existing battleManager instance.
Apply this diff to fix the issue:
- const battleManager = new BattleManager(
+ const localBattleManager = new BattleManager(
dojo.setup.components,
dojo.network.provider,
defender?.battle_id || battleAdjusted?.entity_id || 0,
);And update references within this scope accordingly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const battleManager = new BattleManager( | |
| dojo.setup.components, | |
| dojo.network.provider, | |
| defender?.battle_id || battleAdjusted?.entity_id || 0, | |
| ); | |
| const localBattleManager = new BattleManager( | |
| dojo.setup.components, | |
| dojo.network.provider, | |
| defender?.battle_id || battleAdjusted?.entity_id || 0, | |
| ); |
| @@ -75,7 +75,7 @@ export const Castle = () => { | |||
| </p> | |||
| <div className="mb-2 font-semibold uppercase">Upgrade Cost to {RealmLevels[realm.level + 1]}</div> | |||
| <div className="flex gap-4"> | |||
| {configManager.realmUpgradeCosts[getNextRealmLevel]?.map((a) => ( | |||
| {configManager.realmUpgradeCosts[getNextRealmLevel]?.map((a: any) => ( | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace any type with a more specific type.
Using any type bypasses TypeScript's type checking. Consider defining an interface for the upgrade cost object.
-{configManager.realmUpgradeCosts[getNextRealmLevel]?.map((a: any) => (
+interface UpgradeCost {
+ resource: number;
+ amount: number;
+}
+{configManager.realmUpgradeCosts[getNextRealmLevel]?.map((a: UpgradeCost) => (Committable suggestion skipped: line range outside the PR's diff.
| const tileManager = new TileManager(dojo.setup.components, dojo.network.provider, { | ||
| col: structure.position!.x, | ||
| row: structure.position!.y, | ||
| }); | ||
|
|
||
| const action = paused ? tileManager.resumeProduction : tileManager.pauseProduction; | ||
| action(innerCol, innerRow).then(() => { | ||
| action(dojo.account.account, innerCol, innerRow).then(() => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for production management
While the TileManager instantiation has been updated correctly, the production pause/resume operations lack error handling.
Consider adding error handling:
const handlePauseResumeProduction = (paused: boolean, innerCol: number, innerRow: number) => {
setIsLoading({ isLoading: true, innerCol, innerRow });
const tileManager = new TileManager(dojo.setup.components, dojo.network.provider, {
col: structure.position!.x,
row: structure.position!.y,
});
const action = paused ? tileManager.resumeProduction : tileManager.pauseProduction;
- action(dojo.account.account, innerCol, innerRow).then(() => {
- setIsLoading({ isLoading: false, innerCol, innerRow });
- });
+ action(dojo.account.account, innerCol, innerRow)
+ .then(() => {
+ setIsLoading({ isLoading: false, innerCol, innerRow });
+ })
+ .catch((error) => {
+ console.error('Failed to update production:', error);
+ setIsLoading({ isLoading: false, innerCol, innerRow });
+ // Consider adding user notification here
+ });
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const tileManager = new TileManager(dojo.setup.components, dojo.network.provider, { | |
| col: structure.position!.x, | |
| row: structure.position!.y, | |
| }); | |
| const action = paused ? tileManager.resumeProduction : tileManager.pauseProduction; | |
| action(innerCol, innerRow).then(() => { | |
| action(dojo.account.account, innerCol, innerRow).then(() => { | |
| const tileManager = new TileManager(dojo.setup.components, dojo.network.provider, { | |
| col: structure.position!.x, | |
| row: structure.position!.y, | |
| }); | |
| const action = paused ? tileManager.resumeProduction : tileManager.pauseProduction; | |
| action(dojo.account.account, innerCol, innerRow) | |
| .then(() => { | |
| setIsLoading({ isLoading: false, innerCol, innerRow }); | |
| }) | |
| .catch((error) => { | |
| console.error('Failed to update production:', error); | |
| setIsLoading({ isLoading: false, innerCol, innerRow }); | |
| // Consider adding user notification here | |
| }); |
| setup: { components }, | ||
| account: { account }, | ||
| } = useDojo(); | ||
|
|
||
| const progress = useHyperstructureProgress(hyperstructureEntityId); | ||
| const latestChangeEvent = LeaderboardManager.instance(useDojo()).getCurrentCoOwners(hyperstructureEntityId); | ||
| const latestChangeEvent = LeaderboardManager.instance(components).getCurrentCoOwners(hyperstructureEntityId); | ||
| const needTosetCoOwners = !latestChangeEvent && progress.percentage === 100; | ||
| const shares = | ||
| LeaderboardManager.instance(useDojo()).getAddressShares(ContractAddress(account.address), hyperstructureEntityId) || | ||
| 0; | ||
| LeaderboardManager.instance(components).getAddressShares( | ||
| ContractAddress(account.address), | ||
| hyperstructureEntityId, | ||
| ) || 0; |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent LeaderboardManager instantiation patterns detected
The codebase shows inconsistent usage patterns:
client/apps/landing/src/hooks/use-get-all-players.tsx: Usesdojoinstead ofdojo.setup.componentsclient/apps/game/src/ui/modules/world-structures/WorldStructuresMenu.tsx: Uses directcomponentsinstead ofdojo.setup.components
All other instances correctly use dojo.setup.components. Consider standardizing the pattern across all files.
🔗 Analysis chain
Verify the LeaderboardManager instantiation pattern.
The instantiation pattern has been updated to use components from the setup object, which aligns with the new SDK's requirements.
Let's verify this pattern is consistently applied across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for inconsistent LeaderboardManager instantiations
# Expect: All instantiations should use setup.components
rg -A 2 "LeaderboardManager\.instance\("
Length of output: 2710
| "dependencies": { | ||
| "@dojoengine/core": "1.0.0-alpha.24", | ||
| "@latticexyz/utils": "^2.0.0-next.12", |
There was a problem hiding this comment.
💡 Codebase verification
Use stable version ^2.0.0 instead of pre-release version
A stable version 2.0.0 is available in the NPM registry. Since it's from the same major version, it should be API compatible and more reliable than the pre-release version.
🔗 Analysis chain
Verify the pre-release dependency version.
The package uses a pre-release version of @latticexyz/utils (^2.0.0-next.12). Consider:
- Using a stable version if available
- Documenting why a pre-release version is necessary
- Planning for an upgrade when a stable version is released
Run the following script to check for stable versions:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for stable versions of @latticexyz/utils
# Test: Check NPM registry for available versions
curl -s https://registry.npmjs.org/@latticexyz/utils | jq '.versions | keys[]' | grep -v "next\|alpha\|beta\|rc"
Length of output: 37759
Summary by CodeRabbit
Release Notes
Import Reorganization
@bibliothecadao/eternumManager Initialization Updates
componentsandnetwork.providerType Management
HexPosition,ArmyInfo, andStructureto centralized moduleUtility Function Changes
Configuration Management
Performance and Type Safety
These changes represent a significant refactoring effort to centralize and standardize the application's architecture, focusing on modularity and type safety.