balancing #2110
balancing #2110
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
You are out of MentatBot reviews. Your usage will refresh November 18 at 08:00 AM. |
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces modifications across several components and constants in the codebase. Notably, the 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 (
|
|
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: 2
🧹 Outside diff range and nitpick comments (5)
sdk/packages/eternum/src/constants/troops.ts (1)
18-33: Document the rationale for specific consumption valuesWhile the new values create a clearer hierarchy between troop types, it would be helpful to document:
- The reasoning behind the specific percentage increases
- How these values were determined to be optimal
- The expected impact on game balance
This documentation would help with future balancing efforts and make it easier to understand the design decisions.
Consider adding a comment block above the
TROOPS_FOOD_CONSUMPTIONconstant explaining these points.+ /** + * Food consumption rates for different troop types. + * Balancing rationale: + * - Paladin: Highest exploration cost due to [reason] + * - Knight: Balanced costs reflecting versatility + * - Crossbowman: Lower base costs but increased by [%] to [achieve goal] + */ export const TROOPS_FOOD_CONSUMPTION: Record<number, TroopFoodConsumption> = {client/src/ui/containers/BaseContainer.tsx (1)
24-27: Consider isolating RTL effects to scrollbar onlyThe current RTL implementation might affect child component text direction. Consider using a more specific approach to control scrollbar position.
Here's a suggested improvement using CSS custom properties:
- { - "[direction:rtl]": scrollbarSide === "left", - }, + { + "[--scrollbar-position:left]": scrollbarSide === "left", + "styled-scrollbar": true + },Add these styles to your CSS:
.styled-scrollbar { direction: initial; &::-webkit-scrollbar { position: absolute; left: var(--scrollbar-position, right); } }This approach:
- Prevents unintended RTL effects on child components
- Maintains proper text direction
- Provides better scrollbar position control
sdk/packages/eternum/src/constants/quests.ts (1)
67-69: Significant reduction in military requirements needs justificationThe 75% reduction in military unit requirements (2 → 0.5) for Knight, Crossbowman, and Paladin represents a major balance change. This could significantly affect game progression and difficulty.
Consider documenting:
- The reasoning behind this substantial reduction
- The expected impact on game balance
- Whether this change is part of a broader military units rebalancing
sdk/packages/eternum/src/constants/global.ts (1)
31-32: Consider compound effects of multiple balance changesThese changes collectively modify three major game systems:
- Faster hyperstructure progression
- Increased early-game resource production
- Reduced military capacity
Please verify that the compound effect of these changes maintains the desired game balance and progression curve. Consider running gameplay simulations to validate the combined impact.
Also applies to: 51-51, 105-109
sdk/packages/eternum/src/constants/resources.ts (1)
527-527: Significant reduction in Donkey production costThe Lords resource requirement for Donkey production has been reduced by 95% (0.001 -> 0.00005). This dramatic decrease will make transport units much more accessible.
Consider if this significant reduction might lead to an oversupply of transport units. You might want to:
- Monitor the transport unit economy after this change
- Consider implementing a production cap if needed
- Adjust gradually in smaller increments to better control the economic impact
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
client/src/ui/containers/BaseContainer.tsx(1 hunks)client/src/ui/modules/navigation/LeftNavigationModule.tsx(1 hunks)client/src/ui/modules/navigation/RightNavigationModule.tsx(1 hunks)sdk/packages/eternum/src/constants/global.ts(3 hunks)sdk/packages/eternum/src/constants/quests.ts(2 hunks)sdk/packages/eternum/src/constants/resources.ts(2 hunks)sdk/packages/eternum/src/constants/troops.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- client/src/ui/modules/navigation/LeftNavigationModule.tsx
- client/src/ui/modules/navigation/RightNavigationModule.tsx
🔇 Additional comments (10)
sdk/packages/eternum/src/constants/troops.ts (1)
18-19: Verify the gameplay impact of increased food consumption rates
The changes significantly increase resource consumption across all troop types:
- Paladin & Knight: +25% for exploration
- Knight: +25% for travel
- Crossbowman: +25-50% for both exploration and travel
Combined with the reduced mercenary limits (10,000 → 4,000) mentioned in the summary, this represents a substantial rebalancing that could significantly impact gameplay dynamics.
Let's analyze the impact on resource consumption:
Also applies to: 24-27, 30-33
client/src/ui/containers/BaseContainer.tsx (2)
12-12: LGTM: Well-typed prop addition
The new optional scrollbarSide prop is properly typed with a clear union type that constrains valid values.
15-18: LGTM: Clean prop destructuring
The prop destructuring is well-organized and maintains proper TypeScript typing.
sdk/packages/eternum/src/constants/quests.ts (1)
28-29: Verify food balance changes across the game
The 20% increase in food quest requirements (1000 → 1200) for both Wheat and Fish could impact early game progression.
✅ Verification successful
Based on the search results, I can now verify the food balance changes across the game. Let me analyze the findings:
Food balance changes are part of a broader resource rebalancing
The 20% increase in food quest requirements (1000 → 1200) for both Wheat and Fish is balanced by:
- Food production buildings (Farm and Fishing Village) have complementary resource costs:
- Farm costs 450 Fish to build
- Fishing Village costs 450 Wheat to build
- Food consumption rates are carefully tuned:
- Military units consume 0.05 of each food type
- Donkeys consume 0.025 Wheat
- Resource production consumes 0.004-0.005 food per tick
- Food is required for realm upgrades in balanced amounts:
- City: 3000 each
- Kingdom: 5000 each
- Empire: 9000 each
The increased quest requirement aligns with the game's overall resource economy and progression system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other food-related constants and balance changes
rg -l "Wheat|Fish" --type ts | xargs rg "(amount|consumption|production|FOOD)" -B 2 -A 2
# Look for any food-related configuration in other constant files
fd -e ts -e json -p "constant|config" | xargs rg "food|wheat|fish" -i
Length of output: 119448
sdk/packages/eternum/src/constants/global.ts (3)
51-51: Verify economic impact of increased starting resource production
The 50% increase in STARTING_RESOURCES_INPUT_PRODUCTION_FACTOR (4 → 6) will significantly boost early-game resource generation. Please confirm this change:
- Aligns with the intended early-game progression speed
- Doesn't create economic imbalances with existing realms
#!/bin/bash
# Description: Search for related resource production balance discussions
# and code references to understand the impact
# Search for related resource balance discussions
gh issue list --search "resource production balance economy" --state all --json number,title,body
# Search for code references to understand usage and dependencies
rg -A 3 "STARTING_RESOURCES_INPUT_PRODUCTION_FACTOR"105-105: Verify impact of reduced mercenary limits on military balance
The 60% reduction in mercenary upper bounds (10,000 → 4,000) for all troop types will significantly affect military strategies. While the relative balance between troop types is maintained, please confirm:
- This reduction aligns with intended military gameplay
- Existing armies above the new limits will be handled gracefully
#!/bin/bash
# Description: Search for related military balance discussions and
# verify handling of existing armies
# Search for related military balance discussions
gh issue list --search "mercenary army balance limits" --state all --json number,title,body
# Search for code handling army size limits
rg -A 5 "MERCENARIES_.*_UPPER_BOUND"Also applies to: 107-107, 109-109
31-32: Verify significant hyperstructure balance changes
The changes drastically alter hyperstructure progression:
- Points per cycle doubled (10 → 20)
- Completion points reduced by 98% (2,000,000 → 40,000)
These modifications will significantly accelerate hyperstructure completion. Please confirm if this aligns with the intended game balance and progression curve.
sdk/packages/eternum/src/constants/resources.ts (3)
409-411: Significant reduction in resource generation rates
Several key resources have had their output rates reduced:
- Wheat and Fish: 20% decrease (75 -> 60)
- Ancient Fragment: 33% decrease (15 -> 10)
These changes will significantly slow down the resource economy, especially considering that food (Wheat/Fish) is a fundamental resource required for most production chains.
#!/bin/bash
# Check for any food consumption or production modifiers
rg -A 5 "(?:Wheat|Fish).*production"
# Check for Ancient Fragment dependencies
ast-grep --pattern 'ResourcesIds.AncientFragment'405-411: Verify the cumulative economic impact
These changes represent significant adjustments to the game's economy:
- Reduced military unit production
- Reduced food production
- Reduced Ancient Fragment production
- Dramatically cheaper transport units
#!/bin/bash
# Look for economic simulation or testing files
fd -t f -e ts -e js | grep -i "simulation\|balance\|economy"
# Check for configuration files that might need updating
fd -t f "config" | grep -i "economy\|balance\|production"
# Look for any hardcoded values that might need adjustment
rg "(?:0\.05|75|15|0\.001).*production"Also applies to: 527-527
405-407: Military units production rate reduced by 20%
The output rate for Knight, Crossbowman, and Paladin has been decreased from 0.05 to 0.04, which will slow down military unit production.
✅ Verification successful
Military units production rate adjustment is part of a balanced combat system
The 20% reduction in military unit production (from 0.05 to 0.04) appears to be a deliberate balance change that affects all military units (Knight, Crossbowman, Paladin) equally. The codebase shows these units are part of an intricate combat system with:
- Rock-paper-scissors mechanics (each unit type has advantages/disadvantages against others)
- Balanced stamina and health values
- Equal resource weights and costs
- Consistent usage across battle simulations, army management, and building production
Since the change maintains the relative balance between unit types while only adjusting the overall production rate, this appears to be a valid game economy adjustment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any military unit production multipliers or modifiers in other files
rg -l "Knight|Crossbowman|Paladin" | grep -i "production\|modifier\|multiplier"
# Check for any references to the old production rate
rg "0\.05.*(?:Knight|Crossbowman|Paladin)"
Length of output: 213
Script:
#!/bin/bash
# Search for military unit references in a broader context
rg -A 5 -B 5 "Knight|Crossbowman|Paladin" --type ts
# Look for production or output rate configurations
ast-grep --pattern 'export const $_ = {
$$$
Knight: $_,
$$$
}'
# Check for any test files or configuration files related to military units
fd -t f -e test.ts -e spec.ts -e config.ts | xargs rg "Knight|Crossbowman|Paladin"
Length of output: 97077
| "flex relative flex-col transition-all duration-400 bg-brown/90 bg-hex-bg animatedBackground", | ||
| { | ||
| "[direction:rtl]": scrollbarSide === "left", | ||
| }, |
There was a problem hiding this comment.
💡 Codebase verification
RTL scrollbar positioning needs implementation in navigation modules
The verification reveals that while BaseContainer has RTL support through the scrollbarSide prop, it's not being utilized in the navigation modules where BaseContainer is implemented:
LeftNavigationModule.tsxuses BaseContainer with right-rounded corners (rounded-r-2xl) suggesting left-side placementRightNavigationModule.tsxuses BaseContainer with left-rounded corners (rounded-l-2xl) suggesting right-side placement
These modules should be updated to explicitly set the scrollbarSide prop to match their positioning:
- Set
scrollbarSide="left"in LeftNavigationModule - Set
scrollbarSide="right"in RightNavigationModule
🔗 Analysis chain
Verify BaseContainer usage and styling
Let's ensure the new scrollbar positioning works correctly with existing implementations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for BaseContainer usage to verify proper prop usage and potential styling conflicts
echo "Checking BaseContainer usage patterns..."
rg -t tsx -t jsx "BaseContainer" --context 3
# Look for potential RTL related styles that might conflict
echo "Checking for potential RTL style conflicts..."
rg -t css -t scss "direction:\s*rtl" --context 2
Length of output: 344
Script:
#!/bin/bash
# Search for BaseContainer usage patterns
echo "Checking BaseContainer usage patterns..."
rg "BaseContainer" -l
# Look for potential RTL style conflicts using a more generic search
echo "Checking for potential RTL style conflicts..."
rg "direction.*rtl" -l
# Check the actual implementation of BaseContainer
echo "Checking BaseContainer implementation..."
rg -A 5 "BaseContainer" "client/src/ui/containers/BaseContainer.tsx"
Length of output: 1055
Script:
#!/bin/bash
# Check how BaseContainer is used in navigation modules
echo "Checking BaseContainer usage in LeftNavigationModule..."
rg -A 5 "BaseContainer" "client/src/ui/modules/navigation/LeftNavigationModule.tsx"
echo "Checking BaseContainer usage in RightNavigationModule..."
rg -A 5 "BaseContainer" "client/src/ui/modules/navigation/RightNavigationModule.tsx"
# Check for any other RTL-related configurations
echo "Checking for any HTML dir or RTL configurations..."
rg "dir=.*rtl" -l
Length of output: 2071
| [QuestType.Population]: [{ resource: ResourcesIds.Knight, amount: 0.5 }], | ||
| [QuestType.Market]: [{ resource: ResourcesIds.Paladin, amount: 0.5 }], | ||
| [QuestType.Mine]: [{ resource: ResourcesIds.Crossbowman, amount: 0.5 }], | ||
| [QuestType.Pillage]: [{ resource: ResourcesIds.Donkey, amount: 0.3 }], | ||
| [QuestType.Hyperstructure]: [{ resource: ResourcesIds.Paladin, amount: 1 }], | ||
| [QuestType.Contribution]: [{ resource: ResourcesIds.Knight, amount: 1 }], | ||
| [QuestType.PauseProduction]: [{ resource: ResourcesIds.Wood, amount: 0.5 }], | ||
| [QuestType.CreateDefenseArmy]: [{ resource: ResourcesIds.AncientFragment, amount: 0.5 }], |
There was a problem hiding this comment.
Review thematic consistency of quest requirements
Some quest resource requirements seem counterintuitive to their themes:
- Population quest requiring Knights
- Market quest requiring Paladins
- Mine quest requiring Crossbowmen
Consider using resources that better align with each quest's theme:
- Population → Basic resources or food
- Market → Trade-related units (e.g., Donkey)
- Mine → Mining-related resources
This could impact player intuition and game design clarity. Consider revising the resource requirements to maintain thematic consistency with each quest type.
Summary by CodeRabbit
Release Notes
New Features
scrollbarSidefor customizable scrollbar positioning in the UI.Enhancements
Bug Fixes
These updates aim to enhance user experience through improved aesthetics and balanced gameplay mechanics.