[client] if not enough food to move, should display how much is missing #2045
Conversation
Army capacity and max realms mint
|
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 11 at 08:00 AM. |
WalkthroughThe changes primarily involve the refactoring of the Changes
Assessment against linked issues
Possibly related PRs
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: 0
🧹 Outside diff range and nitpick comments (4)
client/src/ui/components/worldmap/armies/ArmyWarning.tsx (2)
20-20: Consider memoizing the food calculation.The food calculation uses
useUIStore.getState()outside of useMemo, which could cause unnecessary recalculations on store updates.- const food = armyManager.getFood(useUIStore.getState().currentDefaultTick); + const currentDefaultTick = useUIStore((state) => state.currentDefaultTick); + const food = useMemo(() => + armyManager.getFood(currentDefaultTick), + [armyManager, currentDefaultTick] + );
36-75: Consider reducing code duplication.The warning messages have similar structure and styling. Consider extracting a reusable warning component.
interface WarningMessageProps { message: React.ReactNode; } const WarningMessage = ({ message }: WarningMessageProps) => ( <div className="text-xxs font-semibold text-center bg-red rounded px-1 py-0.5"> <div className="flex"> <span className="w-5">⚠️</span> <span>{message}</span> </div> </div> );Then use it like:
- <div className="text-xxs font-semibold text-center bg-red rounded px-1 py-0.5"> - <div className="flex"> - <span className="w-5">⚠️</span> - <span>Not enough stamina to explore/travel</span> - </div> - </div> + <WarningMessage message="Not enough stamina to explore/travel" />client/src/ui/components/worldmap/armies/ArmyInfoLabel.tsx (2)
82-82: Consider adding error boundary and prop type documentation.While the ArmyWarning integration looks good, consider these improvements for better maintainability:
- Wrap with error boundary to prevent UI crashes
- Document prop types with TSDoc comments
Example implementation:
+ // Wrap with error boundary + <ErrorBoundary fallback={<div className="text-error">Failed to load army warnings</div>}> <ArmyWarning army={army} /> + </ErrorBoundary> + // In ArmyWarning.tsx: + /** + * Props for the ArmyWarning component + * @param army - Army information including food and movement capabilities + */ + interface ArmyWarningProps { + army: ArmyInfo; + }
Line range hint
83-115: Enhance accessibility with ARIA labels.The UI structure is clean and well-organized, but could benefit from improved accessibility.
Add ARIA labels to improve screen reader support:
- <div id="army-info-label-content" className="self-center flex justify-between w-full"> + <div + id="army-info-label-content" + className="self-center flex justify-between w-full" + role="region" + aria-label="Army Information" + >- <div className="grid grid-cols-3 gap-2 relative justify-between w-full text-gold"> + <div + className="grid grid-cols-3 gap-2 relative justify-between w-full text-gold" + role="group" + aria-label="Army Units" + >
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
client/src/ui/components/worldmap/armies/ArmyInfoLabel.tsx(3 hunks)client/src/ui/components/worldmap/armies/ArmyWarning.tsx(1 hunks)client/src/ui/components/worldmap/armies/SelectedArmy.tsx(2 hunks)
🔇 Additional comments (8)
client/src/ui/components/worldmap/armies/SelectedArmy.tsx (2)
9-9: LGTM: Clean import addition
The import statement is properly placed and follows the project's import conventions.
85-85: Verify ArmyWarning integration with parent conditions
The integration looks good. The ArmyWarning component is properly guarded by the showTooltip condition and the non-null assertion is safe here. However, let's verify the component renders correctly under different army states.
✅ Verification successful
Let me gather more information about the ArmyWarning component and its usage.
Based on the gathered information, I can now provide the final response:
ArmyWarning integration is properly implemented and safe
The verification confirms:
- ArmyWarning component properly handles the army prop and uses it safely for capacity, food, and troop calculations
- The component is rendered conditionally with
showTooltip && ownArmy && isMapView - The non-null assertion is safe as
ownArmyis guaranteed to exist when rendered - The component is consistently used across the codebase with proper army prop passing
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify ArmyWarning component integration and props interface
# Test 1: Check ArmyWarning component interface
echo "Checking ArmyWarning component interface..."
ast-grep --pattern 'interface $interface {
army: $_
}'
# Test 2: Verify parent conditions that affect ArmyWarning rendering
echo "Checking conditions affecting ArmyWarning visibility..."
rg -A 2 'showTooltip.*&&.*ownArmy'
Length of output: 558
Script:
#!/bin/bash
# Check ArmyWarning component implementation and usage
echo "Checking ArmyWarning component implementation..."
rg -A 10 "export.*ArmyWarning"
echo "Checking ArmyWarning usage patterns..."
rg "ArmyWarning.*army="
echo "Checking parent component's army-related conditions..."
rg -A 5 "const.*ownArmy.*=" client/src/ui/components/worldmap/armies/SelectedArmy.tsx
Length of output: 1991
client/src/ui/components/worldmap/armies/ArmyWarning.tsx (3)
1-12: LGTM! Clean imports and well-defined interface.
The imports are properly organized and the interface is clearly defined with the necessary prop type.
24-29: LGTM! Well-structured food requirement calculations.
The missing food calculations are properly memoized and clearly implement the PR objective of showing how much food is missing.
63-74: LGTM! Clear and user-friendly missing food warning.
The implementation effectively addresses the PR objective by clearly showing how much wheat and fish is missing, with proper handling of cases where either or both resources are needed.
client/src/ui/components/worldmap/armies/ArmyInfoLabel.tsx (3)
Line range hint 1-20: LGTM! Clean separation of concerns in imports.
The removal of complex calculation imports and addition of the ArmyWarning component shows good modularization of the codebase.
Line range hint 23-38: LGTM! Well-structured component with clear responsibilities.
The component maintains a clean structure focusing on its primary responsibility of displaying army information, while delegating warning logic to a specialized component.
82-82: Verify ArmyWarning implementation for food requirements.
To ensure this change meets the PR objectives of displaying missing food information, we should verify the ArmyWarning component implementation.
✅ Verification successful
ArmyWarning component correctly implements food requirement checks
The implementation properly handles food requirements by:
- Computing required food costs for exploration using
computeExploreFoodCosts - Checking both wheat and fish requirements against army's current food supplies
- Displaying clear warning messages when food requirements are not met, showing exact missing amounts
- Integrating with the army movement and food management systems
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for the ArmyWarning component implementation
echo "Searching for ArmyWarning component implementation..."
rg -l "ArmyWarning" --type tsx
# Look for food-related calculations and displays
echo "Checking food-related logic in ArmyWarning..."
rg -A 10 "food" --type tsx
Length of output: 331
Script:
#!/bin/bash
# Search for ArmyWarning component implementation with correct file extension
echo "Searching for ArmyWarning component implementation..."
rg -l "ArmyWarning"
# Look for food-related calculations and displays
echo "Checking food-related logic..."
rg -A 10 "food" --type ts
Length of output: 10763
Script:
#!/bin/bash
# Get the full implementation of ArmyWarning component
echo "Getting ArmyWarning component implementation..."
cat client/src/ui/components/worldmap/armies/ArmyWarning.tsx
# Look for food cost computation function
echo "Looking for food cost computation..."
rg -A 5 "computeExploreFoodCosts"
Length of output: 7716
Fixes #2042
Summary by CodeRabbit
New Features
ArmyWarningcomponent to display warnings related to army status, including food availability and stamina.SelectedArmycomponent with conditional rendering of theArmyWarningfor improved user feedback.Bug Fixes
ArmyInfoLabelcomponent by removing complex calculations, leading to a more straightforward display of army information.