refactor hooks #2642
refactor hooks #2642
Conversation
… refactor/clean
… raschel/clean
|
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. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Warning Rate limit exceeded@aymericdelab has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 44 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThis pull request introduces a comprehensive refactoring of the game's client-side hooks and components, focusing on centralizing data retrieval, simplifying utility functions, and improving the overall modularity of the codebase. The changes span multiple files and modules, with a primary emphasis on restructuring how entities, resources, and player data are accessed and managed. Changes
Sequence DiagramsequenceDiagram
participant Client as Game Client
participant Hooks as Custom Hooks
participant Utilities as Utility Functions
participant Components as Dojo Components
Client->>Hooks: Request player data
Hooks->>Components: Query components
Components-->>Hooks: Return component values
Hooks->>Utilities: Process data with utility functions
Utilities-->>Hooks: Return processed data
Hooks-->>Client: Provide refined data
Possibly Related PRs
Suggested Reviewers
Poem
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 (
|
|
Failed to generate code suggestions for PR |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🔭 Outside diff range comments (7)
client/apps/game/src/ui/components/military/army-management-card.tsx (1)
Line range hint
137-141: Add missing dependencies to useEffect.The useEffect hook uses
owner_entityanddojo.setup.componentsbut they're not included in the dependency array. This could lead to stale balance checks if these values change.Add the missing dependencies:
setCanCreate(canCreate); - }, [troopCounts]); + }, [troopCounts, owner_entity, dojo.setup.components]);client/apps/game/src/ui/modules/entity-details/realm/castle.tsx (1)
Line range hint
38-49: Correct the dependency array ofuseMemo.The
useMemohook relies ondojo.setup.componentsthrough thegetBalancefunction. Adddojo.setup.componentsto the dependency array to ensure the memoized value updates when components change.Apply this diff:
- }, [getBalance, structureEntityId]); + }, [structureEntityId, dojo.setup.components]);client/apps/game/src/ui/components/fragmentMines/fragment-mine-panel.tsx (1)
Line range hint
33-44: Improve error handling in name change logicThe current error handling only logs errors and doesn't inform the user. The loading state might also get stuck if an error occurs.
Consider this improved implementation:
try { await provider.set_entity_name({ signer: account, entity_id: entity.entity_id, name: naming }); } catch (e) { console.error(e); + // Show error to user + toast.error('Failed to change name. Please try again.'); + return; } finally { + setIsLoading(false); } - setIsLoading(false); setEditName(false);client/apps/game/src/ui/components/structures/construction/structure-construction-menu.tsx (1)
Line range hint
121-127: Optimize balance checks and remove hardcoded resource ID.The balance check is repeated for each resource, and the resource ID is hardcoded to AncientFragment instead of using the actual resource from the cost object.
- const balance = getBalance(entityId || 0, ResourcesIds.AncientFragment, dojo.setup.components); + const resourceId = cost[Number(resourceId)].resource_tier === ResourceTier.Lords + ? ResourcesIds.AncientFragment + : Number(resourceId); + const balance = getBalance(entityId || 0, resourceId, dojo.setup.components); return ( <ResourceCost key={index} type="horizontal" - resourceId={ResourcesIds.AncientFragment} + resourceId={resourceId} amount={cost[Number(resourceId)].min_amount * 1000} balance={balance.balance} />client/apps/game/src/hooks/helpers/use-resource-arrivals.tsx (1)
Line range hint
89-90: Add missing dependencies to createArrivalInfo useCallback.The callback depends on
hasMinWeightandPosition, but they're not included in the dependency array.const createArrivalInfo = useCallback( (id: Entity): ArrivalInfo | undefined => { // ... function body ... }, - [account, playerStructurePositions], + [account, playerStructurePositions, hasMinWeight, Position], );client/apps/game/src/ui/modules/rewards/rewards.tsx (1)
Line range hint
78-91: Remove duplicate setIsLoading(false) call.The
setIsLoading(false)is called twice in theclaimRewardsfunction - once in the finally block and once after it.const claimRewards = useCallback(async () => { setIsLoading(true); try { await claim_leaderboard_rewards({ signer: account, token: env.VITE_LORDS_ADDRESS!, }); } catch (error) { console.error("Error claiming rewards", error); } finally { setIsLoading(false); } - setIsLoading(false); }, [account]);client/apps/game/src/ui/components/resources/travel-info.tsx (1)
Line range hint
36-56: Update useEffect dependencies to include dojo.setup.components.The useEffect hook depends on
dojo.setup.componentsthrough thegetBalancecall, but it's not included in the dependency array. This could lead to stale data if components change.Apply this diff to fix the dependency array:
useEffect(() => { const totalWeight = getTotalResourceWeight(resources); const multipliedWeight = multiplyByPrecision(totalWeight); setResourceWeight(multipliedWeight); const { balance } = getBalance(entityId, ResourcesIds.Donkey, dojo.setup.components); const currentDonkeyAmount = isAmm ? 0 : resources.find((r) => r.resourceId === ResourcesIds.Donkey)?.amount || 0; const calculatedDonkeyBalance = divideByPrecision(balance) - currentDonkeyAmount; setDonkeyBalance(calculatedDonkeyBalance); const onlyDonkeysAndLords = resources.every( (r) => r.resourceId === ResourcesIds.Donkey || r.resourceId === ResourcesIds.Lords, ); if (setCanCarry) { onlyDonkeysAndLords ? setCanCarry(true) : setCanCarry(calculatedDonkeyBalance >= neededDonkeys); } - }, [resources, entityId, resourceWeight, donkeyBalance, setCanCarry]); + }, [resources, entityId, resourceWeight, donkeyBalance, setCanCarry, dojo.setup.components]);
🧹 Nitpick comments (29)
client/apps/game/src/ui/components/worldmap/armies/action-info.tsx (1)
97-105: Consider enhancing error handling for Army component.The current implementation silently falls back to default values when the Army component is not found. Consider logging a warning or handling the error more explicitly for better debugging.
const army = getComponentValue(components.Army, getEntityIdFromKeys([BigInt(selectedEntityId)])); + if (!army) { + console.warn(`Army component not found for entity ${selectedEntityId}`); + } return ( army?.troops || { knight_count: 0n, paladin_count: 0n, crossbowman_count: 0n, } );client/apps/game/src/ui/components/military/army-management-card.tsx (3)
250-250: Optimize balance calculations with useMemo.The balance calculations within the map function will be recalculated on every render. Consider memoizing the balances for better performance.
Here's how you could optimize it:
+ const troopBalances = useMemo(() => { + return troops.reduce((acc, troop) => ({ + ...acc, + [troop.name]: getBalance(owner_entity, troop.name, dojo.setup.components).balance + }), {}); + }, [owner_entity, troops, dojo.setup.components]); <div className="grid grid-cols-3 gap-3 my-4"> {troops.map((troop) => { - const balance = getBalance(owner_entity, troop.name, dojo.setup.components).balance; + const balance = troopBalances[troop.name];
Line range hint
89-102: Improve error handling for army operations.The error handling for
handleDeleteArmyonly logs to console. Consider adding user feedback for failed operations.Example improvement:
try { await armyManager.deleteArmy(account, army?.entity_id || 0); setSelectedEntity && setSelectedEntity(null); } catch (e) { console.error(e); + // Add user feedback + const errorMessage = e instanceof Error ? e.message : 'Failed to delete army'; + // Assuming you have a notification system + showNotification({ type: 'error', message: errorMessage }); }
Line range hint
1-289: Consider splitting the component for better maintainability.The component handles multiple responsibilities (army management, troop management, naming). Consider splitting it into smaller, focused components:
- ArmyHeader (name editing)
- TroopManagement (troop counts and balance)
- ArmyActions (delete/reinforce buttons)
This would improve maintainability and make the code easier to test.
client/apps/game/src/ui/components/worldmap/players/players-panel.tsx (2)
Line range hint
37-52: Consider extracting the player transformation logic.The player data transformation logic is complex and handles multiple concerns (structures, guild info, whitelist status). Consider extracting this into a separate utility function for better maintainability and testability.
+ const transformPlayerData = ( + player: PlayerInfo, + index: number, + components: any, + userGuild: any, + account: any + ): PlayerCustom => { + const structuresEntityIds = runQuery([ + Has(Structure), + HasValue(Owner, { address: ContractAddress(player.address) }), + ]); + + const structures = Array.from(structuresEntityIds) + .map((entityId) => { + const structure = getComponentValue(Structure, entityId); + if (!structure) return undefined; + return getEntityName(structure.entity_id, components); + }) + .filter((structure): structure is string => structure !== undefined); + + const guild = getGuildFromPlayerAddress(player.address); + const isInvited = userGuild + ? getComponentValue( + GuildWhitelist, + getEntityIdFromKeys([player.address, BigInt(userGuild.entityId)]) + )?.is_whitelisted ?? false + : false; + + return { + ...player, + structures, + isUser: player.address === ContractAddress(account.address), + points: player.points || 0, + rank: index + 1, + isInvited, + guild, + }; + }; const playersWithStructures: PlayerCustom[] = useMemo(() => { const sortedPlayers = [...players].sort((a, b) => (b.points || 0) - (a.points || 0)); - const playersWithStructures = sortedPlayers.map((player, index) => { - // ... existing transformation logic ... - }); + const playersWithStructures = sortedPlayers.map((player, index) => + transformPlayerData(player, index, components, userGuild, account) + ); return playersWithStructures; }, [isLoading, players]);
Line range hint
74-90: Consider optimizing search performance.The search functionality handles multiple criteria well, but consider these optimizations:
- Memoize individual search criteria results
- Consider debouncing the search input
- Add early return conditions for empty structures
+ const getSearchMatches = useMemo(() => { + const normalizedTerm = normalizeDiacriticalMarks(searchTerm.toLowerCase()); + return { + nameMatches: new Set( + playersWithStructures + .filter(player => + normalizeDiacriticalMarks(player.name.toLowerCase()).includes(normalizedTerm) + ) + .map(p => p.address) + ), + addressMatches: new Set( + playersWithStructures + .filter(player => + toHexString(player.address).toLowerCase().includes(normalizedTerm) + ) + .map(p => p.address) + ), + structureMatches: new Set( + playersWithStructures + .filter(player => + player.structures.some( + structure => structure && + normalizeDiacriticalMarks(structure.toLowerCase()).includes(normalizedTerm) + ) + ) + .map(p => p.address) + ) + }; + }, [playersWithStructures, searchTerm]); const filteredPlayers = useMemo(() => { if (searchTerm === "") return playersWithStructures; + const { nameMatches, addressMatches, structureMatches } = getSearchMatches; + return playersWithStructures.filter(player => + nameMatches.has(player.address) || + addressMatches.has(player.address) || + structureMatches.has(player.address) + ); - return playersWithStructures.filter((player) => { - // ... existing filtering logic ... - }); }, [playersWithStructures, searchTerm]);client/apps/game/src/hooks/helpers/use-resources.tsx (1)
Line range hint
10-15: Refine dependencies inuseMemohook to prevent unnecessary recomputationsIncluding
dojo.setupin the dependency array may causeresourceManagerto recompute more often than necessary ifdojo.setupis a complex object that changes on every render. Consider narrowing down the dependencies to specific properties or values that affect theresourceManager.Apply this diff to refine the dependencies:
const resourceManager = useMemo(() => { return new ResourceManager(dojo.setup.components, entityId, resourceId); - }, [dojo.setup, entityId, resourceId, production]); + }, [dojo.setup.components, entityId, resourceId]);If
dojo.setup.componentsis stable or memoized, this change will help prevent unnecessary recomputations.client/apps/game/src/hooks/helpers/use-realm.tsx (1)
10-10: Clarify variable naming for better readabilityDestructuring
account: { account }results inaccount.account, which can be confusing. Consider renaming the inneraccountto improve code clarity.Apply this diff to rename the variable:
const { - account: { account }, + account: { account: userAccount }, setup: { components }, } = useDojo();Update references to
account.addresswithuserAccount.addressin the code.client/apps/game/src/hooks/helpers/use-entities.tsx (3)
20-24: Includecomponentsin the dependency array ofuseMemo.The
useMemohook relies oncomponentsthrough thegetRealmfunction. To ensure the memoized value updates correctly whencomponentschange, addcomponentsto the dependency array.Apply this diff:
}, [entities]); + }, [entities, components]);
43-47: Includecomponentsin the dependency array ofuseMemo.Similarly, the
useMemohook depends oncomponentsvia thegetStructurefunction. Addcomponentsto the dependency array to prevent potential stale data whencomponentschange.Apply this diff:
}, [entities]); + }, [entities, components]);
36-40: Remove redundantHas(components.Owner)from the query.The
HasValue(components.Owner, ...)condition already ensures that entities have theOwnercomponent. IncludingHas(components.Owner)is redundant and can be removed to simplify the query.Apply this diff:
const entities = useEntityQuery([ Has(components.Structure), Has(components.Position), - Has(components.Owner), HasValue(components.Owner, { address: playerAddress || ContractAddress(account.address) }), ]);client/apps/game/src/ui/components/cityview/realm/settle-realm-component.tsx (1)
Line range hint
27-49: Improve accessibility by replacing the clickabledivwith abuttonelement.The
SeasonPassRealmcomponent uses a clickablediv, which may not be accessible to keyboard users or screen readers. Replacing it with abuttonelement enhances accessibility.Apply this diff:
- <div + <button key={seasonPassRealm.realmId} className={`flex flex-col gap-2 p-2 rounded-md bg-gold/10 transition-colors duration-200 border border-2 ${ selected ? "border-gold bg-gold/30" : "border-transparent" } ${className} hover:border-gold`} onClick={() => setSelected(!selected)} + type="button" > ... - </div> + </button>client/apps/game/src/ui/components/hyperstructures/contribution-summary.tsx (1)
Line range hint
93-100: Optimize sorting logic for contributors.Consider handling BigInt comparisons carefully to ensure correct sorting of contributors based on their contributions.
Apply this diff to use
BigIntcomparison methods:- return amountA > amountB ? -1 : amountA < amountB ? 1 : 0; + return amountB - amountA;client/apps/game/src/ui/components/bank/resource-bar.tsx (1)
151-154: Optimize balance retrieval withuseMemo.Memoize the balance computation to prevent unnecessary recalculations during renders, enhancing performance.
Apply this diff to implement memoization:
+ const resourceBalance = useMemo(() => { + return divideByPrecision(getBalance(entityId, resource.id, dojo.setup.components).balance); + }, [entityId, resource.id, dojo.setup.components]); <ResourceCost resourceId={resource.id} - amount={divideByPrecision(getBalance(entityId, resource.id, dojo.setup.components).balance)} + amount={resourceBalance} className="border-0 bg-transparent" />client/apps/game/src/hooks/helpers/use-players.tsx (1)
14-22: Consider adding error handling for edge cases.The implementation is clean and efficient with proper memoization. However, consider adding error handling for potential edge cases:
- Invalid addressName format
- Empty or malformed name strings
const players = useMemo(() => { return entities .map((id) => { const addressName = getComponentValue(components.AddressName, id); - if (!addressName) return; + if (!addressName?.address || !addressName?.name) return; + const name = addressName.name.toString().trim(); + if (!name) return; return { entity: id, address: addressName.address, name: addressName.name.toString() }; }) .filter(Boolean) as Player[]; }, [entities]);client/apps/game/src/hooks/helpers/use-structure-entity-id.tsx (1)
Line range hint
33-52: Consider simplifying effect logicThe effect contains complex conditional logic and many dependencies which could lead to unnecessary re-renders.
Consider:
- Extracting the position calculation logic
- Memoizing the structure query results
- Splitting into smaller, focused effects
+ const position = useMemo(() => { + return new PositionInterface({ + x: hexPosition.col, + y: hexPosition.row, + }).getContract(); + }, [hexPosition]); + const structureQuery = useMemo(() => { + const { x, y } = position; + return runQuery([Has(Structure), HasValue(Position, { x, y })]) + .values() + .next().value; + }, [position]); useEffect(() => { - const { x, y } = new PositionInterface({ - x: hexPosition.col, - y: hexPosition.row, - }).getContract(); - const structureEntity = runQuery([Has(Structure), HasValue(Position, { x, y })]) - .values() - .next().value; + const structureEntity = structureQuery;client/apps/game/src/hooks/helpers/use-battles.tsx (1)
43-43: Good architectural improvement!The refactoring improves type safety and modularity by:
- Using ContractAddress for account addresses
- Using the more specific usePlayerRealms hook instead of the generic useEntities
Consider documenting this pattern of using specific hooks over generic ones in the team's coding guidelines, as it improves maintainability and type safety.
Also applies to: 49-49
client/apps/game/src/ui/modules/social/end-season-button.tsx (1)
51-55: Consider memoizing LeaderboardManager instance.While the change to use LeaderboardManager is good for centralization, the instance creation in the render cycle could be optimized.
+const leaderboardManager = useMemo( + () => LeaderboardManager.instance(setup.components), + [setup.components] +); const endGame = useCallback(async () => { if (!hasReachedFinalPoints) { return; } const contributions = Array.from( - LeaderboardManager.instance(setup.components).getHyperstructuresWithContributionsFromPlayer( + leaderboardManager.getHyperstructuresWithContributionsFromPlayer( ContractAddress(account.address), ), );client/apps/game/src/ui/components/trading/realm-production.tsx (1)
81-85: Consider accessibility improvements for resource icons.The resource icons should include proper aria labels for screen readers.
{realm.resourcesProduced.map((resourceId) => ( <ResourceIcon resource={ResourcesIds[resourceId]} size="sm" key={resourceId} + aria-label={`Resource: ${ResourcesIds[resourceId]}`} /> ))}client/apps/game/src/ui/components/trading/select-resources.tsx (1)
1-1: LGTM! Consistent refactoring approach.The changes align with the broader refactoring pattern, maintaining consistent usage of utility functions with dojo context.
However, there's a potential optimization opportunity:
Consider memoizing the balance calculations to prevent unnecessary recalculations:
+const resourceBalance = useMemo(() => + getBalance(entity_id, id, dojo.setup.components), + [entity_id, id, dojo.setup.components] +); + const options = [orderedResources.find((res) => res.id === id), ...unselectedResources].map((res: any) => ({ id: res.id, label: ( <ResourceCost resourceId={res.id} - amount={divideByPrecision(getBalance(entity_id, res.id, dojo.setup.components)?.balance || 0)} + amount={divideByPrecision(resourceBalance?.balance || 0)} /> ), }));Also applies to: 8-8, 25-25, 55-55, 60-63
client/apps/game/src/ui/components/entities/entity.tsx (1)
8-10: LGTM! Consistent refactoring with improved type safety.The changes maintain the same functionality while providing better type safety through explicit contract addresses and components.
However, there's a potential performance optimization:
Consider memoizing the entity info to prevent unnecessary recalculations on each render:
+const entityInfo = useMemo(() => + getEntityInfo( + arrival.entityId, + ContractAddress(dojo.account.account.address), + dojo.setup.components + ), + [arrival.entityId, dojo.account.account.address, dojo.setup.components] +); -const entity = getEntityInfo(arrival.entityId, ContractAddress(dojo.account.account.address), dojo.setup.components);Also applies to: 34-35, 41-41, 44-44, 53-53, 58-58
client/apps/game/src/utils/army.ts (2)
108-116: Consider using BigInt comparison for troop counts.The
Number()conversion of BigInt values could potentially lead to precision loss for very large numbers. Consider using BigInt comparison instead.- (Number(army.troops.knight_count) !== 0 || - Number(army.troops.crossbowman_count) !== 0 || - Number(army.troops.paladin_count) !== 0), + (army.troops.knight_count !== 0n || + army.troops.crossbowman_count !== 0n || + army.troops.paladin_count !== 0n),
118-122: Remove redundant null check in condition.The
army &&check is redundant since theentityArmiesparameter type isArmyInfo[](not optional). Also,realmPositionis a required parameter, so its null check is unnecessary.- return entityArmies.some( - (army) => army && realmPosition && (army.position.x !== realmPosition.x || army.position.y !== realmPosition.y), - ); + return entityArmies.some( + (army) => army.position.x !== realmPosition.x || army.position.y !== realmPosition.y + );client/apps/game/src/ui/modules/social/social.tsx (1)
40-42: Consider memoizing initial playerInfo calculation.The initial playerInfo calculation could be memoized using useMemo to avoid unnecessary recalculations during renders.
- const [playerInfo, setPlayerInfo] = useState<PlayerInfo[]>( - getPlayerInfo(players, ContractAddress(account.address), components), - ); + const initialPlayerInfo = useMemo( + () => getPlayerInfo(players, ContractAddress(account.address), components), + [players, account.address, components] + ); + const [playerInfo, setPlayerInfo] = useState<PlayerInfo[]>(initialPlayerInfo);client/apps/game/src/ui/components/structures/construction/structure-construction-menu.tsx (1)
42-45: Consider memoizing balance checks.The
checkBalancefunction is recreated on every render and performs balance checks for each resource. Consider memoizing the results to avoid unnecessary recalculations.- const checkBalance = (cost: any) => - Object.keys(cost).every((resourceId) => { - const resourceCost = cost[Number(resourceId)]; - const balance = getBalance(entityId, resourceCost.resource, dojo.setup.components); - return balance.balance >= multiplyByPrecision(resourceCost.amount); - }); + const checkBalance = useMemo( + () => (cost: any) => + Object.keys(cost).every((resourceId) => { + const resourceCost = cost[Number(resourceId)]; + const balance = getBalance(entityId, resourceCost.resource, dojo.setup.components); + return balance.balance >= multiplyByPrecision(resourceCost.amount); + }), + [entityId, dojo.setup.components] + );client/apps/game/src/ui/modules/entity-details/building-entity-details.tsx (1)
52-56: Consider extracting complex parameter passing.The
getEntityInfocall with multiple parameters could be simplified by creating a wrapper function or custom hook.+const useEntityInfo = (entityId: ID) => { + const dojo = useDojo(); + return getEntityInfo( + entityId, + ContractAddress(dojo.account.account.address), + dojo.setup.components + ); +}; -const selectedStructureInfo = getEntityInfo( - structureEntityId, - ContractAddress(dojo.account.account.address), - dojo.setup.components, -); +const selectedStructureInfo = useEntityInfo(structureEntityId);Also applies to: 79-79
client/apps/game/src/ui/modules/navigation/quest-menu.tsx (1)
204-207: Consider using React Context for components prop.The
componentsprop is being passed down through multiple levels. Consider using React Context to avoid prop drilling.+const ComponentsContext = React.createContext<Components | null>(null); +export const ComponentsProvider: React.FC<{ children: React.ReactNode }> = ({ children }) => { + const { setup: { components } } = useDojo(); + return ( + <ComponentsContext.Provider value={components}> + {children} + </ComponentsContext.Provider> + ); +}; +const useComponents = () => { + const context = React.useContext(ComponentsContext); + if (!context) { + throw new Error('useComponents must be used within a ComponentsProvider'); + } + return context; +}; -const QuestRewards = ({ realmEntityId, prizes }: { realmEntityId: ID; prizes: Prize[] | undefined }) => { - const { - setup: { components }, - } = useDojo(); +const QuestRewards = ({ realmEntityId, prizes }: { realmEntityId: ID; prizes: Prize[] | undefined }) => { + const components = useComponents();Also applies to: 214-214
client/apps/game/src/ui/modules/entity-details/realm/buildings.tsx (1)
29-29: Verify the impact of null coalescing.The use of
||for fallback values might not handle all edge cases. Consider using nullish coalescing (??) which only falls back onnullorundefined:- const buildings = useBuildings(realm?.position.x || 0, realm?.position.y || 0); + const buildings = useBuildings(realm?.position.x ?? 0, realm?.position.y ?? 0);client/apps/game/src/hooks/helpers/use-structures.tsx (1)
46-46: Consider adding null checks for getEntityName results.While updating the API to include components is good, the function calls might return undefined. Consider adding null coalescing operators or default values:
- const name = getEntityName(structure.entity_id, components); + const name = getEntityName(structure.entity_id, components) ?? "";Also applies to: 90-90, 133-133, 180-180
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (82)
client/apps/game/src/hooks/helpers/use-battle-events.tsx(3 hunks)client/apps/game/src/hooks/helpers/use-battles.tsx(2 hunks)client/apps/game/src/hooks/helpers/use-buildings.tsx(2 hunks)client/apps/game/src/hooks/helpers/use-contributions.tsx(1 hunks)client/apps/game/src/hooks/helpers/use-entities.tsx(1 hunks)client/apps/game/src/hooks/helpers/use-fragment-mines.tsx(1 hunks)client/apps/game/src/hooks/helpers/use-get-all-players.tsx(0 hunks)client/apps/game/src/hooks/helpers/use-guilds.tsx(10 hunks)client/apps/game/src/hooks/helpers/use-hyperstructures.tsx(3 hunks)client/apps/game/src/hooks/helpers/use-players.tsx(1 hunks)client/apps/game/src/hooks/helpers/use-quests.tsx(4 hunks)client/apps/game/src/hooks/helpers/use-realm.tsx(1 hunks)client/apps/game/src/hooks/helpers/use-resource-arrivals.tsx(1 hunks)client/apps/game/src/hooks/helpers/use-resources.tsx(1 hunks)client/apps/game/src/hooks/helpers/use-structure-entity-id.tsx(2 hunks)client/apps/game/src/hooks/helpers/use-structures.tsx(9 hunks)client/apps/game/src/hooks/helpers/use-trade.tsx(3 hunks)client/apps/game/src/hooks/use-starting-tutorial.tsx(1 hunks)client/apps/game/src/ui/components/bank/add-liquidity.tsx(4 hunks)client/apps/game/src/ui/components/bank/liquidity-table.tsx(3 hunks)client/apps/game/src/ui/components/bank/resource-bar.tsx(4 hunks)client/apps/game/src/ui/components/bank/swap.tsx(2 hunks)client/apps/game/src/ui/components/battles/battle-list-item.tsx(2 hunks)client/apps/game/src/ui/components/cityview/realm/settle-realm-component.tsx(1 hunks)client/apps/game/src/ui/components/construction/select-preview-building.tsx(9 hunks)client/apps/game/src/ui/components/entities/entity.tsx(3 hunks)client/apps/game/src/ui/components/fragmentMines/fragment-mine-panel.tsx(1 hunks)client/apps/game/src/ui/components/hyperstructures/co-owners.tsx(5 hunks)client/apps/game/src/ui/components/hyperstructures/contribution-summary.tsx(5 hunks)client/apps/game/src/ui/components/hyperstructures/hyperstructure-panel.tsx(4 hunks)client/apps/game/src/ui/components/hyperstructures/hyperstructure-resource-chip.tsx(2 hunks)client/apps/game/src/ui/components/hyperstructures/leaderboard.tsx(4 hunks)client/apps/game/src/ui/components/hyperstructures/resource-exchange.tsx(3 hunks)client/apps/game/src/ui/components/military/army-chip.tsx(1 hunks)client/apps/game/src/ui/components/military/army-list.tsx(1 hunks)client/apps/game/src/ui/components/military/army-management-card.tsx(3 hunks)client/apps/game/src/ui/components/military/entities-army-table.tsx(3 hunks)client/apps/game/src/ui/components/military/pillage-history.tsx(4 hunks)client/apps/game/src/ui/components/quest/quest-details.ts(1 hunks)client/apps/game/src/ui/components/resources/deposit-resources.tsx(1 hunks)client/apps/game/src/ui/components/resources/inventory-resources.tsx(2 hunks)client/apps/game/src/ui/components/resources/realm-resources-io.tsx(1 hunks)client/apps/game/src/ui/components/resources/realm-transfer.tsx(3 hunks)client/apps/game/src/ui/components/resources/travel-info.tsx(3 hunks)client/apps/game/src/ui/components/structures/construction/structure-construction-menu.tsx(5 hunks)client/apps/game/src/ui/components/trading/market-modal.tsx(3 hunks)client/apps/game/src/ui/components/trading/market-order-panel.tsx(2 hunks)client/apps/game/src/ui/components/trading/realm-production.tsx(4 hunks)client/apps/game/src/ui/components/trading/resource-arrivals.tsx(1 hunks)client/apps/game/src/ui/components/trading/select-entity-from-list.tsx(2 hunks)client/apps/game/src/ui/components/trading/select-resources.tsx(3 hunks)client/apps/game/src/ui/components/trading/trade-history-event.tsx(3 hunks)client/apps/game/src/ui/components/trading/transfer-between-entities.tsx(3 hunks)client/apps/game/src/ui/components/trading/transfer-view.tsx(3 hunks)client/apps/game/src/ui/components/worldmap/armies/action-info.tsx(5 hunks)client/apps/game/src/ui/components/worldmap/armies/army-info-label.tsx(3 hunks)client/apps/game/src/ui/components/worldmap/guilds/guild-members.tsx(3 hunks)client/apps/game/src/ui/components/worldmap/guilds/guilds.tsx(2 hunks)client/apps/game/src/ui/components/worldmap/players/player-list.tsx(1 hunks)client/apps/game/src/ui/components/worldmap/players/players-panel.tsx(2 hunks)client/apps/game/src/ui/layouts/world.tsx(5 hunks)client/apps/game/src/ui/modules/chat/chat.tsx(2 hunks)client/apps/game/src/ui/modules/entity-details/building-entity-details.tsx(4 hunks)client/apps/game/src/ui/modules/entity-details/enemy-armies.tsx(3 hunks)client/apps/game/src/ui/modules/entity-details/realm/buildings.tsx(3 hunks)client/apps/game/src/ui/modules/entity-details/realm/castle.tsx(3 hunks)client/apps/game/src/ui/modules/military/battle-view/battle-side-view.tsx(3 hunks)client/apps/game/src/ui/modules/military/battle-view/battle-twitter-share-button.tsx(3 hunks)client/apps/game/src/ui/modules/military/military.tsx(1 hunks)client/apps/game/src/ui/modules/navigation/left-navigation-module.tsx(4 hunks)client/apps/game/src/ui/modules/navigation/quest-menu.tsx(5 hunks)client/apps/game/src/ui/modules/navigation/top-left-navigation.tsx(3 hunks)client/apps/game/src/ui/modules/navigation/top-navigation.tsx(1 hunks)client/apps/game/src/ui/modules/rewards/rewards.tsx(5 hunks)client/apps/game/src/ui/modules/settings/settings.tsx(1 hunks)client/apps/game/src/ui/modules/social/end-season-button.tsx(2 hunks)client/apps/game/src/ui/modules/social/player-id.tsx(4 hunks)client/apps/game/src/ui/modules/social/social.tsx(5 hunks)client/apps/game/src/ui/modules/stream/event-stream.tsx(2 hunks)client/apps/game/src/ui/modules/world-structures/world-structures-menu.tsx(6 hunks)client/apps/game/src/ui/utils/realms.tsx(1 hunks)client/apps/game/src/utils/army.ts(1 hunks)
⛔ Files not processed due to max files limit (8)
- client/apps/game/src/utils/entities.ts
- client/apps/game/src/utils/players.ts
- client/apps/game/src/utils/realm.ts
- client/apps/game/src/utils/resources.ts
- client/sdk/packages/eternum/src/constants/resources.ts
- client/sdk/packages/eternum/src/modelManager/LeaderboardManager.ts
- client/sdk/packages/eternum/src/types/common.ts
- client/sdk/packages/eternum/src/utils/index.ts
💤 Files with no reviewable changes (1)
- client/apps/game/src/hooks/helpers/use-get-all-players.tsx
✅ Files skipped from review due to trivial changes (2)
- client/apps/game/src/ui/components/quest/quest-details.ts
- client/apps/game/src/ui/components/resources/deposit-resources.tsx
🔇 Additional comments (116)
client/apps/game/src/ui/components/worldmap/armies/action-info.tsx (4)
9-9: LGTM! Good refactoring of resource balance handling.The change from
useResourceBalancehook to a utility function improves testability and follows a more functional approach.
27-29: Great type safety improvements!The changes enhance type safety by:
- Using more precise
numbertype for entity IDs instead ofstring- Adding explicit type definition for the
getBalancefunction
128-128: Good addition of null check!Adding the null check for
selectedEntityIdimproves the component's safety by preventing potential runtime errors.
136-138: LGTM! Clean implementation of getBalance.The implementation correctly passes the components context to the utility function while maintaining proper typing.
client/apps/game/src/ui/components/military/army-management-card.tsx (1)
21-21: LGTM! Clean refactoring of resource balance handling.The change from
useResourceBalancehook togetBalanceutility function aligns with the PR's objective of centralizing data retrieval.client/apps/game/src/ui/components/worldmap/players/players-panel.tsx (1)
21-28: Clean component destructuring!The separation of component destructuring improves code readability and maintainability.
client/apps/game/src/hooks/helpers/use-resources.tsx (2)
2-2: Imports are appropriate and necessaryThe imported identifiers
ID,ResourceManager, andResourcesIdsfrom"@bibliothecadao/eternum"are correctly utilized in the code.
4-4:useMemohook correctly importedThe
useMemohook from React is properly imported and used for memoization.client/apps/game/src/hooks/helpers/use-contributions.tsx (2)
2-4: Imported modules and types are correctly utilizedThe imports from
"@bibliothecadao/eternum"and"@dojoengine/recs"are necessary and properly used in theusePlayerContributionshook.
6-19:usePlayerContributionshook effectively retrieves player contributionsThe hook efficiently queries and filters contributions related to the specified player and hyperstructure. The use of
useEntityQuery,HasValue, and proper type guarding ensures accurate results.client/apps/game/src/hooks/helpers/use-realm.tsx (4)
2-2: ImportinggetRealmInfoutilityThe
getRealmInfofunction is correctly imported from"@/utils/realm"and used to construct realm information.
21-21: EnsuregetRealmInfohandles undefined components gracefullyThe
getRealmInfofunction should account for potential undefined components to prevent runtime errors. Confirm that it includes necessary checks.If
getRealmInfoalready includes these checks, no action is needed.
29-47:useRealmshook correctly retrieves realm dataThe
useRealmshook efficiently queries all realms and maps their component values. The use ofuseMemooptimizes performance by memoizing the computed realms.
16-16: Ensure correct type casting withContractAddressIf
account.addressis already of typeContractAddress, wrapping it again may be redundant. Verify if additional type casting is necessary.Run the following script to check the type of
account.addressand usage:✅ Verification successful
ContractAddress wrapping is necessary and correct
The ContractAddress wrapping is required here as it ensures type safety when interacting with the Owner component in the entity query system. The Owner component expects a ContractAddress type (which is a specific bigint representation) for contract interactions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check the type of account.address in the codebase # Find where account.address is defined and its type rg 'account:\s*{\s*account\s*}' -A 5 # Check usages of account.address rg 'account\.address' -A 2Length of output: 35894
Script:
#!/bin/bash # Find type definitions and implementations related to account and ContractAddress # Look for type definitions of account rg -t typescript "type.*Account" -B 2 -A 2 # Look for ContractAddress type/function definition rg -t typescript "ContractAddress.*=|type.*ContractAddress|interface.*ContractAddress" -B 2 -A 2 # Look for direct assignments or comparisons with account.address ast-grep --pattern 'account.address = $_' -l typescript ast-grep --pattern '$_ === account.address' -l typescriptLength of output: 331
Script:
#!/bin/bash # Find TypeScript files that might contain type definitions fd "\.(ts|tsx)$" | grep -i "type\|interface" # Look for ContractAddress definition and usage rg "ContractAddress" -g "*.{ts,tsx}" --no-filename | grep "type\|interface\|function" # Look for direct account.address usage without ContractAddress wrapping rg "account\.address" -g "*.{ts,tsx}" --no-filename | grep -v "ContractAddress"Length of output: 11716
client/apps/game/src/ui/components/hyperstructures/contribution-summary.tsx (3)
20-22: ConfirmLeaderboardManagerinstantiation dependencies.Ensure that all dependencies required for
LeaderboardManager.instance(components)are included and thatcomponentsencompasses all necessary data for proper initialization.
32-34: Efficient use ofuseMemofor contributions calculation.Memoizing
contributionswithuseMemooptimizes performance by avoiding unnecessary recalculations whenleaderboardManagerorhyperstructureEntityIdchange.
103-103: Verify synchronous usage ofgetAddressName.Confirm that
getAddressNameis a synchronous function and that it won't cause any unexpected behavior if it performs asynchronous operations.client/apps/game/src/ui/components/bank/resource-bar.tsx (1)
Line range hint
28-29: Addition ofonFocusandonBlurprops enhances functionality.Introducing
onFocusandonBlurcallbacks improves the component's flexibility by allowing parent components to respond to focus events.client/apps/game/src/ui/modules/navigation/top-navigation.tsx (2)
10-10: Confirm removal ofgetPlayersprop inSocialcomponent.Ensure that the
Socialcomponent functions correctly without thegetPlayersprop and that it internally handles player data retrieval.
10-10: SimplifiedSocialcomponent usage is clean and effective.Removing unnecessary props streamlines the component and reduces complexity.
client/apps/game/src/hooks/use-starting-tutorial.tsx (1)
11-11: Verify the return type of useQuests hook.The change from destructuring to direct assignment suggests that the
useQuestshook's return type has been modified. Please ensure this change is consistent with the hook's new implementation.client/apps/game/src/ui/modules/military/military.tsx (1)
12-17: LGTM! Clean implementation of the refactored hook usage.The changes effectively utilize the useDojo context and maintain type safety while simplifying the structure retrieval logic.
client/apps/game/src/hooks/helpers/use-battle-events.tsx (2)
7-27: Great refactoring of battle event logic!The new
useBattleEventDatahelper effectively consolidates duplicate logic from battle event hooks. The use of TypeScript generics and proper memoization demonstrates good practices.
29-38: Clean implementation of battle event hooksThe refactored hooks (
useBattleStart,useBattleJoin,useBattleLeave) are now more maintainable and DRY by leveraging the shared helper function.Also applies to: 41-54, 57-70
client/apps/game/src/ui/components/resources/realm-resources-io.tsx (2)
15-15: Good: Made realmEntityId prop requiredMaking the prop required improves type safety by ensuring the component always receives a valid ID.
20-21: Verify type safety of ID conversionThe conversion of
realmEntityIdtoBigIntand passing it togetEntityIdFromKeyscould potentially fail if an invalid ID is provided.✅ Verification successful
ID conversion is properly type-safe
The conversion is safe as the
realmEntityIdcomes from a validated Structure entity and is only used when the structure type is explicitly checked. The pattern is part of the core architecture and is properly type-safe through TypeScript's type system and runtime validation in parent components.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for proper ID validation across the codebase rg -A 2 "getEntityIdFromKeys\(\[BigInt\("Length of output: 52223
Script:
#!/bin/bash # Search for the component definition and type of realmEntityId ast-grep --pattern 'interface $_ { $$$ realmEntityId: $_ $$$ }' # Search for any validation or try-catch around BigInt conversion rg -A 2 "try.*BigInt\(" rg "if.*realmEntityId"Length of output: 795
Script:
#!/bin/bash # Find the component definition and usage rg -A 5 "realm-resources-io" ast-grep --pattern 'const RealmResourcesIO = ({ $_ }: { $_ }) => {' # Find where realmEntityId is passed to this component rg -B 3 "<RealmResourcesIO.*realmEntityId="Length of output: 3332
client/apps/game/src/hooks/helpers/use-structure-entity-id.tsx (1)
26-26: Good: Using typed ContractAddress for player structuresThe switch to
usePlayerStructureswith proper typing improves type safety.client/apps/game/src/ui/components/fragmentMines/fragment-mine-panel.tsx (1)
19-19: Good: Properly passing components to utility functionThe update to include components parameter in
getAddressNameFromEntityimproves modularity.client/apps/game/src/ui/utils/realms.tsx (1)
2-2: Verify impact of removing wonder prefix functionalityThe removal of getRealmName function eliminates the "WONDER" prefix for realms with wonders. This might affect UI components that relied on this distinction.
Let's check for any UI components that might be affected:
✅ Verification successful
No impact found from removing wonder prefix functionality
The affected components are already using
getRealmNameByIdand don't rely on the wonder prefix functionality. The removal ofgetRealmNamefunction appears to be a safe cleanup.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for components that might have relied on the removed getRealmName function rg -l "getRealmName|has_wonder.*realm" "client/apps/game/src/ui/components"Length of output: 207
Script:
#!/bin/bash # Check the content of affected files echo "=== transfer-view.tsx ===" rg -A 5 -B 5 "getRealmName|has_wonder.*realm" "client/apps/game/src/ui/components/trading/transfer-view.tsx" echo -e "\n=== army-info-label.tsx ===" rg -A 5 -B 5 "getRealmName|has_wonder.*realm" "client/apps/game/src/ui/components/worldmap/armies/army-info-label.tsx"Length of output: 2376
client/apps/game/src/ui/components/bank/liquidity-table.tsx (1)
45-45: Good simplification of data flowThe changes improve the code by:
- Using the specific usePlayerStructures hook
- Simplifying playerStructureIds derivation
- Adding proper typing with ContractAddress
Also applies to: 47-47
client/apps/game/src/hooks/helpers/use-buildings.tsx (2)
9-19: Good improvement in hook specificity and performanceThe useBuildings hook now properly requires location parameters and uses useEntityQuery with useMemo for better performance.
59-79: Review the dependency array in useBuildingQuantitiesThe useMemo dependency array might cause unnecessary rerenders or stale data:
- entityUpdate is an array-like object that might cause reference equality issues
- Consider using individual building quantities as dependencies instead
Let's check for potential issues:
Consider refactoring the dependency array:
return useMemo( () => ({ food: getBuildingQuantity(BuildingType.Farm) + getBuildingQuantity(BuildingType.FishingVillage), resource: getBuildingQuantity(BuildingType.Resource), workersHut: getBuildingQuantity(BuildingType.WorkersHut), markets: getBuildingQuantity(BuildingType.Market), }), - [structureEntityId, entityUpdate], + [structureEntityId, + getBuildingQuantity(BuildingType.Farm), + getBuildingQuantity(BuildingType.FishingVillage), + getBuildingQuantity(BuildingType.Resource), + getBuildingQuantity(BuildingType.WorkersHut), + getBuildingQuantity(BuildingType.Market)], );client/apps/game/src/ui/components/trading/trade-history-event.tsx (2)
1-1: LGTM! Refactoring aligns with centralized context pattern.The change from useEntitiesUtils to useDojo follows a better pattern of accessing shared components through a centralized context.
Also applies to: 28-30
39-39: Verify the impact of the getAddressNameFromEntity signature change.The function now requires the components parameter. While the change here is correct, we should ensure all other usages of this function have been updated.
✅ Verification successful
All usages of getAddressNameFromEntity are properly updated
All occurrences of the function across the codebase include both the entityId and components parameters. No additional updates are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining calls to getAddressNameFromEntity that might not include the components parameter rg "getAddressNameFromEntity\([^,)]*\)" --type tsLength of output: 51
Script:
#!/bin/bash # Search for all occurrences of getAddressNameFromEntity with surrounding context rg -i "getAddressNameFromEntity" --type ts -B 2 -A 2Length of output: 15487
client/apps/game/src/ui/components/trading/realm-production.tsx (1)
21-30: LGTM! Good use of memoization for derived data.The memoization of realmsProduction prevents unnecessary recalculations of the transformed data.
client/apps/game/src/ui/components/trading/resource-arrivals.tsx (1)
10-10: LGTM! Good move to centralize type definitions.Moving the ArrivalInfo type to @bibliothecadao/eternum improves type sharing across the project.
client/apps/game/src/ui/components/hyperstructures/leaderboard.tsx (1)
23-23: LGTM! Consistent usage of components.The changes improve consistency by explicitly passing components to utility functions and using the correct component paths.
Also applies to: 51-51, 76-76
client/apps/game/src/ui/components/trading/transfer-view.tsx (1)
2-2: LGTM! Improved modularity with specific hooks.The changes enhance modularity by replacing the generic
useEntitieshook with specific hooks for player realms and structures.Also applies to: 4-4, 20-21
client/apps/game/src/ui/components/hyperstructures/hyperstructure-resource-chip.tsx (1)
1-1: LGTM! Refactoring improves code modularity.The transition from custom hooks to utility functions with explicit dojo context makes the data flow more transparent and maintainable.
However, let's verify that all callers of these utility functions have been updated with the new signature.
Also applies to: 8-8, 29-30, 34-35
client/apps/game/src/ui/components/worldmap/armies/army-info-label.tsx (1)
39-47: LGTM! Good use of hook refactoring pattern.The changes properly implement the hook refactoring pattern by:
- Using
useDojoto access components centrally- Passing components explicitly to
getRealmAddressNameclient/apps/game/src/ui/modules/military/battle-view/battle-side-view.tsx (1)
8-8: LGTM! Refactoring improves modularity.The change from using
useEntitiesUtilshook to directgetAddressNameFromEntityfunction import with explicitcomponentsparameter improves code clarity and maintainability.Also applies to: 97-97
client/apps/game/src/ui/components/military/army-list.tsx (1)
10-10: LGTM! Type safety improvement.Adding explicit
PlayerStructuretype improves type safety and makes the component's requirements clearer.client/apps/game/src/ui/components/battles/battle-list-item.tsx (1)
10-10: LGTM! Consistent with the hook refactoring pattern.The change from
useEntitiesUtilshook to directgetAddressNameFromEntityfunction import maintains consistency with the broader refactoring effort.Also applies to: 108-108
client/apps/game/src/hooks/helpers/use-resource-arrivals.tsx (1)
3-9: LGTM! Improved code organization.Moving types and constants to
@bibliothecadao/eternumreduces duplication and centralizes shared definitions.client/apps/game/src/ui/components/worldmap/guilds/guilds.tsx (1)
10-10: LGTM! Type renaming is consistent.The change from
PlayertoPlayerInfotype is consistent with the refactoring mentioned in the PR summary.Also applies to: 19-19
client/apps/game/src/ui/components/military/pillage-history.tsx (2)
18-18: LGTM! Components dependency is now explicit.The changes improve code clarity by making the dependency on
componentsexplicit in utility function calls.Also applies to: 27-27, 34-34
132-141: LGTM! Query definition is more concise.The query definition and subscription logic are now more streamlined with direct usage of components.
client/apps/game/src/ui/components/bank/add-liquidity.tsx (2)
2-2: LGTM! Improved modularity with specific hooks.The change from
useEntitiestousePlayerStructuresimproves code organization by using a more focused hook.Also applies to: 29-29, 31-31
64-65: LGTM! Balance retrieval is more explicit.The balance retrieval now correctly includes the components context, making the dependencies clear.
client/apps/game/src/ui/modules/social/player-id.tsx (2)
59-67: LGTM! Improved component organization.The component destructuring is now more organized and maintainable.
82-82: LGTM! Utility function calls are more explicit.The utility functions now correctly receive the components context, making dependencies clear.
Also applies to: 111-111
client/apps/game/src/ui/components/worldmap/guilds/guild-members.tsx (2)
10-10: LGTM! Type safety improvements.The changes improve type safety by using the more specific
PlayerInfotype instead of the genericPlayertype.Also applies to: 15-15
26-26: LGTM! Enhanced context through components.The addition of
componentsto thegetGuildFromEntityIdfunction call provides better context for guild data retrieval.Also applies to: 39-39
client/apps/game/src/ui/components/hyperstructures/resource-exchange.tsx (1)
6-6: LGTM! Improved resource management.The changes enhance resource management by:
- Centralizing resource-related utilities through the imported
getResourcesFromBalancefunction- Providing better context through the
componentsparameterAlso applies to: 48-49
client/apps/game/src/hooks/helpers/use-trade.tsx (1)
2-2: LGTM! Enhanced player-specific data handling.The changes improve the code by:
- Using the more specific
usePlayerRealmshook instead of the genericuseEntities- Simplifying the trade filtering logic using array mapping
Also applies to: 169-170, 181-181
client/apps/game/src/hooks/helpers/use-quests.tsx (4)
6-8: LGTM! Better organization of utility functions.The changes improve code organization by centralizing utility functions and type imports.
18-36: LGTM! Enhanced quest management.The changes improve the code by:
- Defining quest types as a constant array for better maintainability
- Using
useMemofor efficient quest generation
55-59: LGTM! Improved position retrieval.The changes enhance position retrieval by using the
getEntityInfoutility with proper context through components.
89-89: LGTM! Simplified quest count management.The changes simplify the unclaimed quests count management by directly returning the count.
Also applies to: 208-208
client/apps/game/src/ui/components/worldmap/players/player-list.tsx (1)
8-8: LGTM! Type updates look good.The change from
PlayertoPlayerInfotype is consistent with the broader refactoring effort.Also applies to: 12-12
client/apps/game/src/ui/modules/stream/event-stream.tsx (2)
5-5: LGTM! Direct imports improve code clarity.Importing utility functions directly instead of through a hook makes the dependencies more explicit.
51-52: Verify the impact of function signature changes.The utility functions now require additional parameters (
components). Let's verify that all callers have been updated correctly.Also applies to: 58-60
✅ Verification successful
All function calls are properly updated
A thorough search of the codebase confirms that all calls to both
getAddressNameFromEntityandgetPlayerAddressFromEntityinclude the requiredcomponentsparameter. No outdated function calls were found.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining calls to these functions without the components parameter rg "getAddressNameFromEntity\([^,]+" --type ts --type tsx rg "getPlayerAddressFromEntity\([^,]+" --type ts --type tsxLength of output: 177
Script:
#!/bin/bash # Search for function calls using ast-grep # First, find getAddressNameFromEntity calls ast-grep --pattern 'getAddressNameFromEntity($_)' # Then search for getPlayerAddressFromEntity calls ast-grep --pattern 'getPlayerAddressFromEntity($_)'Length of output: 105
Script:
#!/bin/bash # Find function definitions and usages with broader patterns rg "function.*getAddressNameFromEntity|const.*getAddressNameFromEntity" -t typescript rg "function.*getPlayerAddressFromEntity|const.*getPlayerAddressFromEntity" -t typescript # Also search for imports of these functions rg "import.*getAddressNameFromEntity" -t typescript rg "import.*getPlayerAddressFromEntity" -t typescriptLength of output: 429
Script:
#!/bin/bash # Find all TypeScript files and search within them fd -e ts -e tsx | xargs rg "getAddressNameFromEntity" fd -e ts -e tsx | xargs rg "getPlayerAddressFromEntity"Length of output: 5463
client/apps/game/src/ui/modules/entity-details/building-entity-details.tsx (1)
3-3: LGTM! Hook replacement improves specificity.Using
usePlayerStructuresinstead of the genericuseEntitiesbetter reflects the intent of the code.Also applies to: 50-50
client/apps/game/src/ui/modules/navigation/quest-menu.tsx (1)
2-2: LGTM! Import organization improves clarity.The changes better organize imports and hooks, making dependencies clearer.
Also applies to: 8-9, 25-25, 36-36
client/apps/game/src/ui/modules/entity-details/realm/buildings.tsx (2)
24-27: Good use of useMemo for caching realm info!The memoization of
getRealmInfoprevents unnecessary recalculations when dependencies haven't changed, improving performance.
59-61: LGTM! Improved null safety.Good defensive programming by moving the owner check after the realm null check.
client/apps/game/src/hooks/helpers/use-hyperstructures.tsx (2)
28-28: LGTM! Improved component organization.Good separation of concerns by first destructuring the components object and then individual components. This makes the component dependencies more explicit and easier to maintain.
Also applies to: 31-31
49-49: Verify the impact of the API change.The
getAddressNameFromEntityfunction now requires the components parameter. Ensure all other call sites have been updated accordingly.✅ Verification successful
All call sites properly updated ✅
All occurrences of
getAddressNameFromEntityin the codebase have been verified to include both theentityIdandcomponentsparameters as required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining calls to getAddressNameFromEntity that might be missing the components parameter rg "getAddressNameFromEntity\([^,]+" --type ts --type tsxLength of output: 87
Script:
#!/bin/bash # Search for all occurrences of getAddressNameFromEntity using ripgrep rg "getAddressNameFromEntity" --type typescript --type-add "typescript:*.{ts,tsx}" # Use ast-grep to find function calls ast-grep --pattern 'getAddressNameFromEntity($_)' # Also search for the function definition to understand the required parameters ast-grep --pattern 'function getAddressNameFromEntity($_) { $$$ }'Length of output: 4080
client/apps/game/src/ui/modules/rewards/rewards.tsx (1)
46-48: LGTM! Good use of singleton pattern.Using LeaderboardManager.instance with useMemo ensures consistent state management and prevents unnecessary reinstantiation.
client/apps/game/src/ui/components/resources/realm-transfer.tsx (3)
2-2: LGTM! Import changes align with hook refactoring.The replacement of
useEntitieswithusePlayerStructuresimproves modularity by using a more specific hook.
36-36: LGTM! Simplified player structures access.The change improves code readability by directly accessing the player structures array instead of using a function call.
Line range hint
110-117: LGTM! Player structures mapping is correct.The mapping logic properly handles player structures and passes all required props to the RealmTransferBalance component.
client/apps/game/src/ui/modules/navigation/left-navigation-module.tsx (2)
1-1: LGTM! Import changes improve modularity.The updated imports better organize dependencies and align with the hook refactoring effort.
Also applies to: 12-13
54-58: LGTM! Enhanced entity info retrieval.The changes properly integrate the Dojo context and improve entity info retrieval by providing necessary parameters.
Also applies to: 72-72
client/apps/game/src/ui/components/hyperstructures/co-owners.tsx (4)
3-3: LGTM! Import changes improve code organization.The addition of specific hooks and utilities enhances modularity and code organization.
Also applies to: 13-13
57-60: LGTM! Improved components setup.The explicit destructuring of components enhances code readability and maintainability.
119-119: LGTM! Enhanced player name retrieval.The updated getAddressName call properly includes components for context.
179-179: LGTM! Simplified player data retrieval.The use of the dedicated usePlayers hook improves code clarity and maintainability.
client/apps/game/src/ui/modules/settings/settings.tsx (3)
22-22: LGTM! Import change improves modularity.The addition of the getAddressName utility import enhances code organization.
30-30: LGTM! Added components setup.The proper integration of components from the Dojo context enhances functionality.
35-35: LGTM! Enhanced address name retrieval.The updated getAddressName call properly includes components for context.
client/apps/game/src/hooks/helpers/use-guilds.tsx (2)
172-183: LGTM! Clean destructuring of components.The components are now properly destructured from
dojo.setup.components, improving code readability and maintainability.
206-227: LGTM! Well-implemented memoized function.The
getGuildFromEntityIdfunction is properly memoized with appropriate dependencies, preventing unnecessary recalculations and improving performance.client/apps/game/src/ui/components/military/army-chip.tsx (1)
17-17: LGTM! Import moved to utils.The
armyHasTroopsimport has been moved to@/utils/army, aligning with the codebase's organization of utility functions.client/apps/game/src/ui/modules/world-structures/world-structures-menu.tsx (2)
42-48: LGTM! Efficient memoization of hyperstructures.The
myHyperstructuresvalue is now properly memoized with appropriate dependencies, preventing unnecessary recalculations of hyperstructure contributions.
182-184: LGTM! Clean components destructuring.Components are properly destructured from
dojo.setup, improving code readability and maintainability.client/apps/game/src/ui/components/hyperstructures/hyperstructure-panel.tsx (2)
69-69: LGTM! Using player-specific contributions hook.The change to
usePlayerContributionsimproves code clarity by explicitly indicating the hook's purpose.
77-77: LGTM! Structured component access.Using
components.Hyperstructureprovides a more structured and maintainable way to access components.client/apps/game/src/ui/layouts/world.tsx (3)
8-8: LGTM! Import changes improve modularity.The replacement of
useEntitieswithusePlayerStructuresmakes the code more specific and focused. Adding thePlayerStructuretype improves type safety.Also applies to: 16-16
117-117: LGTM! Hook usage is simplified and dependencies are properly updated.The direct use of
usePlayerStructureshook makes the code cleaner and more maintainable. TheuseMemodependencies are correctly updated to reflect the new data source.Also applies to: 120-121
338-338: LGTM! Props updated consistently with hook changes.The
TopLeftNavigationcomponent now receivesplayerStructuresdirectly from the new hook, maintaining consistency with the refactoring.client/apps/game/src/ui/components/trading/transfer-between-entities.tsx (1)
14-14: LGTM! Explicit dependency injection improves maintainability.The changes properly inject the
componentsdependency intogetRealmAddressName, making the code's dependencies more explicit and easier to test.Also applies to: 253-253
client/apps/game/src/ui/components/bank/swap.tsx (1)
13-13: LGTM! Resource balance handling is more modular.The changes improve modularity by:
- Using a dedicated utility function instead of a hook
- Explicitly passing the components dependency
- Properly memoizing the balance calculations
Also applies to: 78-85
client/apps/game/src/ui/components/trading/market-modal.tsx (1)
9-9: LGTM! Market modal refactoring improves code organization.The changes:
- Use more specific hooks (useBank, usePlayerStructures)
- Simplify structure handling by using the hook directly
- Maintain consistent mapping in the Select component
Also applies to: 11-11, 62-62, 218-222
client/apps/game/src/ui/modules/chat/chat.tsx (3)
4-4: LGTM! Hook replacement aligns with refactoring goals.The replacement of
useGetAllPlayerswithusePlayersaligns with the PR's objective of centralizing data retrieval.
47-47: LGTM! Clean implementation of the new hook.The direct usage of
usePlayers()simplifies the code by removing the need for additional filtering logic.
Line range hint
276-276: LGTM! Dependencies are properly specified.The dependency array includes all necessary dependencies for proper memoization.
client/apps/game/src/ui/modules/navigation/top-left-navigation.tsx (4)
15-26: LGTM! Clean import organization.The imports are well-organized, with related imports grouped together. The addition of
ContractAddressand other types from@bibliothecadao/eternumimproves type safety.
104-107: LGTM! Proper context destructuring.The destructuring of
dojocontext is clean and provides clear access to both setup and account information.
123-126: LGTM! Improved entity info retrieval.The updated
getEntityInfocall now includes the necessary context (ContractAddress(account.address)andsetup.components), making the data dependencies explicit.
130-130: LGTM! Optimized dependencies.The dependency array has been simplified to include only the essential dependencies, which helps prevent unnecessary re-renders.
client/apps/game/src/ui/components/trading/market-order-panel.tsx (2)
19-19: LGTM! Clean import.The import of
getRealmAddressNamefrom the realm utils module is appropriate.
360-361: LGTM! Updated function call with proper context.The
getRealmAddressNamecall now includesdojo.setup.components, aligning with the refactoring to provide explicit component context.client/apps/game/src/ui/components/construction/select-preview-building.tsx (4)
23-24: LGTM! Clean utility imports.The direct imports of utility functions align with the refactoring goal of removing custom hooks in favor of utility functions.
47-47: LGTM! Clean realm info retrieval.Direct use of
getRealmInfowith proper parameters simplifies the code.
77-77: LGTM! Updated balance check.The
getBalancecall now includes the requireddojo.setup.componentsparameter.
Line range hint
461-627: LGTM! Consistent balance retrieval pattern.All instances of
getBalancecalls have been consistently updated to includedojo.setup.components. The pattern is applied uniformly across resource costs, building costs, and ongoing costs.client/apps/game/src/ui/components/trading/select-entity-from-list.tsx (3)
1-1: LGTM! Import changes align with hook refactoring.The replacement of
useRealmwithuseDojoand the direct import ofgetRealmAddressNamefrom utils aligns with the centralization of data retrieval.Also applies to: 3-3
22-24: LGTM! Proper destructuring of components from useDojo.The destructuring pattern follows the standard approach for accessing components through the Dojo context.
31-31: LGTM! Updated function call with required components parameter.The
getRealmAddressNamefunction call has been properly updated to include the requiredcomponentsparameter.client/apps/game/src/ui/modules/military/battle-view/battle-twitter-share-button.tsx (3)
1-1: LGTM! Import changes align with hook refactoring.The replacement of entity utilities with
useDojoand direct import ofgetAddressNameFromEntityfollows the centralization pattern.Also applies to: 5-5
26-28: LGTM! Proper destructuring of components from useDojo.The destructuring pattern follows the standard approach for accessing components through the Dojo context.
40-43: LGTM! Updated function call with required components parameter.The
getAddressNameFromEntityfunction call has been properly updated to include the requiredcomponentsparameter.client/apps/game/src/ui/components/resources/travel-info.tsx (3)
2-2: LGTM! Import changes align with hook refactoring.The replacement of resource balance hooks with
useDojoand direct import ofgetBalancefollows the centralization pattern.Also applies to: 12-12
29-30: LGTM! Proper initialization of dojo context.The
useDojohook is correctly initialized at the component level.
41-41: LGTM! Updated function call with required components parameter.The
getBalancefunction call has been properly updated to include the requiredcomponentsparameter.
| import { getEntityName } from "@/utils/entities"; | ||
| import { ContractAddress, PlayerInfo } from "@bibliothecadao/eternum"; |
There was a problem hiding this comment.
💡 Codebase verification
Migration from Player[] to PlayerInfo[] is incomplete
Several components still use the old Player[] type while others have migrated to PlayerInfo[]. This inconsistency needs to be addressed in:
client/apps/landing/src/components/modules/player-leaderboard-panel.tsxclient/apps/game/src/utils/players.tsclient/apps/game/src/ui/elements/select-address.tsxclient/apps/game/src/ui/modules/chat/chat.tsxclient/apps/game/src/hooks/helpers/use-players.tsx
🔗 Analysis chain
Verify the impact of PlayerInfo type change across components.
The transition from Player[] to PlayerInfo[] and the centralization of entity utilities look good. However, let's verify the consistency of these changes across dependent components.
Also applies to: 16-16
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining Player[] type usage that might need updating
rg "Player\[\]" --type typescript
# Verify PlayerInfo usage in related components
rg "PlayerInfo" --type typescript
# Check for any remaining useEntitiesUtils imports that might need updating
rg "useEntitiesUtils" --type typescript
Length of output: 214
Script:
#!/bin/bash
# Search for Player[] type usage in .ts and .tsx files
rg "Player\[\]" -g "*.{ts,tsx}"
# Check PlayerInfo type usage
rg "PlayerInfo" -g "*.{ts,tsx}"
# Look for any remaining useEntitiesUtils
rg "useEntitiesUtils" -g "*.{ts,tsx}"
# Check getEntityName imports and usage
rg "getEntityName" -g "*.{ts,tsx}"
# Search for potential type definitions
ast-grep --pattern 'type $_ = Player[]'
ast-grep --pattern 'interface $_ { $$$: Player[] }'
Length of output: 10044
| if (!realmInfo) return; | ||
|
|
There was a problem hiding this comment.
Ensure isLoading is set to false when realmInfo is not available.
If realmInfo is not available, the function returns early without resetting isLoading, leaving the component in a loading state indefinitely.
Apply this diff to fix the issue:
const levelUpRealm = async () => {
setIsLoading(true);
if (!realmInfo) {
+ setIsLoading(false);
return;
}
await dojo.setup.systemCalls.upgrade_realm({
signer: dojo.account.account,
realm_entity_id: realmInfo.entityId,
});
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.
| if (!realmInfo) return; | |
| const levelUpRealm = async () => { | |
| setIsLoading(true); | |
| if (!realmInfo) { | |
| setIsLoading(false); | |
| return; | |
| } | |
| await dojo.setup.systemCalls.upgrade_realm({ | |
| signer: dojo.account.account, | |
| realm_entity_id: realmInfo.entityId, | |
| }); | |
| setIsLoading(false); | |
| }; |
| leaderboardManager.getContributionsTotalPercentage( | ||
| hyperstructureEntityId, | ||
| resourceContributions[playerAddress], | ||
| ) * 100, |
There was a problem hiding this comment.
💡 Codebase verification
Add null check to prevent NaN in percentage calculation
The current implementation could return NaN if totalHyperstructureContribution is zero. The suggested fix using || 0 is appropriate as it will handle this edge case and any other potential undefined/NaN scenarios, while also improving code readability.
🔗 Analysis chain
Handle potential undefined values in percentage calculation.
Ensure that getContributionsTotalPercentage returns a valid number to prevent potential issues when multiplying by 100.
Apply this diff to add a default value:
- percentage:
- leaderboardManager.getContributionsTotalPercentage(
- hyperstructureEntityId,
- resourceContributions[playerAddress],
- ) * 100,
+ const totalPercentage = leaderboardManager.getContributionsTotalPercentage(
+ hyperstructureEntityId,
+ resourceContributions[playerAddress],
+ ) || 0;
+ percentage: totalPercentage * 100,🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Find implementation of getContributionsTotalPercentage
ast-grep --pattern 'getContributionsTotalPercentage($$$) {
$$$
}'
# Search for similar percentage calculations
rg "getContributionsTotalPercentage.*\*\s*100" -A 2
# Look for the leaderboardManager class definition
ast-grep --pattern 'class $_ implements LeaderboardManager {
$$$
}'
Length of output: 195
Script:
#!/bin/bash
# Find any mentions of getContributionsTotalPercentage
rg "getContributionsTotalPercentage"
# Find LeaderboardManager interface or class
rg "LeaderboardManager"
# Find files that might contain the implementation
fd -e ts -e tsx "leaderboard"
Length of output: 4613
Script:
#!/bin/bash
# Get the implementation of getContributionsTotalPercentage from LeaderboardManager
cat client/sdk/packages/eternum/src/modelManager/LeaderboardManager.ts | grep -A 10 "getContributionsTotalPercentage"
# Get the implementation from LeaderboardUtils
cat client/sdk/packages/eternum/src/modelManager/utils/LeaderboardUtils.ts | grep -A 15 "getContributionsTotalPercentage"
Length of output: 1522
| setSelectedResourceBalance( | ||
| divideByPrecision(getBalance(entityId, Number(resourceId), dojo.setup.components).balance), | ||
| ); | ||
| }, [resourceId, getBalance, entityId]); | ||
|
|
There was a problem hiding this comment.
Include dojo.setup.components in useEffect dependencies.
Add dojo.setup.components to the dependency array to ensure selectedResourceBalance updates correctly when components change.
Apply this diff to fix the dependencies:
}, [resourceId, getBalance, entityId]);
+}, [resourceId, getBalance, entityId, dojo.setup.components]);📝 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.
| setSelectedResourceBalance( | |
| divideByPrecision(getBalance(entityId, Number(resourceId), dojo.setup.components).balance), | |
| ); | |
| }, [resourceId, getBalance, entityId]); | |
| setSelectedResourceBalance( | |
| divideByPrecision(getBalance(entityId, Number(resourceId), dojo.setup.components).balance), | |
| ); | |
| }, [resourceId, getBalance, entityId, dojo.setup.components]); | |
|
|
||
| return fragmentMines; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add safety checks and simplify the transformation logic.
The implementation has several potential safety issues:
- Unsafe optional chaining with
fragmentMine! - Potential undefined values in nested property access
- Complex transformation logic that could be simplified
Consider applying these improvements:
export const useFragmentMines = () => {
// ... existing setup code ...
const fragmentMines = useEntityQuery([Has(Structure), HasValue(Structure, { category: "FragmentMine" })]).map(
(fragmentMineEntityId) => {
const fragmentMine = getComponentValue(Structure, fragmentMineEntityId);
+ if (!fragmentMine?.entity_id) return null;
const position = getComponentValue(Position, fragmentMineEntityId);
const entityName = getComponentValue(EntityName, fragmentMineEntityId);
+ const entityIdKeys = [BigInt(fragmentMine.entity_id)];
const owner = toHexString(
- getComponentValue(Owner, getEntityIdFromKeys([BigInt(fragmentMine!.entity_id)]))?.address || 0n,
+ getComponentValue(Owner, getEntityIdFromKeys(entityIdKeys))?.address ?? 0n,
);
+ const buildingQuery = runQuery([HasValue(Building, { outer_entity_id: fragmentMine.entity_id })]);
+ const buildingId = buildingQuery.values().next().value ?? ("0" as Entity);
const building = getComponentValue(
Building,
- runQuery([HasValue(Building, { outer_entity_id: fragmentMine!.entity_id })])
- .values()
- .next().value ?? ("0" as Entity),
+ buildingId,
);
+ const productionQuery = runQuery([HasValue(Production, { entity_id: fragmentMine.entity_id })]);
+ const productionId = productionQuery.values().next().value ?? ("0" as Entity);
const production = getComponentValue(
Production,
- runQuery([HasValue(Production, { entity_id: fragmentMine!.entity_id })])
- .values()
- .next().value ?? ("0" as Entity),
+ productionId,
);
return {
...fragmentMine,
...position,
...building,
...production,
owner,
name: entityName
? shortString.decodeShortString(entityName.name.toString())
- : `FragmentMine ${fragmentMine?.entity_id}`,
+ : `FragmentMine ${fragmentMine.entity_id}`,
};
- },
+ }).filter(Boolean),
);
return fragmentMines;
};📝 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.
| return fragmentMines; | |
| export const useFragmentMines = () => { | |
| // ... existing setup code ... | |
| const fragmentMines = useEntityQuery([Has(Structure), HasValue(Structure, { category: "FragmentMine" })]).map( | |
| (fragmentMineEntityId) => { | |
| const fragmentMine = getComponentValue(Structure, fragmentMineEntityId); | |
| if (!fragmentMine?.entity_id) return null; | |
| const position = getComponentValue(Position, fragmentMineEntityId); | |
| const entityName = getComponentValue(EntityName, fragmentMineEntityId); | |
| const entityIdKeys = [BigInt(fragmentMine.entity_id)]; | |
| const owner = toHexString( | |
| getComponentValue(Owner, getEntityIdFromKeys(entityIdKeys))?.address ?? 0n, | |
| ); | |
| const buildingQuery = runQuery([HasValue(Building, { outer_entity_id: fragmentMine.entity_id })]); | |
| const buildingId = buildingQuery.values().next().value ?? ("0" as Entity); | |
| const building = getComponentValue( | |
| Building, | |
| buildingId, | |
| ); | |
| const productionQuery = runQuery([HasValue(Production, { entity_id: fragmentMine.entity_id })]); | |
| const productionId = productionQuery.values().next().value ?? ("0" as Entity); | |
| const production = getComponentValue( | |
| Production, | |
| productionId, | |
| ); | |
| return { | |
| ...fragmentMine, | |
| ...position, | |
| ...building, | |
| ...production, | |
| owner, | |
| name: entityName | |
| ? shortString.decodeShortString(entityName.name.toString()) | |
| : `FragmentMine ${fragmentMine.entity_id}`, | |
| }; | |
| }).filter(Boolean); | |
| return fragmentMines; | |
| }; |
| // todo: pay attention to expensive query | ||
| const realms = useRealms(); |
There was a problem hiding this comment.
💡 Codebase verification
Consider implementing pagination or virtual scrolling for realm queries
The TODO comment flags a legitimate performance concern. The current implementation:
- Fetches all realms at once without pagination
- Performs multiple component lookups per realm
- Processes each realm with expensive operations
Suggested improvements:
- Implement pagination or virtual scrolling to limit the number of realms loaded at once
- Consider implementing a more efficient batch lookup for realm components
- Cache processed realm data to avoid repeated transformations
🔗 Analysis chain
Address the TODO comment about query performance.
The comment indicates a potential performance issue with the query. This should be investigated and documented properly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances of useRealms to understand the impact
rg "useRealms\(" --type ts
Length of output: 122
Script:
#!/bin/bash
# Search for useRealms hook definition
ast-grep --pattern 'export function useRealms() {
$$$
}'
# Also search for any other exports of useRealms
rg "export.*useRealms" --type ts
# Look for the underlying query implementation
rg "getRealms|fetchRealms|queryRealms" --type ts
Length of output: 335
Script:
#!/bin/bash
# Get the hook implementation
cat client/apps/game/src/hooks/helpers/use-realm.tsx
# Get the getRealms implementation
cat client/apps/game/src/utils/realm.ts
Length of output: 11630
| () => | ||
| dynamic.map( | ||
| (resourceId): Resource => ({ | ||
| resourceId, | ||
| amount: getBalance(entityId, resourceId, dojo.setup.components).balance, | ||
| }), | ||
| ), | ||
| [dynamic, entityId, getBalance], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove getBalance from the dependency array.
The getBalance function is imported and its reference won't change during the component's lifecycle. Including it in the dependency array could cause unnecessary re-renders.
- [dynamic, entityId, getBalance],
+ [dynamic, entityId],📝 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.
| () => | |
| dynamic.map( | |
| (resourceId): Resource => ({ | |
| resourceId, | |
| amount: getBalance(entityId, resourceId, dojo.setup.components).balance, | |
| }), | |
| ), | |
| [dynamic, entityId, getBalance], | |
| () => | |
| dynamic.map( | |
| (resourceId): Resource => ({ | |
| resourceId, | |
| amount: getBalance(entityId, resourceId, dojo.setup.components).balance, | |
| }), | |
| ), | |
| [dynamic, entityId], |
| @@ -29,7 +29,7 @@ export const EntitiesArmyTable = () => { | |||
| </div> | |||
| <BattleSimulation /> | |||
| <PillageSimulation /> | |||
| {playerStructures().map((entity: any, index: number) => { | |||
| {playerStructures.map((entity: any, index: number) => { | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove any type and use proper typing.
Using any type defeats TypeScript's type checking. Consider using the correct type from @bibliothecadao/eternum.
- {playerStructures.map((entity: any, index: number) => {
+ {playerStructures.map((entity: PlayerStructure, index: number) => {Committable suggestion skipped: line range outside the PR's diff.
| import { getEntityInfo } from "@/utils/entities"; | ||
| import { ArmyInfo, BattleManager, ContractAddress } from "@bibliothecadao/eternum"; |
There was a problem hiding this comment.
LGTM! Consistent refactoring with improved error handling.
The changes maintain functionality while adding proper null handling for battle conditions.
However, there's a potential issue with the dependencies array:
The getEntityInfo function is listed in the dependencies array of useCallback but it's imported from a module. Remove it from the dependencies as it won't change between renders:
useCallback(
(army: ArmyInfo, index: number) => {
// ... function body ...
},
[
nextBlockTimestamp,
ownArmySelected,
ownArmySelected?.entity_id,
position,
- getEntityInfo,
useIsStructureImmune,
setBattleView,
setTooltip,
structureAtPosition,
ownArmyIsImmune,
dojo,
],
);Also applies to: 30-34, 48-52
| useEffect(() => { | ||
| setPlayers(getPlayers()); | ||
| setPlayerInfo(getPlayerInfo(players, ContractAddress(account.address), components)); | ||
| setIsLoading(false); | ||
| }, [playersByRank]); |
There was a problem hiding this comment.
Fix incomplete useEffect dependencies.
The useEffect hook is missing dependencies that it uses within its callback. Add players, account, and components to the dependency array to prevent stale closures.
useEffect(() => {
setPlayerInfo(getPlayerInfo(players, ContractAddress(account.address), components));
setIsLoading(false);
- }, [playersByRank]);
+ }, [playersByRank, players, account.address, components]);📝 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.
| useEffect(() => { | |
| setPlayers(getPlayers()); | |
| setPlayerInfo(getPlayerInfo(players, ContractAddress(account.address), components)); | |
| setIsLoading(false); | |
| }, [playersByRank]); | |
| useEffect(() => { | |
| setPlayerInfo(getPlayerInfo(players, ContractAddress(account.address), components)); | |
| setIsLoading(false); | |
| }, [playersByRank, players, account.address, components]); |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
client/apps/game/src/utils/entities.ts (5)
16-31: Consider adding input validation and error handling.The function could benefit from additional error handling:
- Validate entityId before conversion to BigInt
- Add null checks for position and owner values
export const getRealm = (entityId: ID, components: ClientComponents) => { + if (!entityId || entityId <= 0) return undefined; const { Realm, Owner, Position } = components; const entity = getEntityIdFromKeys([BigInt(entityId)]); const realm = getComponentValue(Realm, entity); if (!realm) return undefined; const position = getComponentValue(Position, entity); const owner = getComponentValue(Owner, entity); + if (!position || !owner) return undefined; return { ...realm, position, name: getRealmNameById(realm.realm_id), owner, } as RealmWithPosition; };
52-55: Consider breaking down the complex component destructuring.The large number of destructured components makes the function harder to maintain. Consider grouping related components into separate objects.
- const { ArrivalTime, Movable, CapacityCategory, CapacityConfig, EntityOwner, Owner, Structure, Army, Position } = - components; + const movementComponents = { ArrivalTime, Movable, Position } = components; + const capacityComponents = { CapacityCategory, CapacityConfig } = components; + const ownershipComponents = { EntityOwner, Owner } = components; + const entityComponents = { Structure, Army } = components;
118-123: Move abbreviations to constants.The abbreviations object should be defined as a constant outside the function to improve maintainability and reusability.
+const STRUCTURE_ABBREVIATIONS: Record<string, string> = { + FragmentMine: "FM", + Hyperstructure: "HS", + Bank: "BK", +}; export const getEntityName = (entityId: ID, components: ClientComponents, abbreviate: boolean = false) => { // ... if (abbreviate && structure) { - const abbreviations: Record<string, string> = { - FragmentMine: "FM", - Hyperstructure: "HS", - Bank: "BK", - }; - const abbr = abbreviations[structure.category]; + const abbr = STRUCTURE_ABBREVIATIONS[structure.category];
137-157: Add JSDoc documentation and consistent error handling.These utility functions would benefit from clear documentation of their purpose and return types, along with consistent error handling.
+/** + * Retrieves the name associated with a contract address + * @param address - The contract address to look up + * @param components - Client components containing the AddressName component + * @returns The name string if found, undefined otherwise + */ export const getAddressName = (address: ContractAddress, components: ClientComponents) => { + if (!address) return undefined; const addressName = getComponentValue(components.AddressName, getEntityIdFromKeys([BigInt(address)])); return addressName ? addressName.name.toString() : undefined; }; +/** + * Retrieves the name associated with an entity's owner address + * @param entityId - The entity ID to look up + * @param components - Client components containing required components + * @returns The owner's name string if found, undefined otherwise + */ export const getAddressNameFromEntity = (entityId: ID, components: ClientComponents) => { + if (!entityId) return undefined; const address = getPlayerAddressFromEntity(entityId, components); if (!address) return; const addressName = getComponentValue(components.AddressName, getEntityIdFromKeys([BigInt(address)])); return addressName ? addressName.name.toString() : undefined; };
1-157: Consider organizing utilities into logical groups.The file contains well-structured utilities, but could benefit from organizing related functions into separate modules:
- Entity retrieval (getRealm, getStructure)
- Entity information (getEntityInfo, getEntityName)
- Address management (getAddressName, getAddressNameFromEntity, getPlayerAddressFromEntity)
This would improve maintainability and make the code more modular.
client/apps/game/src/utils/realm.ts (2)
8-19: Add error handling and improve type safety.The function could benefit from the following improvements:
- Add explicit return type annotation
- Handle potential shortString decoding errors
- Consider returning null/undefined instead of empty string for better error handling
Consider this implementation:
-export const getRealmAddressName = (realmEntityId: ID, components: ClientComponents) => { +export const getRealmAddressName = (realmEntityId: ID, components: ClientComponents): string | null => { const owner = getComponentValue(components.Owner, getEntityIdFromKeys([BigInt(realmEntityId)])); const addressName = owner ? getComponentValue(components.AddressName, getEntityIdFromKeys([owner.address])) : undefined; if (addressName) { + try { return shortString.decodeShortString(String(addressName.name)); + } catch (error) { + console.error(`Failed to decode address name: ${error}`); + return null; + } } else { - return ""; + return null; } };
43-72: Improve type safety and readability of getRealmInfo.The function could benefit from better type safety and readability improvements:
- Type-safe population spreading
- Document magic numbers
- Break down complex calculations
export function getRealmInfo(entity: Entity, components: ClientComponents): RealmInfo | undefined { const realm = getComponentValue(components.Realm, entity); const owner = getComponentValue(components.Owner, entity); const position = getComponentValue(components.Position, entity); const population = getComponentValue(components.Population, entity); if (realm && owner && position) { const { realm_id, entity_id, produced_resources, order, level } = realm; const name = getRealmNameById(realm_id); const { address } = owner; + // Extract population-related calculations + const BASE_POPULATION_CAPACITY = configManager.getBasePopulationCapacity(); + const currentPopulation = population?.population ?? 0; + const populationCapacity = population?.capacity ?? 0; + const totalCapacity = populationCapacity + BASE_POPULATION_CAPACITY; + const hasAvailableCapacity = !population || totalCapacity > currentPopulation; return { realmId: realm_id, entityId: entity_id, name, level, resourceTypesPacked: produced_resources, order, position, - ...population, + population: currentPopulation, + capacity: populationCapacity, - hasCapacity: - !population || population.capacity + configManager.getBasePopulationCapacity() > population.population, + hasCapacity: hasAvailableCapacity, owner: address, ownerName: getRealmAddressName(realm.entity_id, components), hasWonder: realm.has_wonder, }; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/apps/game/src/utils/entities.ts(1 hunks)client/apps/game/src/utils/realm.ts(1 hunks)
🔇 Additional comments (2)
client/apps/game/src/utils/entities.ts (1)
132-135: LGTM!The function is well-structured and handles the wonder case appropriately.
client/apps/game/src/utils/realm.ts (1)
1-7: LGTM! Well-structured imports.The imports are well-organized and properly typed, following TypeScript best practices.
| const position = getComponentValue(Position, entity); | ||
| const owner = getComponentValue(Owner, entity); | ||
| const structureName = getEntityName(structure.entity_id, components); | ||
| const name = structureName ? `${structure?.category} ${structureName}` : structure.category || ""; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle potential undefined values in name construction.
The current implementation might result in "undefined undefined" if both structureName and structure.category are undefined.
- const name = structureName ? `${structure?.category} ${structureName}` : structure.category || "";
+ const category = structure?.category || "";
+ const name = structureName ? `${category} ${structureName}` : category;📝 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 name = structureName ? `${structure?.category} ${structureName}` : structure.category || ""; | |
| const category = structure?.category || ""; | |
| const name = structureName ? `${category} ${structureName}` : category; |
| const entityIdBigInt = BigInt(entityId); | ||
| const arrivalTime = getComponentValue(ArrivalTime, getEntityIdFromKeys([entityIdBigInt])); | ||
| const movable = getComponentValue(Movable, getEntityIdFromKeys([entityIdBigInt])); | ||
|
|
||
| const entityCapacityCategory = getComponentValue(CapacityCategory, getEntityIdFromKeys([entityIdBigInt])) | ||
| ?.category as unknown as string; | ||
| const capacityCategoryId = CAPACITY_CONFIG_CATEGORY_STRING_MAP[entityCapacityCategory] || 0n; | ||
| const capacity = getComponentValue(CapacityConfig, getEntityIdFromKeys([BigInt(capacityCategoryId)])); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize repeated BigInt conversions and improve type safety.
The entityId is converted to BigInt multiple times. Consider converting once and reusing the value.
+ if (!entityId) return undefined;
const entityIdBigInt = BigInt(entityId);
+ const entityKeys = getEntityIdFromKeys([entityIdBigInt]);
- const arrivalTime = getComponentValue(ArrivalTime, getEntityIdFromKeys([entityIdBigInt]));
- const movable = getComponentValue(Movable, getEntityIdFromKeys([entityIdBigInt]));
+ const arrivalTime = getComponentValue(ArrivalTime, entityKeys);
+ const movable = getComponentValue(Movable, entityKeys);📝 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 entityIdBigInt = BigInt(entityId); | |
| const arrivalTime = getComponentValue(ArrivalTime, getEntityIdFromKeys([entityIdBigInt])); | |
| const movable = getComponentValue(Movable, getEntityIdFromKeys([entityIdBigInt])); | |
| const entityCapacityCategory = getComponentValue(CapacityCategory, getEntityIdFromKeys([entityIdBigInt])) | |
| ?.category as unknown as string; | |
| const capacityCategoryId = CAPACITY_CONFIG_CATEGORY_STRING_MAP[entityCapacityCategory] || 0n; | |
| const capacity = getComponentValue(CapacityConfig, getEntityIdFromKeys([BigInt(capacityCategoryId)])); | |
| if (!entityId) return undefined; | |
| const entityIdBigInt = BigInt(entityId); | |
| const entityKeys = getEntityIdFromKeys([entityIdBigInt]); | |
| const arrivalTime = getComponentValue(ArrivalTime, entityKeys); | |
| const movable = getComponentValue(Movable, entityKeys); | |
| const entityCapacityCategory = getComponentValue(CapacityCategory, getEntityIdFromKeys([entityIdBigInt])) | |
| ?.category as unknown as string; | |
| const capacityCategoryId = CAPACITY_CONFIG_CATEGORY_STRING_MAP[entityCapacityCategory] || 0n; | |
| const capacity = getComponentValue(CapacityConfig, getEntityIdFromKeys([BigInt(capacityCategoryId)])); |
| const getRealmNameById = (realmId: ID): string => { | ||
| const features = realmsJson["features"][realmId - 1]; | ||
| if (!features) return ""; | ||
| return features["name"]; | ||
| }; |
There was a problem hiding this comment.
Add bounds checking and improve type safety.
The current implementation could lead to runtime errors due to unsafe array access and lack of bounds checking.
-const getRealmNameById = (realmId: ID): string => {
+const getRealmNameById = (realmId: ID): string => {
+ if (realmId <= 0) {
+ console.warn(`Invalid realmId: ${realmId}`);
+ return "";
+ }
+
+ const index = Number(realmId) - 1;
+ if (index >= realmsJson.features.length) {
+ console.warn(`RealmId ${realmId} exceeds realm count`);
+ return "";
+ }
+
- const features = realmsJson["features"][realmId - 1];
+ const features = realmsJson.features[index];
if (!features) return "";
- return features["name"];
+ return features.name || "";
};📝 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 getRealmNameById = (realmId: ID): string => { | |
| const features = realmsJson["features"][realmId - 1]; | |
| if (!features) return ""; | |
| return features["name"]; | |
| }; | |
| const getRealmNameById = (realmId: ID): string => { | |
| if (realmId <= 0) { | |
| console.warn(`Invalid realmId: ${realmId}`); | |
| return ""; | |
| } | |
| const index = Number(realmId) - 1; | |
| if (index >= realmsJson.features.length) { | |
| console.warn(`RealmId ${realmId} exceeds realm count`); | |
| return ""; | |
| } | |
| const features = realmsJson.features[index]; | |
| if (!features) return ""; | |
| return features.name || ""; | |
| }; |
| interface Attribute { | ||
| trait_type: string; | ||
| value: any; | ||
| } | ||
|
|
||
| let realms: { | ||
| [key: string]: any; | ||
| } = {}; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace 'any' types with more specific types.
Using any reduces TypeScript's type safety benefits. Consider defining more specific types:
interface Attribute {
trait_type: string;
- value: any;
+ value: string | number | boolean; // or more specific union type based on possible values
}
let realms: {
- [key: string]: any;
+ [key: string]: {
+ features?: {
+ name: string;
+ // Add other known properties
+ }[];
+ // Add other known properties
+ };
} = {};Committable suggestion skipped: line range outside the PR's diff.
| const loadRealms = async () => { | ||
| const response = await fetch("/jsons/realms.json"); | ||
| realms = await response.json(); | ||
| }; | ||
|
|
||
| loadRealms(); |
There was a problem hiding this comment.
Improve error handling and configuration in loadRealms.
The current implementation has several potential issues:
- No error handling for failed fetch requests
- Hardcoded path should be configurable
- Immediate invocation could cause issues in testing environments
Consider this improved implementation:
+const REALMS_JSON_PATH = process.env.REALMS_JSON_PATH || '/jsons/realms.json';
+const MAX_RETRIES = 3;
-const loadRealms = async () => {
+export const loadRealms = async (retries = MAX_RETRIES): Promise<void> => {
try {
- const response = await fetch("/jsons/realms.json");
+ const response = await fetch(REALMS_JSON_PATH);
+ if (!response.ok) {
+ throw new Error(`HTTP error! status: ${response.status}`);
+ }
realms = await response.json();
+ } catch (error) {
+ console.error(`Failed to load realms: ${error}`);
+ if (retries > 0) {
+ console.log(`Retrying... (${retries} attempts remaining)`);
+ await new Promise(resolve => setTimeout(resolve, 1000));
+ return loadRealms(retries - 1);
+ }
+ throw error;
}
};
-loadRealms();
+// Consider moving this to your app's initialization logic
+// if (process.env.NODE_ENV !== 'test') {
+// loadRealms().catch(console.error);
+// }📝 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 loadRealms = async () => { | |
| const response = await fetch("/jsons/realms.json"); | |
| realms = await response.json(); | |
| }; | |
| loadRealms(); | |
| const REALMS_JSON_PATH = process.env.REALMS_JSON_PATH || '/jsons/realms.json'; | |
| const MAX_RETRIES = 3; | |
| export const loadRealms = async (retries = MAX_RETRIES): Promise<void> => { | |
| try { | |
| const response = await fetch(REALMS_JSON_PATH); | |
| if (!response.ok) { | |
| throw new Error(`HTTP error! status: ${response.status}`); | |
| } | |
| realms = await response.json(); | |
| } catch (error) { | |
| console.error(`Failed to load realms: ${error}`); | |
| if (retries > 0) { | |
| console.log(`Retrying... (${retries} attempts remaining)`); | |
| await new Promise(resolve => setTimeout(resolve, 1000)); | |
| return loadRealms(retries - 1); | |
| } | |
| throw error; | |
| } | |
| }; | |
| // Consider moving this to your app's initialization logic | |
| // if (process.env.NODE_ENV !== 'test') { | |
| // loadRealms().catch(console.error); | |
| // } |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
client/apps/game/src/utils/players.ts (1)
18-34: Consider adding error handling and optimizing queries.A few suggestions for improvement:
- The
runQueryforisAlivecheck could be cached if used frequently- Add error handling for
getComponentValueand guild name resolution- Consider logging or handling edge cases when guild data is missing
const playerInfo = players .map((player) => { + try { const isAlive = !!runQuery([HasValue(Owner, { address: player.address })]).size; const guildMember = getComponentValue(GuildMember, player.entity); - const guildName = guildMember ? getEntityName(guildMember.guild_entity_id, components) : ""; + const guildName = guildMember ? getEntityName(guildMember.guild_entity_id, components) : null; return { entity: player.entity, address: player.address, name: player.name, isAlive, - guildName, + guildName: guildName ?? "", }; + } catch (error) { + console.error(`Failed to process player ${player.address}:`, error); + return undefined; + } }) .filter((player) => player !== undefined);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
client/apps/game/src/utils/players.ts(1 hunks)
🔇 Additional comments (2)
client/apps/game/src/utils/players.ts (2)
1-6: LGTM! Well-organized imports.The imports are logically organized and include all necessary dependencies for the function's operation.
7-12: LGTM! Well-structured function signature.The function signature is well-typed and the component destructuring improves readability.
| const playersByRank = useLeaderBoardStore((state) => state.playersByRank); | ||
|
|
||
| const totalPoints = playersByRank.reduce((sum, [, points]) => sum + points, 0); |
There was a problem hiding this comment.
Critical: Hook usage in utility function.
The useLeaderBoardStore hook is being called inside a utility function. This violates React's rules of hooks, which state that hooks must be called from either React function components or custom hooks. This could lead to runtime errors.
Consider refactoring to pass the leaderboard data as a parameter:
export const getPlayerInfo = (
players: Player[],
playerAddress: ContractAddress,
components: ClientComponents,
+ playersByRank: [string, number][],
): PlayerInfo[] => {
const { Realm, Owner, GuildMember, Hyperstructure, Structure } = components;
- const playersByRank = useLeaderBoardStore((state) => state.playersByRank);Committable suggestion skipped: line range outside the PR's diff.
| let unrankedCount = 0; | ||
|
|
||
| return playerInfo.map((player) => { | ||
| const rankIndex = playersByRank.findIndex(([address]) => address === player.address); | ||
| if (rankIndex === -1) unrankedCount++; | ||
|
|
||
| const points = rankIndex === -1 ? 0 : playersByRank[rankIndex][1]; | ||
|
|
||
| return { | ||
| entity: player.entity, | ||
| name: player.name, | ||
| address: player.address, | ||
| points, | ||
| rank: rankIndex === -1 ? Number.MAX_SAFE_INTEGER : rankIndex + 1, | ||
| percentage: calculatePlayerSharePercentage(points, totalPoints), | ||
| lords: 0, | ||
| realms: runQuery([Has(Realm), HasValue(Owner, { address: player.address })]).size, | ||
| mines: runQuery([ | ||
| HasValue(Structure, { category: StructureType[StructureType.FragmentMine] }), | ||
| HasValue(Owner, { address: player.address }), | ||
| ]).size, | ||
| hyperstructures: runQuery([Has(Hyperstructure), HasValue(Owner, { address: player.address })]).size, | ||
| isAlive: player.isAlive, | ||
| guildName: player.guildName || "", | ||
| isUser: player.address === playerAddress, | ||
| }; | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize queries and add validation.
The current implementation has several areas for improvement:
- Multiple separate
runQuerycalls could be combined or cached - Consider validating points and percentages
- The rank calculation could be more explicit
+ const playerQueries = runQuery([
+ Has(Realm, Hyperstructure, Structure),
+ HasValue(Owner, { address: player.address }),
+ ]).reduce((acc, entity) => {
+ if (Has(Realm)(entity)) acc.realms++;
+ if (Has(Hyperstructure)(entity)) acc.hyperstructures++;
+ if (HasValue(Structure, { category: StructureType[StructureType.FragmentMine] })(entity)) acc.mines++;
+ return acc;
+ }, { realms: 0, mines: 0, hyperstructures: 0 });
return {
// ... other fields ...
- points,
+ points: Math.max(0, points),
- percentage: calculatePlayerSharePercentage(points, totalPoints),
+ percentage: Math.min(100, Math.max(0, calculatePlayerSharePercentage(points, totalPoints))),
- realms: runQuery([Has(Realm), HasValue(Owner, { address: player.address })]).size,
- mines: runQuery([
- HasValue(Structure, { category: StructureType[StructureType.FragmentMine] }),
- HasValue(Owner, { address: player.address }),
- ]).size,
- hyperstructures: runQuery([Has(Hyperstructure), HasValue(Owner, { address: player.address })]).size,
+ ...playerQueries,Committable suggestion skipped: line range outside the PR's diff.
Summary by CodeRabbit
Release Notes
New Features
usePlayersto retrieve and process player data.armyHasTroopsandarmyHasTraveled.getRealm,getStructure,getEntityInfo,getEntityName,getRealmName,getAddressName,getAddressNameFromEntity, andgetPlayerAddressFromEntity.getRealmAddressNameandgetRealmNameByIdfor improved realm management.Improvements
componentsinto various function calls.Changes
useGetAllPlayershook in favor ofusePlayersfor direct player data access.LeaderboardManager.Refactoring