Enh/wonders #2367
Enh/wonders #2367
Conversation
…in names + minimapAdd wonder in names + minimapAdd wonder in names + minimapAdd wonder in names + minimapAdd wonder in names + minimapAdd wonder in names + minimapAdd wonder in names + minimap
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@ponderingdemocritus has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 56 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 (5)
WalkthroughThis pull request introduces multiple modifications across several files, primarily focusing on enhancing the handling of realm and structure names, as well as the integration of a new Changes
Possibly related PRs
Suggested reviewers
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 (
|
|
You are out of MentatBot reviews. Your usage will refresh December 16 at 08:00 AM. |
|
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: 0
🧹 Outside diff range and nitpick comments (3)
client/src/types/index.ts (1)
36-36: LGTM! Consider adding JSDoc documentation.The
hasWonderproperty addition maintains type consistency withStructureSystemUpdate. Consider adding JSDoc documentation to clarify its purpose.Add documentation like this:
+ /** Indicates whether the structure has a wonder associated with it */ hasWonder: boolean;client/src/hooks/helpers/useEntities.tsx (1)
256-275: Improve abbreviation handling with enum mapping.The abbreviation logic is well-structured but could be more maintainable by using an enum mapping.
Consider this refactoring to improve type safety and maintainability:
- const abbreviations: Record<string, string> = { - [StructureType[StructureType.FragmentMine]]: "FM", - [StructureType[StructureType.Hyperstructure]]: "HS", - [StructureType[StructureType.Bank]]: "BK", - }; + const STRUCTURE_ABBREVIATIONS = { + [StructureType.FragmentMine]: "FM", + [StructureType.Hyperstructure]: "HS", + [StructureType.Bank]: "BK", + } as const; + + const structureTypeKey = structure.category as keyof typeof StructureType; + const structureType = StructureType[structureTypeKey]; + const abbr = STRUCTURE_ABBREVIATIONS[structureType];client/src/three/components/Minimap.ts (1)
254-260: Consider simplifying the label selection logicThe nested conditionals could be simplified for better readability and maintainability.
Consider refactoring to:
- if (structure.isMine) { - labelImg = structure.hasWonder ? this.labelImages.get("MY_REALM_WONDER") : this.labelImages.get("MY_REALM"); - } else { - labelImg = structure.hasWonder - ? this.labelImages.get("REALM_WONDER") - : this.labelImages.get(`STRUCTURE_${structureType}`); - } + const labelKey = structure.isMine + ? (structure.hasWonder ? "MY_REALM_WONDER" : "MY_REALM") + : (structure.hasWonder ? "REALM_WONDER" : `STRUCTURE_${structureType}`); + labelImg = this.labelImages.get(labelKey);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
client/public/textures/my_realm_wonder_label.pngis excluded by!**/*.pngclient/public/textures/realm_wonder_label.pngis excluded by!**/*.png
📒 Files selected for processing (12)
client/src/hooks/helpers/useEntities.tsx(4 hunks)client/src/hooks/helpers/useRealm.tsx(5 hunks)client/src/hooks/helpers/useStructures.tsx(1 hunks)client/src/three/components/Minimap.ts(3 hunks)client/src/three/components/StructureManager.ts(4 hunks)client/src/three/systems/SystemManager.ts(2 hunks)client/src/three/systems/types.ts(1 hunks)client/src/types/index.ts(1 hunks)client/src/ui/elements/ResourceCost.tsx(1 hunks)client/src/ui/modules/social/PlayerId.tsx(1 hunks)client/src/ui/utils/realms.tsx(2 hunks)sdk/packages/eternum/src/provider/index.ts(0 hunks)
💤 Files with no reviewable changes (1)
- sdk/packages/eternum/src/provider/index.ts
✅ Files skipped from review due to trivial changes (1)
- client/src/ui/modules/social/PlayerId.tsx
🔇 Additional comments (14)
client/src/three/systems/types.ts (1)
23-23: LGTM! Verify consistent usage across the codebase.
The addition of hasWonder property is well-typed and aligns with the PR objectives.
Let's verify the consistent usage of this property across the codebase:
✅ Verification successful
Let me search for actual usage of the hasWonder property to verify its implementation.
Property is consistently implemented across the codebase
The hasWonder property is properly integrated and used consistently throughout the codebase:
- Correctly propagated from realm data in
SystemManager.ts - Properly handled in structure updates within
StructureManager.ts - Appropriately used in the minimap rendering logic
- Consistently typed in related interfaces
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for structure update handling to ensure hasWonder is properly handled
ast-grep --pattern 'StructureSystemUpdate' -A 5
Length of output: 1169
Script:
#!/bin/bash
# Search for hasWonder property usage
rg "hasWonder" -A 3
# Also search for structure update handling implementations
ast-grep --pattern 'function $_(update: StructureSystemUpdate)'
Length of output: 4258
client/src/ui/elements/ResourceCost.tsx (1)
61-61: Verify the need for increased decimal precision.
Increasing maximumFractionDigits from 1 to 6 significantly affects number display precision. This could impact readability for users.
Let's check where this component is used with wonder-related calculations:
✅ Verification successful
Based on the analysis of the codebase, I can now generate the final response:
The increased decimal precision is necessary for accurate resource calculations.
The codebase shows that resources are handled with high precision internally (using resourcePrecision) and then displayed using divideByPrecision for various calculations including:
- Bank/market operations (swaps, liquidity)
- Resource production and consumption rates
- Trading and exchange calculations
- Wonder-related resource multipliers (3x resources as seen in docs)
The precision increase from 1 to 6 digits ensures accurate representation of these calculations, particularly for:
- Small fractional resource amounts in trading
- Precise resource production rates
- Bank liquidity calculations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for ResourceCost component usage with wonder-related props
rg -A 5 '<ResourceCost.*hasWonder'
# Search for any wonder-related calculations that might require high precision
rg -A 5 'hasWonder.*amount'
Length of output: 66
Script:
#!/bin/bash
# Let's first find where ResourceCost component is used
rg -l '<ResourceCost'
# Then check the actual usage patterns
rg -A 3 '<ResourceCost'
# Also search for wonder-related code near resource calculations
rg -A 5 'wonder.*resource|resource.*wonder' -i
Length of output: 14347
Script:
#!/bin/bash
# Let's check for any decimal calculations or precision handling in the codebase
rg -A 3 'divideByPrecision|currencyFormat'
# Also check for any resource amount calculations that might need high precision
rg -A 3 'amount.*resource|resource.*amount' -i
Length of output: 67270
client/src/ui/utils/realms.tsx (2)
1-4: LGTM!
The import of ComponentValue from @dojoengine/recs is appropriately added to support the new type-safe realm component handling.
81-84: LGTM! Clean implementation of realm name formatting.
The getRealmName function effectively combines the base realm name with wonder status, maintaining a single source of truth by reusing getRealmNameById.
client/src/hooks/helpers/useStructures.tsx (1)
52-52: LGTM! Robust null-safety handling.
The addition of the empty string fallback (|| "") ensures type safety and prevents potential undefined/null assignments to the name property.
client/src/three/components/StructureManager.ts (2)
93-93: LGTM! Clean integration of wonder status.
The hasWonder property is properly destructured and passed through to the structure creation.
Also applies to: 109-109
230-230: LGTM! Consistent structure data enhancement.
The hasWonder property is appropriately added to the structure info object, maintaining consistency with the wonder-related changes across the codebase.
Also applies to: 243-243
client/src/hooks/helpers/useEntities.tsx (1)
114-115: LGTM: Realm name retrieval updated to support wonder status.
The change from getRealmNameById to getRealmName aligns with the new wonder system, ensuring realm names reflect their wonder status appropriately.
client/src/hooks/helpers/useRealm.tsx (2)
32-32: LGTM: RealmInfo interface updated with hasWonder property.
The addition of hasWonder to the RealmInfo interface properly types the new wonder functionality.
241-241: Verify consistent usage of hasWonder property.
The hasWonder property is consistently added to realm objects across multiple functions. Let's verify its usage in the UI components.
Also applies to: 291-291, 338-338
✅ Verification successful
hasWonder property is consistently implemented across the codebase
The property is properly used throughout the codebase with consistent implementation:
- Defined as a boolean type in type definitions (
types/index.tsandthree/systems/types.ts) - Correctly mapped from API's
has_wonderinuseRealm.tsx - Properly utilized in UI components:
- Used in
Minimap.tsfor conditional label rendering - Used in
realms.tsxutils for realm name formatting
- Used in
- Correctly propagated through the system managers and structure components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for UI components using the hasWonder property
# to ensure the property is being utilized appropriately.
# Search for direct usage of hasWonder in UI components
echo "Checking for hasWonder usage in UI components..."
rg -t tsx -t ts "hasWonder" ./client/src/ui/
# Search for realm wonder-related UI elements
echo "Checking for wonder-related UI elements..."
rg -t tsx -t ts "wonder" ./client/src/ui/
Length of output: 344
Script:
#!/bin/bash
# Let's try a different approach without file type restrictions
# First, check for hasWonder usage across the client directory
echo "Checking for hasWonder usage..."
rg "hasWonder" ./client/src/
# Then check for wonder-related UI elements
echo "Checking for wonder-related UI elements..."
rg "wonder" ./client/src/
# Also check for has_wonder (the API format) usage
echo "Checking for has_wonder usage..."
rg "has_wonder" ./client/src/
Length of output: 3219
client/src/three/systems/SystemManager.ts (1)
167-167: LGTM: Structure system properly integrated with wonder system.
The wonder system integration is well-implemented:
- Safe initialization with default
false - Proper type-checking for Realm structures
- Consistent property passing in return object
Also applies to: 171-171, 181-181
client/src/three/components/Minimap.ts (3)
18-19: LGTM: New label constants follow conventions
The new wonder-related label constants follow the existing naming pattern and resource path structure.
464-465: LGTM: Label loading follows existing pattern
The new wonder label loading calls are properly integrated with the existing loading logic.
254-260: 🛠️ Refactor suggestion
Add safety check for hasWonder property
The code assumes the hasWonder property exists on all realm structures. Consider adding a safety check.
Let's verify the structure type definition:
Add a safety check:
+ const hasWonder = 'hasWonder' in structure ? structure.hasWonder : false;
if (structure.isMine) {
- labelImg = structure.hasWonder ? this.labelImages.get("MY_REALM_WONDER") : this.labelImages.get("MY_REALM");
+ labelImg = hasWonder ? this.labelImages.get("MY_REALM_WONDER") : this.labelImages.get("MY_REALM");
} else {
- labelImg = structure.hasWonder
+ labelImg = hasWonder
? this.labelImages.get("REALM_WONDER")
: this.labelImages.get(`STRUCTURE_${structureType}`);
}
|
can you fix build |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (8)
client/src/hooks/helpers/use-get-all-players.tsx (3)
Line range hint
27-63: Consider performance optimizations for data processingThe current implementation performs multiple sequential
runQueryoperations and processes large arrays with map/filter operations. Consider the following optimizations:
- Batch the
runQueryoperations- Memoize the points calculations
- Use
useMemofor expensive computationsExample optimization:
const useMemoizedQueries = (address: string) => { return useMemo(() => ({ realms: runQuery([Has(Realm), HasValue(Owner, { address })]).size, mines: runQuery([ HasValue(Structure, { category: StructureType[StructureType.FragmentMine] }), HasValue(Owner, { address }), ]).size, hyperstructures: runQuery([Has(Hyperstructure), HasValue(Owner, { address })]).size, }), [address]); };
Line range hint
27-40: Add error handling and loggingThe current implementation lacks error handling for several critical operations:
- Silent filtering of undefined players
- No error handling for failed queries
- No validation for address decoding
Consider adding try-catch blocks and logging:
const getPlayers = (): Player[] => { try { const players = Array.from(Array.from(playerEntities)) .map((id) => { try { const addressName = getComponentValue(AddressName, id); if (!addressName) { console.warn(`No AddressName component for id: ${id}`); return; } // ... rest of the code } catch (error) { console.error(`Error processing player ${id}:`, error); return; } }) .filter((player): player is NonNullable<typeof player> => player !== undefined); // ... rest of the code } catch (error) { console.error('Failed to get players:', error); return []; } };
Line range hint
27-63: Enhance type safetyThe code could benefit from stronger type assertions and proper type narrowing:
- Use type predicates in filter functions
- Add explicit return types for all functions
- Consider creating interfaces for intermediate data structures
Example improvements:
interface PlayerData { addressName: string; address: string; isAlive: boolean; guildName: string; } const isValidPlayer = (player: unknown): player is PlayerData => { return player !== undefined && typeof (player as PlayerData).address === 'string' && typeof (player as PlayerData).addressName === 'string'; }; const getPlayers = (): Player[] => { const players = Array.from(playerEntities) .map((id): PlayerData | undefined => { // ... existing code }) .filter(isValidPlayer); // ... rest of the code };client/src/hooks/helpers/useGuilds.tsx (3)
189-189: Consider extracting the fallback value to a constant.The fallback value 'Unknown' is used in multiple places. Consider extracting it to a constant at the file level for better maintainability.
+const UNKNOWN_ENTITY_NAME = 'Unknown'; + export const useGuilds = () => { // ... - (entityId: number) => getEntityName(entityId) || 'Unknown' + (entityId: number) => getEntityName(entityId) || UNKNOWN_ENTITY_NAME
257-257: Consider memoizing the entity name resolver.The entity name resolver function is recreated on every render. Consider memoizing it with useCallback to prevent unnecessary re-renders of child components.
+ const getEntityNameWithFallback = useCallback( + (entityId: number) => getEntityName(entityId) || 'Unknown', + [getEntityName] + ); const useGuildWhitelist = (guildEntityId: ID, players: Player[]) => { // ... - return formatGuildWhitelist(whitelist, players, GuildWhitelist, getAddressName, (entityId: number) => getEntityName(entityId) || 'Unknown'); + return formatGuildWhitelist(whitelist, players, GuildWhitelist, getAddressName, getEntityNameWithFallback); }; const usePlayerWhitelist = (address: ContractAddress) => { // ... - return formatPlayerWhitelist(whitelist, GuildWhitelist, (entityId: number) => getEntityName(entityId) || 'Unknown'); + return formatPlayerWhitelist(whitelist, GuildWhitelist, getEntityNameWithFallback); };Also applies to: 263-263
Line range hint
1-304: Consider centralizing entity name resolution logic.The current implementation duplicates the fallback logic across multiple locations. Consider creating a dedicated hook for entity name resolution that encapsulates the fallback logic and type safety checks.
Example implementation:
const useEntityNameResolver = () => { const { getEntityName } = useEntitiesUtils(); return useCallback((entityId: number | undefined) => { if (!entityId) return 'Unknown'; return getEntityName(entityId) || 'Unknown'; }, [getEntityName]); };This would:
- Centralize the fallback logic
- Ensure consistent type safety
- Improve maintainability
- Reduce code duplication
client/src/ui/modules/military/battle-view/BattleView.tsx (2)
Line range hint
5-147: Consider memoizing complex conditional structure assignmentThe structure assignment logic combines multiple conditions and could benefit from memoization to prevent unnecessary re-renders.
Consider extracting the structure determination logic into a separate memoized hook:
const useStructureForBattle = ( battleView: BattleView, position: Position, defenderArmies: Army[] ) => { return useMemo(() => { if (battleView?.engage && !battleView?.battleEntityId && !battleView.targetArmy) { return useStructureByPosition({ x: position.x, y: position.y }); } return useStructureByEntityId( defenderArmies.find((army) => army?.protectee)?.protectee?.protectee_id || 0 ); }, [battleView, position, defenderArmies]); };
Line range hint
5-147: Address build issues with proper type safetyIn response to the build issues mentioned in the PR comments, the added type assertions might fix TypeScript errors but introduce potential runtime issues. Instead of using type assertions, consider:
- Properly typing the component props
- Adding runtime type guards
- Handling undefined/null cases explicitly
Would you like help implementing a more robust type-safe solution that addresses both build and runtime concerns?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
client/src/hooks/helpers/use-get-all-players.tsx(1 hunks)client/src/hooks/helpers/useGuilds.tsx(4 hunks)client/src/ui/components/worldmap/armies/ArmyInfoLabel.tsx(2 hunks)client/src/ui/components/worldmap/battles/BattleLabel.tsx(1 hunks)client/src/ui/components/worldmap/structures/StructureLabel.tsx(3 hunks)client/src/ui/modules/entity-details/realm/RealmDetails.tsx(2 hunks)client/src/ui/modules/military/battle-view/BattleView.tsx(2 hunks)
🧰 Additional context used
📓 Learnings (1)
client/src/ui/components/worldmap/structures/StructureLabel.tsx (1)
Learnt from: aymericdelab
PR: BibliothecaDAO/eternum#1818
File: client/src/ui/components/worldmap/structures/StructureLabel.tsx:47-0
Timestamp: 2024-11-12T03:28:50.421Z
Learning: In `StructureLabel.tsx`, when `timer` is negative, the immunity message is not shown, so handling negative `timer` values isn't necessary.
🔇 Additional comments (3)
client/src/hooks/helpers/useGuilds.tsx (2)
205-205: 🛠️ Refactor suggestion
Add type guard for entityId.
The function should handle cases where entityId might be undefined to prevent potential runtime errors.
- (entityId: number) => getEntityName(entityId) || 'Unknown',
+ (entityId: number | undefined) => entityId ? (getEntityName(entityId) || 'Unknown') : 'Unknown',Line range hint 225-231: Ensure type consistency for guild_entity_id.
The conditional check for guildMember.guild_entity_id suggests it might be undefined, but this isn't reflected in the return type of the function.
Consider updating the GuildInfo type to reflect possible undefined values:
interface GuildInfo {
entityId: number | undefined;
name: string;
// ... other fields
}client/src/ui/modules/entity-details/realm/RealmDetails.tsx (1)
1-1: Consider implementing proper TypeScript type guards
The systematic addition of as Structure type assertions across components suggests a underlying type system issue. Instead of using type assertions, consider:
- Implementing a proper type guard:
function isStructure(value: any): value is Structure {
return value && 'category' in value && 'owner' in value;
}- Using the type guard in components:
if (isStructure(structure)) {
// TypeScript now knows structure is Structure
}This approach provides better type safety and runtime checks compared to type assertions.
Also applies to: 1-1, 1-1
| @@ -69,7 +69,7 @@ export const useGetAllPlayers = () => { | |||
| ]).size, | |||
| hyperstructures: runQuery([Has(Hyperstructure), HasValue(Owner, { address: player.address })]).size, | |||
| isAlive: player.isAlive, | |||
| guildName: player.guildName, | |||
| guildName: player.guildName || "", | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove redundant fallback for guildName
The fallback || "" is unnecessary since guildName is already initialized with an empty string on line 30 when guildMember is null. This creates redundant defensive programming.
- guildName: player.guildName || "",
+ guildName: player.guildName,📝 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.
| guildName: player.guildName || "", | |
| guildName: player.guildName, |
| @@ -65,7 +65,7 @@ export const StructureInfoLabel = () => { | |||
| )} | |||
| </Headline> | |||
| <StructureListItem | |||
| structure={structure} | |||
| structure={structure as Structure} | |||
There was a problem hiding this comment.
Use conditional rendering instead of type assertion
Similar to the timer issue, casting potentially undefined structure to Structure is unsafe.
- structure={structure as Structure}
+ structure={structure!}Additionally, consider adding a type guard:
if (!structure) return null;before the JSX return statement.
| @@ -47,7 +47,7 @@ | |||
| const nextBlockTimestamp = useUIStore((state) => state.nextBlockTimestamp); | |||
|
|
|||
| const isImmune = useIsStructureImmune(structure, nextBlockTimestamp || 0); | |||
| const timer = useStructureImmunityTimer(structure, nextBlockTimestamp || 0); | |||
| const timer = useStructureImmunityTimer(structure as Structure, nextBlockTimestamp || 0); | |||
There was a problem hiding this comment.
Add null check before type assertion
The type assertion as Structure could lead to runtime errors if structure is undefined. Consider using optional chaining or a null check.
- const timer = useStructureImmunityTimer(structure as Structure, nextBlockTimestamp || 0);
+ const timer = structure ? useStructureImmunityTimer(structure, nextBlockTimestamp || 0) : 0;Committable suggestion skipped: line range outside the PR's diff.
| @@ -37,7 +37,7 @@ export const BattleInfoLabel = () => { | |||
| battleEntityId={battle.entity_id} | |||
| dojo={dojo} | |||
| currentTimestamp={currentTimestamp} | |||
| structure={structure} | |||
| structure={structure as Structure} | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary type assertion
The BattleInfo component already accepts Structure | undefined in its props type definition. The type assertion is redundant and can be removed.
- structure={structure as Structure}
+ structure={structure}📝 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.
| structure={structure as Structure} | |
| structure={structure} |
| @@ -42,7 +42,7 @@ export const RealmDetails = () => { | |||
| ); | |||
|
|
|||
| const isImmune = useIsStructureImmune(structure, nextBlockTimestamp || 0); | |||
| const timer = useStructureImmunityTimer(structure, nextBlockTimestamp || 0); | |||
| const timer = useStructureImmunityTimer(structure as Structure, nextBlockTimestamp || 0); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Leverage existing checks for type safety
The component already has proper null checking with structure &&. Consider restructuring the immunity timer logic to avoid type assertion.
- const timer = useStructureImmunityTimer(structure as Structure, nextBlockTimestamp || 0);
+ const timer = structure && isImmune
+ ? useStructureImmunityTimer(structure, nextBlockTimestamp || 0)
+ : 0;📝 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 timer = useStructureImmunityTimer(structure as Structure, nextBlockTimestamp || 0); | |
| const timer = structure && isImmune | |
| ? useStructureImmunityTimer(structure, nextBlockTimestamp || 0) | |
| : 0; |
| @@ -3,7 +3,7 @@ import { currencyFormat } from "../../../utils/utils"; | |||
|
|
|||
| import { ArmyInfo, getArmyByEntityId } from "@/hooks/helpers/useArmies"; | |||
| import { useQuery } from "@/hooks/helpers/useQuery"; | |||
| import { useIsStructureImmune, useStructureImmunityTimer, useStructures } from "@/hooks/helpers/useStructures"; | |||
| import { Structure, useIsStructureImmune, useStructureImmunityTimer, useStructures } from "@/hooks/helpers/useStructures"; | |||
There was a problem hiding this comment.
Potential runtime error: Unsafe type assertion
The type assertion as Structure on line 58 could lead to runtime errors since structure might be undefined. The structure variable is conditionally assigned in the useMemo hook above, but the timer hook is called unconditionally.
Consider adding a null check or using optional chaining:
- const timer = useStructureImmunityTimer(structure as Structure, nextBlockTimestamp || 0);
+ const timer = structure ? useStructureImmunityTimer(structure, nextBlockTimestamp || 0) : undefined;Also applies to: 58-58
| @@ -2,7 +2,7 @@ import { BattleManager } from "@/dojo/modelManager/BattleManager"; | |||
| import { useDojo } from "@/hooks/context/DojoContext"; | |||
| import { useBattleManager } from "@/hooks/helpers/battles/useBattles"; | |||
| import { getArmiesByBattleId, getArmyByEntityId, useArmyByArmyEntityId } from "@/hooks/helpers/useArmies"; | |||
| import { useStructureByEntityId, useStructureByPosition } from "@/hooks/helpers/useStructures"; | |||
| import { Structure, useStructureByEntityId, useStructureByPosition } from "@/hooks/helpers/useStructures"; | |||
There was a problem hiding this comment.
Improve type safety for Battle component structure prop
The type assertion as Structure on line 147 could be problematic since structure might be undefined (when both structureFromPosition and structureId are undefined). If the Battle component expects a non-null Structure, this could lead to runtime errors.
Consider these improvements:
- Add proper type guards:
- structure={structure as Structure}
+ structure={structure || undefined}- Update the Battle component's props to handle optional structure:
interface BattleProps {
structure?: Structure;
// ... other props
}- Add runtime validation:
if (structure && !isStructure(structure)) {
console.warn('Invalid structure provided to Battle component');
}Also applies to: 147-147
Summary by CodeRabbit
Release Notes
New Features
hasWonderproperty in realm and structure interfaces to indicate the presence of wonders.Improvements
Bug Fixes
Style
Chores