Mithril listed incorrectly in squence in market (third rarest resource) #2112
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
You are out of MentatBot reviews. Your usage will refresh November 18 at 08:00 AM. |
WalkthroughThe pull request introduces modifications across three components: Changes
Assessment against linked issues
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 (
|
![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 (4)
client/src/ui/components/trading/MarketResourceSideBar.tsx (1)
45-64: Use strict equality comparison for resource IDsThe comparison operator in the
activeprop should use strict equality (===) instead of loose equality (==) when comparing resource IDs to ensure type safety.- active={selectedResource == resourceId} + active={selectedResource === resourceId}client/src/ui/components/trading/MarketOrderPanel.tsx (3)
75-76: Add null checks for resource propertiesWhile optional chaining is used, the fallback to empty string could be improved to provide better user feedback.
Consider this improvement:
-<ResourceIcon size="sm" resource={resource?.trait || ""} withTooltip={false} /> -<div className="truncate text-xs">{resource?.trait || ""}</div> +<ResourceIcon size="sm" resource={resource?.trait || "Unknown"} withTooltip={false} /> +<div className="truncate text-xs">{resource?.trait || "Unknown Resource"}</div>
Line range hint
4-6: Missing test coverage for market order functionalityThe TODO comment indicates missing tests, which is crucial for market functionality.
Would you like me to help create comprehensive test cases for the market order functionality? This should include:
- Resource identification
- Order creation
- Price calculations
- Edge cases
Line range hint
191-444: Improve error handling for resource lookupThe OrderRow component makes multiple calls to
findResourceByIdwithout proper error handling.Consider consolidating resource lookups:
+const resource = useMemo(() => findResourceById(getDisplayResource), [getDisplayResource]); +if (!resource) { + console.error(`Resource not found: ${getDisplayResource}`); + return null; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
client/src/ui/components/bank/LiquidityTable.tsx(1 hunks)client/src/ui/components/trading/MarketOrderPanel.tsx(2 hunks)client/src/ui/components/trading/MarketResourceSideBar.tsx(3 hunks)
🔇 Additional comments (7)
client/src/ui/components/bank/LiquidityTable.tsx (2)
31-36: Formatting improvement looks good
The reformatting of the filter logic improves readability while maintaining the same functionality.
31-36:
Changes don't address Mithril's incorrect position
While the code formatting has been improved, these changes don't fix the core issue mentioned in PR #2064 regarding Mithril's incorrect position in the market sequence. The resource ordering is determined by the RESOURCE_TIERS constant, which needs to be modified to fix Mithril's position as the third rarest resource.
Let's verify the current resource ordering implementation:
client/src/ui/components/trading/MarketResourceSideBar.tsx (3)
3-3: LGTM: Import change aligns with resource sequencing fix
The addition of RESOURCE_TIERS import is appropriate for implementing correct resource ordering.
Line range hint 1-73: Verify changes in production environment
While the implementation looks correct, please ensure to test these changes in a production-like environment to confirm:
- Mithril appears in the correct sequence (third rarest)
- Other resources maintain their expected order
- Price calculations work correctly with the new resource ordering
27-28: Verify RESOURCE_TIERS ordering for Mithril
While using RESOURCE_TIERS is the right approach for maintaining resource order, we should verify that Mithril is correctly positioned as the third rarest resource in the RESOURCE_TIERS constant.
✅ Verification successful
Mithril's position in RESOURCE_TIERS is correct
The verification confirms that Mithril (as Mithral) is correctly positioned in the mythic tier alongside Adamantine and Dragonhide, which represents the rarest resource category in the game's hierarchy: common → uncommon → rare → unique → mythic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify Mithril's position in RESOURCE_TIERS
# Expected: Mithril should be in the third position of the rarest resources
# Search for RESOURCE_TIERS definition
rg -A 10 "RESOURCE_TIERS.*=.*{"
# Alternative: Look for any constants or enums defining resource order
ast-grep --pattern 'export const RESOURCE_TIERS = {
$$$
}'
Length of output: 1995
client/src/ui/components/trading/MarketOrderPanel.tsx (2)
35-35: Improved type safety with ResourcesIds
The change from using resource to resourceId with proper type checking through ResourcesIds enum improves type safety. The memoized resource lookup is also a good optimization.
Also applies to: 43-43, 61-63
Line range hint 446-673: Verify market sequence for Mithril resource
While the code handles resource identifiers correctly, there's no explicit handling of resource sequence or rarity tiers that would address the PR's main objective of fixing Mithril's sequence in the market.
Let's verify the resource sequence implementation:
Consider implementing a dedicated sorting mechanism based on resource rarity tiers to ensure correct resource sequencing in the market.
Fixes #2064
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
resourceIdfor better maintainability and readability.RESOURCE_TIERS.