sync armies fix #2585
sync armies fix #2585
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis pull request focuses on renaming and updating various functions and loading states related to resource and entity management across multiple files in the client source code. The changes primarily involve replacing "Arrivals" with "DonkeysAndArmies" in function names, loading state keys, and related references. The modifications span several TypeScript files, including Changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 23 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: 1
🧹 Nitpick comments (2)
client/src/dojo/queries.ts (1)
Line range hint
194-234: LGTM! The refactored query structure and additional models enhance entity tracking.The changes improve the subscription mechanism by:
- Using precise Member queries for entity ownership
- Adding models for health, protection, and quantity tracking
- Including performance monitoring
Consider adding error handling for the case when the query times out after 1000ms. This could help prevent silent failures and improve debugging.
client/src/ui/layouts/World.tsx (1)
167-167: Consider improving loading state management to prevent race conditions.While the loading state updates are functionally correct, there's a potential race condition where multiple async operations could interfere with each other's loading states.
Consider using a more robust loading state management approach:
- setLoading(LoadingStateKey.DonkeysAndArmies, true); + const loadingId = Date.now(); + setLoading(LoadingStateKey.DonkeysAndArmies, true, loadingId); await debounceAddDonkeysAndArmiesSubscription( dojo.network.toriiClient, dojo.network.contractComponents as any, [...structures.map((structure) => structure.entity_id)], - () => setLoading(LoadingStateKey.DonkeysAndArmies, false), + () => setLoading(LoadingStateKey.DonkeysAndArmies, false, loadingId), );This ensures that only the most recent loading state update is applied.
Also applies to: 189-193
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
client/src/dojo/debouncedQueries.ts(2 hunks)client/src/dojo/queries.ts(2 hunks)client/src/hooks/store/useWorldLoading.tsx(2 hunks)client/src/ui/components/WorldLoading.tsx(1 hunks)client/src/ui/layouts/World.tsx(3 hunks)
🔇 Additional comments (4)
client/src/hooks/store/useWorldLoading.tsx (1)
10-10: Verify all references to LoadingStateKey.Arrivals have been updated
The enum value change from Arrivals to DonkeysAndArmies looks good, but we should ensure all references are updated.
Also applies to: 35-35
✅ Verification successful
All references to LoadingStateKey have been properly updated
All occurrences of the enum value have been consistently updated to LoadingStateKey.DonkeysAndArmies across the codebase:
client/src/hooks/store/useWorldLoading.tsx: Updated in the initial loadingStatesclient/src/ui/layouts/World.tsx: Updated in loading state managementclient/src/ui/components/WorldLoading.tsx: Updated in the loading items display
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to LoadingStateKey.Arrivals
rg -l "LoadingStateKey\.Arrivals"
# Search for new references to LoadingStateKey.DonkeysAndArmies to confirm adoption
rg -l "LoadingStateKey\.DonkeysAndArmies"
Length of output: 196
Script:
#!/bin/bash
# Let's check the actual content of these files to see how the enum is used
rg "LoadingStateKey\.(Arrivals|DonkeysAndArmies)" -A 2 -B 2
Length of output: 1853
client/src/ui/components/WorldLoading.tsx (1)
15-15: LGTM! Loading message is now more descriptive
The loading state check has been updated to use LoadingStateKey.DonkeysAndArmies and the display text "Donkeys and Armies" provides better clarity about what's being loaded.
client/src/dojo/debouncedQueries.ts (1)
5-5: Verify the subscription function renaming
The renaming from addArrivalsSubscription to addDonkeysAndArmiesSubscription looks good, but we should ensure all usages are updated.
Also applies to: 106-114
✅ Verification successful
Function renaming has been properly implemented
All references to the old function names (addArrivalsSubscription and debounceAddResourceArrivals) have been completely replaced with the new names (addDonkeysAndArmiesSubscription and debounceAddDonkeysAndArmiesSubscription). The changes are consistent across all relevant files:
client/src/dojo/debouncedQueries.ts: Exports and implements both functionsclient/src/dojo/queries.ts: Implements the base subscription functionclient/src/ui/layouts/World.tsx: Uses the debounced version correctly
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to the old function names
rg -l "addArrivalsSubscription|debounceAddResourceArrivals"
# Search for new function names to confirm adoption
rg -l "addDonkeysAndArmiesSubscription|debounceAddDonkeysAndArmiesSubscription"
Length of output: 238
Script:
#!/bin/bash
# Let's check the actual content of these files to verify the usage
rg "addArrivalsSubscription|debounceAddResourceArrivals|addDonkeysAndArmiesSubscription|debounceAddDonkeysAndArmiesSubscription" -A 2 -B 2
Length of output: 2012
client/src/ui/layouts/World.tsx (1)
7-7: LGTM! Import statement correctly updated.
The import statement has been properly updated to reflect the renamed function.
| export const debounceAddDonkeysAndArmiesSubscription = debounce( | ||
| async <S extends Schema>( | ||
| client: ToriiClient, | ||
| components: Component<S, Metadata, undefined>[], | ||
| entityID: number[], | ||
| onComplete?: () => void, | ||
| ) => { | ||
| await subscriptionQueue.add(() => addArrivalsSubscription(client, components, entityID), onComplete); | ||
| await subscriptionQueue.add(() => addDonkeysAndArmiesSubscription(client, components, entityID), onComplete); | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
Type inconsistency confirmed between debounced function and its implementation
The type inconsistency is actually a bug. From the code analysis:
-
debounceAddDonkeysAndArmiesSubscriptionacceptsentityID: number[] -
But its implementation
addDonkeysAndArmiesSubscriptionexpectsentityIds: number[](note the plural form) -
All other subscription functions consistently use
entityID: string[] -
client/src/dojo/debouncedQueries.ts: Parameter name mismatch betweenentityIDandentityIds -
client/src/dojo/queries.ts: Consider aligning the type with other subscription functions (string[] instead of number[])
🔗 Analysis chain
Consider type consistency across subscription functions
The entityID parameter is typed as number[] while other similar functions (debouncedAddToSubscription, debouncedAddToSubscriptionOneKey, etc.) use string[]. Please verify if this type difference is intentional.
These commands will help us:
- Find all subscription-related functions
- See the full context of debouncedQueries.ts
- Find function declarations with entityID parameter using AST matching
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for entityID parameter types in subscription functions
ast-grep --pattern 'function $name($params) { $$$}' | rg 'entityID.*\['
Length of output: 73
Script:
#!/bin/bash
# Let's search for all subscription-related functions and their entityID parameter types
rg -A 3 "export (const|function) \w*[Ss]ubscription"
Length of output: 3839
Script:
#!/bin/bash
# Search specifically in the debouncedQueries.ts file
cat client/src/dojo/debouncedQueries.ts
Length of output: 4739
Script:
#!/bin/bash
# Search for function declarations with entityID parameter
ast-grep --pattern 'export const $_ = $$$entityID: $_$$$'
Length of output: 59
Summary by CodeRabbit
New Features
Bug Fixes
Refactor