Docs missing info #2337
Docs missing info #2337
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThe changes in this pull request involve extensive updates to the documentation of the game Eternum, enhancing clarity and readability across various sections. Key modifications include the removal of specific details about stamina in the "World Map," restructuring of the "Resource Production," and the addition of new building types in the "Buildings" section. Other sections, such as "Trading" and "Combat," have been refined for better comprehension, while new content related to "Realm Levels" and "Quest Rewards" has been introduced. Additionally, several components in the codebase have been updated to improve functionality and presentation. 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 (
|
![mentatbot[bot]](https://avatars.githubusercontent.com/in/837875?s=60&v=4){kind=small}
There was a problem hiding this comment.
The PR makes significant improvements to the documentation's clarity and organization. Key improvements include standardized resource icon sizing, better visual hierarchy in building descriptions, clearer warnings about end-of-season asset bridging, and more consistent terminology throughout. The changes make the documentation more professional and easier to navigate. The addition of specific numerical values from configuration adds helpful precision to game mechanics descriptions.
Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!
| @@ -34,7 +31,7 @@ description: Military and combat mechanics in Eternum | |||
|
|
|||
| - Army maximum stamina is determined by its composition | |||
| - The unit with the **lowest** stamina pool sets the army limit | |||
| - Stamina regenerates at a fixed rate for all unit types every Eternum cycle | |||
| - Regenerates {STAMINA_REFILL_PER_TICK} ⚡️ stamina every Eternum day. | |||
There was a problem hiding this comment.
Consider adding the actual stamina regeneration value here for clarity. The current line shows the variable but doesn't explain what it means in game terms.
| - Donkey: {divideByPrecision(EternumGlobalConfig.resources.resourceWeightsGrams[ResourcesIds.Donkey])}kg | ||
| - **Food** (Wheat, Fish): {divideByPrecision(EternumGlobalConfig.resources.resourceWeightsGrams[ResourcesIds.Wheat])}kg | ||
| per unit | ||
| - **Lords**: {divideByPrecision(EternumGlobalConfig.resources.resourceWeightsGrams[ResourcesIds.Lords])}kg per unit |
There was a problem hiding this comment.
Consider adding a note about how these weights affect transport costs and storage capacity planning. This would help players better understand the strategic implications.
| When the game ends, player will have 48 hours to withdraw all in-game assets. After this period, **ALL** assets within | ||
| he game become **permanently locked and inaccessible**, including: | ||
| When the game ends, player will have **48 hours** to withdraw all in-game assets. After this period, **ALL** assets | ||
| within he game become **permanently locked and inaccessible**, including: |
There was a problem hiding this comment.
Fix typo: 'he game' should be 'the game'
| within he game become **permanently locked and inaccessible**, including: | |
| within the game become **permanently locked and inaccessible**, including: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (13)
eternum-docs/docs/pages/overview/entry.mdx (2)
28-30: LGTM! Consider adding version information.The warning about feature availability is clear and well-formatted. To provide more context, consider specifying when this feature is expected to be available (e.g., "Coming in Season 1" or "Planned for Q1 2024").
45-47: Consider clarifying the restoration mechanism.While the distinction between "in-game Realms" and NFT ownership is clear, the restoration mechanism could be more explicit.
Consider this revision for improved clarity:
> In-game Realms can be conquered within a Season, transferring control to the conqueror. > -> Gameplay rights are restored at the start of the next Season, ensuring a dynamic and competitive environment. +> At the start of each new Season, gameplay rights are restored to the original Realm NFT holders, ensuring a dynamic and competitive environment.eternum-docs/src/components/TroopsTable.tsx (3)
35-36: Consider extracting emoji as a constantThe stamina display updates improve visual recognition and hierarchy. However, consider extracting the ⚡️ emoji as a constant to maintain consistency if it's used elsewhere.
+const STAMINA_EMOJI = "⚡️"; + export default function TroopsTable() { // ... - <div className="font-bold">⚡️ Stamina:</div> + <div className="font-bold">{STAMINA_EMOJI} Stamina:</div>
39-42: Consider adding ARIA attributes for better accessibilityThe grid layout is well-structured, but could benefit from accessibility improvements.
- <div className="grid grid-cols-5 gap-2 font-bold"> - <span className="col-span-2">Travel</span> - <span className="col-span-2">Explore</span> - <div></div> + <div className="grid grid-cols-5 gap-2 font-bold" role="rowheader"> + <span className="col-span-2" aria-label="Travel cost">Travel</span> + <span className="col-span-2" aria-label="Explore cost">Explore</span> + <div aria-hidden="true"></div>
64-72: Consider adding error boundaries for config valuesThe stamina consumption section is well-implemented, but consider adding error handling for potentially undefined config values.
+ const travelCost = EternumGlobalConfig.stamina?.travelCost ?? 0; + const exploreCost = EternumGlobalConfig.stamina?.exploreCost ?? 0; <div className="grid grid-cols-5 gap-2"> <div className="col-span-2 text-gray-400"> - {formatNumberWithSpaces(EternumGlobalConfig.stamina.travelCost)} + {formatNumberWithSpaces(travelCost)} </div> <div className="col-span-2 text-gray-400"> - {formatNumberWithSpaces(EternumGlobalConfig.stamina.exploreCost)} + {formatNumberWithSpaces(exploreCost)} </div>eternum-docs/src/components/BuildingCard.tsx (1)
41-45: Consider combining related population metrics.While the implementation is correct, consider combining the population and capacity information into a single metric display for better readability, especially since they're related values.
- {population !== 0 && ( - <div className="mt-2 text-lg text-gray-500 dark:text-gray-300"> - <strong>Population:</strong> +{population} - </div> - )} - {populationCapacity !== 0 && ( - <div className="mt-2 text-lg text-gray-500 dark:text-gray-300"> - <strong>Population Capacity:</strong> +{populationCapacity} - </div> - )} + {(population !== 0 || populationCapacity !== 0) && ( + <div className="mt-2 text-lg text-gray-500 dark:text-gray-300"> + <strong>Population:</strong> {population !== 0 ? `+${population}` : '0'} / {populationCapacity !== 0 ? `+${populationCapacity}` : '0'} + </div> + )}eternum-docs/src/components/RealmUpgradeCosts.tsx (1)
28-33: Clean layout improvements with minor formatting issue.The new layout structure improves the visual hierarchy and organization of resource costs. However, there's a trailing space in the className on line 28.
- <div key={cost.resource} className="flex items-center gap-2 px-3 py-1.5 rounded-md "> + <div key={cost.resource} className="flex items-center gap-2 px-3 py-1.5 rounded-md">eternum-docs/src/components/BuildingCosts.tsx (1)
43-47: Consider extracting repeated cost display logic.The cost display logic is duplicated between wheat and fish resources. Consider extracting this into a reusable component.
+type ResourceCostProps = { + cost: { resource: number; amount: number }; + resource: { trait?: string } | undefined; +}; + +const ResourceCostDisplay = ({ cost, resource }: ResourceCostProps) => ( + <div className="flex items-center gap-1 px-2 py-1.5 rounded-md"> + <ResourceIcon size="xl" id={cost.resource} name={resource?.trait || ""} /> + <div className="flex flex-col"> + <span className="font-medium">{resource?.trait}</span> + <span className="font-medium">{formatAmount(cost.amount)}</span> + </div> + </div> +);eternum-docs/docs/pages/mechanics/key-concepts.mdx (1)
16-17: Consider adding specific stamina valuesWhile the text is clearer, it might be helpful to specify the exact amount or percentage of stamina replenished each Eternum Day to avoid ambiguity.
eternum-docs/docs/pages/seasons/overview.mdx (1)
32-35: Add links to referenced entitiesConsider adding links or references for:
- Lordship Protocol (beyond the current link)
- Season Pool documentation
- Client documentation
This would help users better understand these entities and their roles in fee distribution.
eternum-docs/docs/pages/mechanics/realm/buildings.mdx (1)
47-52: Consider enhancing the Market building description.While the basic information is provided, consider adding more details about:
- Production rate of Donkeys
- $LORDS consumption rate
- Any adjacency bonuses (similar to Farm's description)
<BuildingCard title="Market" image="/buildings/market.png" buildingType={BuildingType.Market} - description={["Produces Donkeys", "Consumes $LORDS", "Essential for trading"]} + description={[ + "Produces Donkeys at X rate", + "Consumes Y $LORDS per Donkey", + "Essential for trading resources between realms", + "Consider strategic placement for optimal trading routes" + ]} />eternum-docs/docs/pages/seasons/rewards.mdx (2)
13-14: Fix typo in warning message.There's a typo in the text: "within he game" should be "within the game".
-When the game ends, player will have **48 hours** to withdraw all in-game assets. After this period, **ALL** assets within he game become **permanently locked and inaccessible**, including: +When the game ends, players will have **48 hours** to withdraw all in-game assets. After this period, **ALL** assets within the game become **permanently locked and inaccessible**, including:
50-51: Improve clarity and consistency in time format.The time format needs to be consistent with previous mentions and the sentence structure could be clearer.
-Any remaining resources or Lords tokens within the game must be bridged out **UP TO 48-hours** from this point until - they become permanently inaccessible +Any remaining resources or Lords tokens must be bridged out within **48 hours** from this point, after which + they become permanently inaccessible
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (20)
eternum-docs/docs/pages/mechanics/key-concepts.mdx(1 hunks)eternum-docs/docs/pages/mechanics/military/combat.mdx(1 hunks)eternum-docs/docs/pages/mechanics/military/units.mdx(2 hunks)eternum-docs/docs/pages/mechanics/realm/buildings.mdx(1 hunks)eternum-docs/docs/pages/mechanics/realm/realm.mdx(1 hunks)eternum-docs/docs/pages/mechanics/resources/resources.mdx(1 hunks)eternum-docs/docs/pages/mechanics/resources/storage.mdx(1 hunks)eternum-docs/docs/pages/mechanics/trading.mdx(2 hunks)eternum-docs/docs/pages/overview/controller.mdx(1 hunks)eternum-docs/docs/pages/overview/disclaimer.mdx(1 hunks)eternum-docs/docs/pages/overview/entry.mdx(1 hunks)eternum-docs/docs/pages/seasons/overview.mdx(2 hunks)eternum-docs/docs/pages/seasons/rewards.mdx(3 hunks)eternum-docs/src/components/BuildingCard.tsx(3 hunks)eternum-docs/src/components/BuildingCosts.tsx(3 hunks)eternum-docs/src/components/RealmUpgradeCosts.tsx(1 hunks)eternum-docs/src/components/ResourceIcon.tsx(1 hunks)eternum-docs/src/components/ResourceTable.tsx(3 hunks)eternum-docs/src/components/TroopsTable.tsx(2 hunks)eternum-docs/vocs.config.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- eternum-docs/docs/pages/mechanics/resources/storage.mdx
- eternum-docs/docs/pages/mechanics/resources/resources.mdx
- eternum-docs/docs/pages/overview/controller.mdx
🔇 Additional comments (26)
eternum-docs/docs/pages/overview/entry.mdx (2)
33-33: LGTM! Consistent token formatting.
The bolding of $LORDS aligns with the formatting in other documentation files and improves visibility of the token reference.
39-39: LGTM! Clear explanation of continuous play mechanics.
The section effectively communicates the requirements for continuous play while maintaining consistent formatting for the $LORDS token reference.
eternum-docs/docs/pages/mechanics/military/units.mdx (2)
1-1: LGTM! Good practice using the constant.
Using the imported constant instead of hardcoding the value ensures the documentation stays synchronized with the actual game mechanics.
34-34: Verify MDX constant interpolation syntax.
Please ensure that the constant is properly interpolated in the MDX format. Some MDX configurations might require a different syntax like {String(STAMINA_REFILL_PER_TICK)} or {${STAMINA_REFILL_PER_TICK}}.
Run the following script to check other MDX files for similar constant interpolation patterns:
✅ Verification successful
MDX constant interpolation is correctly implemented
The current implementation is correct. The evidence shows:
- The constant is properly imported from "@bibliothecadao/eternum"
- The
{STAMINA_REFILL_PER_TICK}syntax is the only pattern used in MDX files - The constant is defined in the SDK with a value of 20
- This is a standard JSX-style interpolation which is compatible with MDX
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check how constants are interpolated in other MDX files
# Look for patterns of constant interpolation in MDX files
rg -t mdx '{[A-Z_]+}' --context 2
Length of output: 120
Script:
#!/bin/bash
# Let's try a different approach to find constant interpolation patterns in MDX
rg -t mdx '\{[A-Z][A-Z0-9_]*\}' -g '*.mdx'
# Also check for alternative interpolation patterns
rg -t mdx '\$\{[A-Z][A-Z0-9_]*\}|\{String\([A-Z][A-Z0-9_]*\)\}' -g '*.mdx'
# Let's also check the specific file for any imports or constant definitions
cat eternum-docs/docs/pages/mechanics/military/units.mdx
Length of output: 2181
Script:
#!/bin/bash
# Let's try searching without file type restriction since mdx isn't recognized
rg '\{[A-Z][A-Z0-9_]*\}' eternum-docs/docs/pages/
# Also check if there are any other files importing and using the same constant
rg "STAMINA_REFILL_PER_TICK" -A 2 -B 2
Length of output: 1760
eternum-docs/src/components/TroopsTable.tsx (2)
1-7: LGTM: Import changes are well-organized
The addition of EternumGlobalConfig import is properly grouped with other imports from the same package.
44-63: LGTM: Well-structured food consumption display
The grid layout for food consumption is well-organized with:
- Clear labeling
- Consistent formatting
- Proper alignment with resource icons
eternum-docs/src/components/BuildingCard.tsx (3)
1-1: LGTM! Clean implementation of building capacity.
The addition of BUILDING_CAPACITY import and the populationCapacity constant with a fallback value of 0 follows good practices.
Also applies to: 20-20
36-40: LGTM! Good UX improvement with conditional rendering.
The population information is now only shown when relevant (non-zero), improving the clarity of the UI. The implementation maintains consistent styling and accessibility.
33-33: Verify the image width reduction impact.
The image width has been reduced from 300px to 250px. Please ensure this change:
- Maintains visual consistency with other building cards
- Doesn't cause image quality or layout issues
✅ Verification successful
Based on the search results, I can now generate the final response since we have clear evidence about the image width usage in BuildingCard.tsx. The file shows two different image width values being used: 200px for mapped images and 250px for the main image.
Image width change is consistent with component design
The width reduction from 300px to 250px for the main image appears intentional as it maintains visual consistency with other images in the component (200px width). The change creates a balanced visual hierarchy where:
- Main image is slightly larger (250px)
- Secondary mapped images are smaller (200px)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other image width definitions in the codebase
rg -A 1 'width="\d+"' --type tsx
Length of output: 62
Script:
#!/bin/bash
# Search for image width definitions with different file extensions
rg -A 1 'width="\d+"' --type ts --type tsx --type jsx --type js
# Also search for any image-related components or styles
rg -A 1 'BuildingCard' --type ts --type tsx --type jsx --type js
Length of output: 186
Script:
#!/bin/bash
# Search for image width definitions using ripgrep without file type filters
rg -A 1 'width="\d+"'
# Search for BuildingCard component references
rg -A 1 'BuildingCard'
# List all files that might contain BuildingCard component
fd BuildingCard
Length of output: 82103
Script:
#!/bin/bash
# Search for image width definitions in BuildingCard.tsx
rg -A 1 'width="[0-9]+"' eternum-docs/src/components/BuildingCard.tsx
# List all files in the repository to understand the structure
fd BuildingCard
Length of output: 341
eternum-docs/src/components/ResourceIcon.tsx (2)
3-12: Well-structured responsive size system!
The new STYLES constant provides a type-safe and responsive sizing system using Tailwind classes. The progression from xs to xxl is logical and the responsive breakpoints are consistently applied.
17-17: Good type safety improvements!
The changes improve type safety and maintainability:
- Type-safe size prop using
keyof typeof STYLES.size - Consistent default size of "xl"
- Clean className implementation using the size system
Also applies to: 20-20, 23-24
eternum-docs/docs/pages/mechanics/realm/realm.mdx (2)
47-50: Clear and informative section introduction!
The re-added "Realm Levels" section provides a clear introduction to the realm upgrade system and its purpose.
52-61: Well-structured progression display!
The use of RealmUpgradeCosts component for each level creates a consistent and clear progression from settlement to empire, with appropriate hex counts and descriptions.
Let's verify the hex counts mentioned in the descriptions:
✅ Verification successful
Let me try another search strategy to find the realm level configurations.
Let me search for the specific hex count configurations.
Based on the search results, I can now generate the final response since I've found the authoritative hex count definitions in the codebase that match exactly with the documentation:
Hex counts are accurately documented!
The hex counts mentioned in the documentation exactly match the official configuration in the codebase (sdk/packages/eternum/src/constants/realmLevels.ts):
- Settlement: 6 buildable hexes
- City: 18 buildable hexes
- Kingdom: 36 buildable hexes
- Empire: 60 buildable hexes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify hex counts in realm levels configuration
# Search for hex count configuration in the codebase
rg -A 1 "buildableHexes"
Length of output: 336
Script:
#!/bin/bash
# Search for realm levels or related configuration
rg -i "realm.*level" -A 5
Length of output: 72804
Script:
#!/bin/bash
# Search for hex count definitions in the codebase
rg -i "buildable.*hex" -A 2
Length of output: 2927
eternum-docs/src/components/ResourceTable.tsx (2)
34-34: LGTM! Improved padding and margin structure.
The padding and margin adjustments enhance the layout consistency.
49-50: Verify text contrast ratio for accessibility.
The text color change from gray-800 to gray-400 might reduce contrast ratio below WCAG standards.
✅ Verification successful
Text contrast appears to follow established patterns and meets accessibility standards
The color change is consistent with the application's design system:
- text-gray-400 is consistently used across documentation components for secondary content
- The component implements proper dark mode support (dark:text-gray-300) which ensures good contrast in both themes
- Other components like BuildingCard use similar gray scale values (text-gray-500/300) for comparable content
The implementation follows Tailwind's color system which is designed with WCAG contrast guidelines in mind, and the dark mode adaptation ensures proper contrast ratios in both themes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other instances of text-gray-400 to ensure consistent usage
rg "text-gray-400" --type tsx
Length of output: 57
Script:
#!/bin/bash
# Search for text-gray-400 class in all files (not limiting to tsx)
rg "text-gray-400"
# Also search for dark mode variant to understand the color scheme
rg "dark:text-gray-300"
# Search for other text color classes to understand the color system
rg "text-gray-[0-9]+"
Length of output: 3749
eternum-docs/src/components/BuildingCosts.tsx (2)
30-34: LGTM! Improved information hierarchy.
The new column layout with separated trait and amount improves readability. Consider verifying the layout in different viewport sizes to ensure optimal spacing.
✅ Verification successful
LGTM! Flex column layout is consistently used throughout the codebase.
The flex flex-col pattern is extensively used across the codebase for vertical stacking layouts, including similar resource display components. The implementation in BuildingCosts.tsx follows the established pattern and maintains consistency with the rest of the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar flex column layouts to ensure consistency
rg "flex flex-col" --type tsx
Length of output: 59
Script:
#!/bin/bash
# Search for similar flex column layouts to ensure consistency, using a more generic file type
rg "flex flex-col" --type ts --type tsx --type jsx --type js
Length of output: 90
Script:
#!/bin/bash
# Search for similar flex column layouts to ensure consistency without file type filters
rg "flex flex-col"
Length of output: 24653
61-70: Verify impact of removing responsive grid columns.
The grid has been simplified from grid-cols-2 sm:grid-cols-3 to just grid-cols-2. This might affect the layout on larger screens.
eternum-docs/docs/pages/mechanics/key-concepts.mdx (3)
22-25: LGTM! Improved clarity of resource production mechanics
The separation into distinct paragraphs and clearer explanation of production mechanics improves readability.
30-33: LGTM! Clear explanation of trading mechanics
The simplified explanation of trading mechanics and emphasis on donkey requirements is well-structured.
39-42: Verify Fragment Mine locations
The addition about Fragment Mines is valuable, but we should ensure this information is consistent across all documentation.
✅ Verification successful
Fragment Mine references are consistent across documentation
The mentions of Fragment Mines across the documentation are consistent and complementary:
- Key concepts introduces them as the source of Ancient Fragments
- Resources page confirms they're the exclusive source
- World map section provides detailed information about their discovery and purpose
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other mentions of Fragment Mines to ensure consistency
rg -i "fragment mine" eternum-docs/
Length of output: 853
eternum-docs/docs/pages/seasons/overview.mdx (2)
11-11: LGTM! Improved grammar
The text now reads more naturally with the corrected phrase "introduction of new mechanics."
24-25: Verify consistency with disclaimer section
The 48-hour bridging period information matches the disclaimer, but we should ensure this critical information is consistently presented throughout the documentation.
✅ Verification successful
Bridging period information is consistently documented across files
After verification, the 48-hour bridging period is consistently documented across multiple files in the codebase:
overview/disclaimer.mdx: "A 48 hour period begins where you must bridge out ALL $LORDS tokens and in-game resources"seasons/rewards.mdx: "Any remaining resources or Lords tokens within the game must be bridged out UP TO 48-hours"seasons/overview.mdx: "UP TO 48 hours after season end"
The information about the 48-hour bridging window is presented consistently throughout the documentation, maintaining the same timeframe and emphasizing the importance of this period for bridging out assets.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for variations of bridging period mentions
rg -i "bridge.*out|bridging.*period" eternum-docs/
Length of output: 1065
eternum-docs/docs/pages/mechanics/military/combat.mdx (1)
35-36: Verify siege duration configuration reference
The siege duration is derived from EternumGlobalConfig.battle.delaySeconds. Please verify:
- This is the correct configuration value for both Realm and Central Bank sieges
- The division by 3600 (to convert seconds to hours) is accurate
✅ Verification successful
These scripts will help us:
- Find any references to siege or delay timing in the codebase
- Locate configuration files
- Examine the content of configuration files to verify the correct configuration path and value
These scripts will help us:
- Find references to battle delay configuration
- Check battle-related configurations in the SDK
- Look for EternumGlobalConfig usage and its structure
These scripts will help us find the battle delay configuration and verify its usage in both the client code and SDK. The previous search showed EternumGlobalConfig usage but we need to find the specific battle delay configuration to verify its correctness.
Based on the search results, I can now provide a final response. The code shows that the battle delay is correctly sourced from the on-chain configuration through the ConfigManager's getBattleDelay() method, which retrieves battle_delay_seconds from the BattleConfig component. The value in the documentation is correctly calculated by dividing this configuration value by 3600 to convert from seconds to hours.
Siege duration configuration reference is correctly implemented
The documentation's usage of EternumGlobalConfig.battle.delaySeconds / 3600 is accurate as:
- The battle delay is sourced from the on-chain BattleConfig component
- The conversion from seconds to hours (division by 3600) is correctly applied
- The same configuration value is used for both Realm and Central Bank sieges
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for siege duration configuration usage
ast-grep --pattern 'EternumGlobalConfig.battle.delaySeconds'
Length of output: 60
Script:
#!/bin/bash
# Search for configuration related to siege or battle delay
rg -i "delay|siege" --type ts --type tsx --type js --type jsx -A 3 -B 3
Length of output: 101
Script:
#!/bin/bash
# Look for global configuration files
fd -e ts -e js -e json "config" --type f
Length of output: 1088
Script:
#!/bin/bash
# Check the content of any config files found
fd -e ts -e js -e json "config" --type f -x cat {}
Length of output: 78873
Script:
#!/bin/bash
# Search for battle delay configuration in the codebase
rg -i "battle.*delay" -A 3 -B 3 --type ts --type tsx --type js --type jsx --type mdx
Length of output: 114
Script:
#!/bin/bash
# Search for battle configuration in the SDK
fd -e ts -e js "battle" -x cat {} -p
Length of output: 434
Script:
#!/bin/bash
# Search for EternumGlobalConfig usage
rg "EternumGlobalConfig" -A 3 -B 3
Length of output: 55522
Script:
#!/bin/bash
# Search for battle delay configuration in the SDK constants
fd -e ts "constants" -x cat {} | grep -A 10 -B 10 "battle.*delay"
Length of output: 67
Script:
#!/bin/bash
# Search for battle configuration in the client code
rg "battle.*config" -A 3 -B 3 --type ts
Length of output: 3858
Script:
#!/bin/bash
# Look for battle-related configuration in the ConfigManager
fd -e ts "ConfigManager" -x cat {} | grep -A 5 -B 5 "battle"
Length of output: 3460
eternum-docs/docs/pages/mechanics/trading.mdx (1)
19-20: LGTM: Consistent formatting of $LORDS token
The bold formatting of $LORDS improves consistency across the documentation.
Also applies to: 24-25
eternum-docs/vocs.config.ts (1)
34-40: Verify existence of season documentation pages
Please ensure the following pages exist and are properly linked:
/seasons/overview/seasons/rewards
✅ Verification successful
Documentation pages for seasons feature are properly linked
The required documentation pages exist at the correct locations:
/seasons/overview→eternum-docs/docs/pages/seasons/overview.mdx/seasons/rewards→eternum-docs/docs/pages/seasons/rewards.mdx
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for existence of season documentation files
fd -e mdx -e md "overview|rewards" docs/pages/seasons/
Length of output: 163
Script:
#!/bin/bash
# First, let's find the correct docs directory structure
fd -t d "docs"
# Then find any markdown/mdx files related to seasons
fd -e md -e mdx "seasons|overview|rewards"
Length of output: 181
eternum-docs/docs/pages/seasons/rewards.mdx (1)
95-99: LGTM: Clear achievement description.
The achievement description is well-structured and provides clear information about the mechanics and reward distribution.
| @@ -19,37 +25,51 @@ export default function TroopsTable() { | |||
| return ( | |||
| <div key={troopId} className="border border-gray-700 p-4 rounded-lg bg-white/5"> | |||
| <div className="flex items-center gap-3 mb-3"> | |||
| <ResourceIcon size={48} id={troopId} name={resource?.trait || ""} /> | |||
| <ResourceIcon size="lg" id={troopId} name={resource?.trait || ""} /> | |||
There was a problem hiding this comment.
💡 Codebase verification
Based on the search results, I can now generate the final response:
ResourceIcon size props are consistently using standardized tokens
The codebase shows consistent usage of standardized size tokens across all ResourceIcon components:
- Common size tokens used: "xs", "sm", "md", "lg", "xl", "xxl"
- Only one numeric size (24) remains in QuestRewards.tsx
- The change from numeric (48) to "lg" aligns with the codebase's standardized approach
🔗 Analysis chain
Verify consistent usage of ResourceIcon size props
The change from numeric (48) to string ("lg") size prop suggests a move towards standardized size tokens. Let's verify this is consistent across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining numeric size props in ResourceIcon components
ast-grep --pattern 'ResourceIcon $$$size={$_}'
# Search for all ResourceIcon size prop usage patterns
rg -A 1 '<ResourceIcon.*size='
Length of output: 26121
| className="p-3 rounded-lg border border-gray-800 dark:bg-gray-400 bg-white/5" | ||
| > | ||
| <div className="flex items-center gap-2"> | ||
| <ResourceIcon size={30} id={cost.resource} name={cost.name || ""} /> | ||
| <ResourceIcon size="lg" id={cost.resource} name={cost.name || ""} /> |
There was a problem hiding this comment.
💡 Codebase verification
Dark mode background color inconsistency detected
The change to dark:bg-gray-400 in ResourceTable is inconsistent with other dark mode backgrounds in the codebase. Other components like StaminaResource and ArmyCapacity use dark:bg-gray-700, which provides better contrast in dark mode. The lighter gray-400 background may compromise readability and break the established visual hierarchy.
eternum-docs/src/components/ResourceTable.tsx: Consider reverting to a darker background color (gray-700 or gray-800) to maintain consistency and ensure proper contrastclient/src/ui/elements/StaminaResource.tsx: Reference implementation using dark:bg-gray-700client/src/ui/elements/ArmyCapacity.tsx: Reference implementation using dark:bg-gray-700
🔗 Analysis chain
Verify dark mode contrast and visual hierarchy.
The background color change in dark mode (gray-800 to gray-400) might affect readability and visual hierarchy.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other dark mode background colors for consistency
rg "dark:bg-gray-[0-9]+" --type tsx
Length of output: 65
Script:
#!/bin/bash
# Search for dark mode background colors with proper file extension
rg "dark:bg-gray-[0-9]+" --type ts --type tsx --type jsx --type js
Length of output: 96
Script:
#!/bin/bash
# Search for dark mode background colors without file type filtering
rg "dark:bg-gray-[0-9]+"
Length of output: 447
| - A **48 hour** period begins where you **must** bridge out **ALL $LORDS** tokens and in-game resources. | ||
| - After this period, **ALL $LORDS** tokens and **Resources** are **permanently** locked within the game. |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent terminology found in bridging period descriptions
The 48-hour period is mentioned across multiple documentation files but with varying terminology:
- "48 hours after" in rewards.mdx
- "UP TO 48-hours" in rewards.mdx and overview.mdx
- "48 hour period" in disclaimer.mdx
This inconsistency could lead to confusion about the exact duration and timing of the bridging window.
🔗 Analysis chain
Critical information about bridging period needs verification
The 48-hour bridging period and permanent locking of tokens are crucial details that need to be consistent across all documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other mentions of the 48-hour period to ensure consistency
rg -i "48.*hour" eternum-docs/
Length of output: 1041
| - **Sending Resource**s: When you want to send resources (like **$LORDS**) to another realm, the bridge calculates the | ||
| distance and travel time based on the coordinates of your realm and the central bank. The transport time depends on | ||
| the distance and is calculated based on the donkey's speed of 24 seconds per kilometer. |
There was a problem hiding this comment.
💡 Codebase verification
Documentation needs to be updated to reference configurable donkey speed
The verification confirms that the donkey speed is indeed configurable and defined as a constant DONKEY_SPEED in sdk/packages/eternum/src/constants/global.ts. The documentation should reference this configurable value instead of hardcoding "24 seconds per kilometer".
- Update
eternum-docs/docs/pages/mechanics/trading.mdxto avoid hardcoding the specific speed value - Consider using a more general description like "The transport time depends on the distance and is calculated based on the configured donkey speed"
🔗 Analysis chain
Consider making donkey speed configurable
The transport time calculation mentions a hardcoded value of "24 seconds per kilometer". Consider referencing this from the game configuration to ensure documentation stays in sync with any game balance changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for donkey speed configuration
ast-grep --pattern 'DONKEY_SPEED'
rg -i 'donkey.*speed'
Length of output: 865
bob0005
changed the title
|
Failed to generate code suggestions for PR |
* contracts: remove season over check in read function * season pool address * add more donkey context * Update links.mdx updated links * doc updates --------- Co-authored-by: Credence <[email protected]> Co-authored-by: ponderingdemocritus <[email protected]> Co-authored-by: RedBeardEth <[email protected]> Co-authored-by: Calcutator <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (23)
landing/package.json (1)
Line range hint
1-116: Add lock file to ensure consistent dependency resolutionConsider adding package-lock.json or yarn.lock to ensure consistent dependency resolution across different environments.
eternum-docs/src/components/TroopsTable.tsx (1)
36-60: Consider adding ARIA labels for better accessibility.While the layout is well-organized, consider adding ARIA labels to improve accessibility for screen readers.
- <div className="text-left"> + <div className="text-left" role="group" aria-label="Travel food consumption"> - <div className="text-left"> + <div className="text-left" role="group" aria-label="Explore food consumption">eternum-docs/src/utils/formatting.tsx (1)
3-5: LGTM with a minor suggestion for robustnessThe function correctly adds spaces before capital letters. Consider handling edge cases like consecutive capitals (e.g., "UNESCO") where you might want to preserve the acronym.
export function addSpacesBeforeCapitals(str: string): string { - return str.replace(/([A-Z])/g, " $1").trim(); + return str.replace(/(?<!^)(?<![\s-])([A-Z])(?=[a-z])|(?<!^)(?<![\s-])([A-Z])(?=[A-Z][a-z])/g, " $1$2").trim(); }eternum-docs/src/components/RealmUpgradeCosts.tsx (1)
21-26: LGTM with accessibility enhancement suggestionThe layout improvements look good. Consider adding ARIA labels for better screen reader support.
- <div key={cost.resource} className="flex items-center gap-2 rounded-md "> + <div + key={cost.resource} + className="flex items-center gap-2 rounded-md" + role="listitem" + aria-label={`${resource?.trait || ''} cost: ${formatAmount(cost.amount)}`} + >eternum-docs/src/components/QuestRewards.tsx (1)
8-25: Consider performance optimization for rewards renderingThe component re-renders the entire grid on every state change. Consider memoizing the rewards mapping.
+import { useMemo } from 'react'; export default function QuestRewards() { + const renderedRewards = useMemo(() => + Object.entries(EternumGlobalConfig.questResources).map(([questType, rewards]) => ( + <div key={questType} className="p-4 rounded-lg border border-gray-200 dark:border-gray-700 bg-white/5"> + {/* ... existing render logic ... */} + </div> + )), + [EternumGlobalConfig.questResources] + ); return ( <div className="grid grid-cols-1 gap-6"> - {Object.entries(EternumGlobalConfig.questResources).map(([questType, rewards]) => ( - // ... existing render logic ... - ))} + {renderedRewards} </div> ); }eternum-docs/docs/pages/mechanics/key-concepts.mdx (3)
16-17: Consider adding specific stamina regeneration rateThe statement about stamina being "partially replenished" is vague. Consider adding the specific regeneration rate (STAMINA_REFILL_PER_TICK) to provide more precise information.
29-30: Fix grammar: Add apostrophe to "Lords Market"The term should be "Lords' Market" (with an apostrophe) as it's a market belonging to multiple Lords.
-Trading is the backbone of the economy in Eternum. The Lords Market is a central marketplace where players can buy and +Trading is the backbone of the economy in Eternum. The Lords' Market is a central marketplace where players can buy and🧰 Tools
🪛 LanguageTool
[uncategorized] ~29-~29: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...backbone of the economy in Eternum. The Lords Market is a central marketplace where p...(AI_HYDRA_LEO_APOSTROPHE_S_XS)
32-33: Improve sentence structure with proper commaAdd a comma after "donkeys" to separate the two independent clauses.
-Trades are facilitated via donkeys and the cost depends on the weight of the resources being transported. Donkeys are +Trades are facilitated via donkeys, and the cost depends on the weight of the resources being transported. Donkeys are🧰 Tools
🪛 LanguageTool
[uncategorized] ~32-~32: Possible missing comma found.
Context: ...each other. Trades are facilitated via donkeys and the cost depends on the weight of t...(AI_HYDRA_LEO_MISSING_COMMA)
eternum-docs/docs/pages/mechanics/realm/realm.mdx (2)
24-26: Consider adding quest prerequisitesThe Quest Rewards section would be more helpful if it included any prerequisites or recommended order for completing the quests.
54-56: Enhance realm upgrade explanationConsider adding a note about whether realm upgrades are reversible and if there are any cooldown periods between upgrades.
eternum-docs/src/components/BuildingCosts.tsx (1)
22-27: LGTM! UI improvements enhance readabilityThe changes improve the component by:
- Using consistent ResourceIcon size props
- Better visual hierarchy with flex column layout
- Reusing formatAmount utility
Consider adding aria-labels to improve accessibility for screen readers.
-<div className="flex flex-col"> +<div className="flex flex-col" aria-label={`${resource?.trait} cost`}>Also applies to: 36-40, 59-63
eternum-docs/docs/pages/mechanics/trading.mdx (1)
58-58: Minor grammar improvement neededConsider adding "The" before "Owner" for better readability.
-- Owner collects AMM trading fees +- The owner collects AMM trading fees🧰 Tools
🪛 LanguageTool
[uncategorized] ~58-~58: A determiner appears to be missing. Consider inserting it.
Context: ... has ended. Ownership Benefits: - Owner collects AMM trading fees, providing a ...(AI_EN_LECTOR_MISSING_DETERMINER)
eternum-docs/docs/pages/seasons/rewards.mdx (4)
13-14: Improve grammar and clarityAdd the missing article and restructure the sentence for better readability.
-When the game ends, player will have **48 hours** to withdraw all in-game assets. After this period, **ALL** assets +When the game ends, the player will have **48 hours** to withdraw all in-game assets. After this period, **ALL** assets🧰 Tools
🪛 LanguageTool
[uncategorized] ~13-~13: Possible missing article found.
Context: ... assets When the game ends, player will have 48 hours to withdraw all ...(AI_HYDRA_LEO_MISSING_THE)
24-26: Reorder steps and maintain consistent formattingThe steps should be reordered to prioritize the most critical actions first, and formatting should be consistent.
-> 1. Remove liquidity from AMM pools before game end -> 2. Cancel open orders on the Order Book -> 3. Bridge out **$LORDS** and **Resources** through the Central Bank before game end +> 1. Cancel open orders on the Order Book +> 2. Remove liquidity from AMM pools +> 3. Bridge out **$LORDS** and **Resources** through the Central BankAlso, consider adding "before game end" to all steps for consistency.
50-51: Improve consistency in time period referencesThe capitalization and phrasing should be consistent with other mentions of the 48-hour period.
-Any remaining resources or Lords tokens within the game must be bridged out **UP TO 48 hours** from this point until - they become permanently inaccessible +Any remaining resources or Lords tokens within the game must be bridged out within **48 hours** from this point before + they become permanently inaccessible
95-99: Clarify achievement points and prize pool calculationThe explanation of how achievement points relate to prize pool distribution could be clearer.
-By completing them, players will earn points and compete for positions on the achievement leaderboard. At the end of the -season, the total number of achievement points will be collated for all players and the prize pool is calculated as a -percentage of the total achievement points collected. +By completing achievements, players: +1. Earn achievement points +2. Compete for positions on the achievement leaderboard +3. Qualify for prize pool rewards, which are distributed proportionally based on each player's achievement points relative to the total points collectedconfig/index.ts (1)
94-94: Enhance documentation for season pool fee recipient configurationThe comment should explicitly document the security implications and purpose of this configuration.
- season_pool_fee_recipient: BigInt(getContractByName(manifest, `${NAMESPACE}-season_systems`)), // Season System holds the Lords... + season_pool_fee_recipient: BigInt(getContractByName(manifest, `${NAMESPACE}-season_systems`)), // Season System contract securely holds and distributes Lords tokens for season rewardsclient/src/ui/modules/rewards/Rewards.tsx (2)
Line range hint
56-65: Add error handling to async operationsThe loading state could get stuck if operations fail. Add proper error handling with try-catch blocks.
const registerToLeaderboard = useCallback(async () => { - setIsLoading(true); + setIsRegistering(true); + try { const contributions = Array.from(getContributions()); const epochs = getEpochs(); await register_to_leaderboard({ signer: account, hyperstructure_contributed_to: contributions, hyperstructure_shareholder_epochs: epochs, }); + } catch (error) { + console.error('Registration failed:', error); + // Consider adding user notification here + } finally { - setIsLoading(false); + setIsRegistering(false); + } }, [getContributions, getEpochs]); const claimRewards = useCallback(async () => { - setIsLoading(true); + setIsClaiming(true); + try { await claim_leaderboard_rewards({ signer: account, token: env.VITE_LORDS_ADDRESS!, }); + } catch (error) { + console.error('Claim failed:', error); + // Consider adding user notification here + } finally { - setIsLoading(false); + setIsClaiming(false); + } }, [account]);Also applies to: 69-74
217-217: Enhance button accessibilityAdd ARIA attributes to improve accessibility when the button is in a loading state.
isLoading={isLoading} + aria-busy={isLoading} + aria-label={isLoading ? 'Processing...' : undefined}landing/src/components/modules/bridge-out-step-1.tsx (1)
213-228: Consider enhancing the donkey balance displayWhile the current implementation is functional, consider adding visual feedback (e.g., red text or warning icon) when the available donkeys are insufficient for the transfer.
- <div className="flex items-center gap-2"> - {donkeysNeeded} / {donkeyBalance.balance} <ResourceIcon resource={"Donkey"} size="md" /> - </div> + <div className="flex items-center gap-2"> + <span className={donkeysNeeded > donkeyBalance.balance ? "text-red-500" : ""}> + {donkeysNeeded} / {donkeyBalance.balance} + </span> + <ResourceIcon resource={"Donkey"} size="md" /> + </div>landing/src/components/modules/bridge-in.tsx (1)
256-272: Fix typo in tooltip textThere's a typo in the tooltip text: "Realmto" should be "Realm to".
- Donkeys are required on your Realmto transport the resources to the bank. Finish the starting quests + Donkeys are required on your Realm to transport the resources to the bank. Finish the starting questslanding/src/dojo/modelManager/ConfigManager.ts (1)
618-633: Consider adding explicit return typeThe implementation follows the established pattern, but would benefit from an explicit return type for better type safety.
- getSeasonBridgeConfig() { + getSeasonBridgeConfig(): { closeAfterEndSeconds: bigint } {contracts/src/systems/hyperstructure/contracts.cairo (1)
Line range hint
404-405: Critical: Removed season check could lead to unexpected behavior.The removal of
SeasonImpl::assert_season_is_not_over(world)allows points calculation after season ends, which could impact leaderboard integrity.Apply this diff to restore the check:
fn get_points(ref self: ContractState, player_address: ContractAddress, hyperstructures_contributed_to: Span<ID>, hyperstructure_shareholder_epochs: Span<(ID, u16)>) -> (u128, u128, u128, u128) { let mut world: WorldStorage = self.world(DEFAULT_NS()); + SeasonImpl::assert_season_is_not_over(world);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
client/src/ui/modules/rewards/Rewards.tsx(4 hunks)config/index.ts(2 hunks)contracts/manifest_prod.json(1 hunks)contracts/src/systems/hyperstructure/contracts.cairo(1 hunks)eternum-docs/docs/pages/development/index.mdx(1 hunks)eternum-docs/docs/pages/mechanics/key-concepts.mdx(1 hunks)eternum-docs/docs/pages/mechanics/military/units.mdx(2 hunks)eternum-docs/docs/pages/mechanics/realm/buildings.mdx(1 hunks)eternum-docs/docs/pages/mechanics/realm/realm.mdx(3 hunks)eternum-docs/docs/pages/mechanics/trading.mdx(4 hunks)eternum-docs/docs/pages/overview/disclaimer.mdx(2 hunks)eternum-docs/docs/pages/overview/links.mdx(1 hunks)eternum-docs/docs/pages/seasons/rewards.mdx(3 hunks)eternum-docs/src/components/BuildingCard.tsx(3 hunks)eternum-docs/src/components/BuildingCosts.tsx(4 hunks)eternum-docs/src/components/QuestRewards.tsx(1 hunks)eternum-docs/src/components/RealmUpgradeCosts.tsx(2 hunks)eternum-docs/src/components/ResourceTable.tsx(3 hunks)eternum-docs/src/components/TroopsTable.tsx(2 hunks)eternum-docs/src/utils/formatting.tsx(1 hunks)landing/package.json(1 hunks)landing/src/components/modules/bridge-in.tsx(7 hunks)landing/src/components/modules/bridge-out-step-1.tsx(5 hunks)landing/src/dojo/modelManager/ConfigManager.ts(4 hunks)landing/src/dojo/setup.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- eternum-docs/docs/pages/development/index.mdx
- eternum-docs/docs/pages/overview/links.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
- eternum-docs/docs/pages/overview/disclaimer.mdx
- eternum-docs/docs/pages/mechanics/military/units.mdx
- eternum-docs/src/components/BuildingCard.tsx
- eternum-docs/src/components/ResourceTable.tsx
🧰 Additional context used
🪛 LanguageTool
eternum-docs/docs/pages/mechanics/key-concepts.mdx
[uncategorized] ~29-~29: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...backbone of the economy in Eternum. The Lords Market is a central marketplace where p...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
[uncategorized] ~32-~32: Possible missing comma found.
Context: ...each other. Trades are facilitated via donkeys and the cost depends on the weight of t...
(AI_HYDRA_LEO_MISSING_COMMA)
eternum-docs/docs/pages/mechanics/trading.mdx
[uncategorized] ~58-~58: A determiner appears to be missing. Consider inserting it.
Context: ... has ended. Ownership Benefits: - Owner collects AMM trading fees, providing a ...
(AI_EN_LECTOR_MISSING_DETERMINER)
eternum-docs/docs/pages/seasons/rewards.mdx
[uncategorized] ~13-~13: Possible missing article found.
Context: ... assets When the game ends, player will have 48 hours to withdraw all ...
(AI_HYDRA_LEO_MISSING_THE)
🔇 Additional comments (16)
landing/package.json (1)
20-28: Verify compatibility of updated dependencies
Multiple related packages are being updated together:
- @cartridge/* packages from ^0.5.1 to ^0.5.3
- @dojoengine/* packages to 1.0.1
While these are minor/patch version updates, coordinated updates across related packages warrant verification.
✅ Verification successful
Let me try another approach to verify the package versions and their compatibility.
Based on the analysis of the shell script results, I can now provide a final response:
Dependencies update appears safe to proceed
The version updates are minor/patch changes and the codebase usage patterns show consistent integration:
- @cartridge/* packages: 0.5.1 -> 0.5.3 (minor) - Used primarily for controller/connector functionality
- @dojoengine/* packages: 1.0.0 -> 1.0.1 (patch) - Core framework dependencies with consistent usage patterns
The presence of pnpm-lock.yaml shows dependency resolution is properly tracked. The updates appear coordinated across related packages and maintain compatibility based on:
- Consistent usage patterns across the codebase
- No breaking changes in import statements or API usage
- Proper dependency resolution in lock file
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check release notes and changelogs for breaking changes
# Check @cartridge packages
echo "Checking @cartridge packages changelog..."
gh api repos/cartridge/connector/releases/latest
gh api repos/cartridge/controller/releases/latest
# Check @dojoengine packages
echo "Checking @dojoengine packages changelog..."
gh api repos/dojoengine/dojo/releases/latest
Length of output: 11230
Script:
#!/bin/bash
# Check for any breaking changes in the codebase that might be related to these package updates
# Look for any migration guides or upgrade notes in the codebase
echo "Checking for migration/upgrade documentation..."
fd -t f "MIGRATION|UPGRADE|CHANGELOG" -I
# Check for usage of potentially changed APIs from these packages
echo "Checking @cartridge/* usage..."
rg "@cartridge/(connector|controller)" -A 2
echo "Checking @dojoengine/* usage..."
rg "@dojoengine/(core|create-burner|react|recs|state|torii-client|utils)" -A 2
# Check package-lock.json or yarn.lock for dependency resolution
echo "Checking for lock files..."
fd -t f "package-lock.json|yarn.lock" -I
Length of output: 59173
eternum-docs/src/components/TroopsTable.tsx (4)
1-7: LGTM! Import changes look good.
The addition of EternumGlobalConfig is properly grouped with other imports from the same package and is used effectively in the component.
17-19: LGTM! Layout improvements enhance readability.
The added padding, margin, and responsive grid layout (1 column on mobile, 3 columns on large screens) improve the visual presentation and responsiveness of the component.
27-33: ResourceIcon size prop follows the standardized approach.
The change from numeric size to string tokens ("xl") aligns with the codebase's standardized approach for ResourceIcon components.
64-78: Verify stamina cost configuration and consider consistent emoji placement.
The stamina costs are correctly retrieved from EternumGlobalConfig, but let's verify these values match the game design.
Also, consider moving the emoji to be consistent with the travel/explore labels:
- <div>Travel</div>
+ <div>Travel ⚡️</div>
- <div>Explore</div>
+ <div>Explore ⚡️</div>
- <div className="flex justify-center items-center mt-4">
- <div>⚡️</div>
- </div>✅ Verification successful
Stamina costs are correctly configured and used consistently across the codebase.
The verification shows that:
- The stamina costs are properly defined in the global configuration and correctly accessed in the UI
- The values are used consistently across the codebase:
- Client-side validation in
ArmyMovementManager.tsandArmyWarning.tsx - Server-side validation in travel and explore systems
- Configuration setup in
setupGlobalsfunction
- Client-side validation in
Regarding the emoji placement suggestion, while it would make the UI more consistent, the current implementation with a single emoji indicator is also valid and clear.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other uses of stamina cost configuration
rg -A 2 'stamina\.(travel|explore)Cost' --type typescript
# Search for documentation of these values
rg -l 'stamina.*cost' --glob '*.md' --glob '*.mdx'
Length of output: 147
Script:
#!/bin/bash
# Search for stamina cost configuration with broader pattern
rg -A 2 'stamina.*Cost'
# Search for EternumGlobalConfig definition
ast-grep --pattern 'const EternumGlobalConfig = {
$$$
}'
# Search for any stamina-related configuration files
fd -e ts -e js -e json 'config|stamina'
Length of output: 5440
Script:
#!/bin/bash
# Let's check the ConfigManager implementation to see how stamina costs are managed
cat client/src/dojo/modelManager/ConfigManager.ts
# Also check the actual configuration values
cat sdk/packages/eternum/src/config/index.ts
Length of output: 49132
eternum-docs/docs/pages/mechanics/key-concepts.mdx (1)
39-41: Verify Fragment Mine locations
The new information about Fragment Mines is important, but it would be helpful to verify if there's a specific distribution pattern or if they're randomly scattered.
✅ Verification successful
Based on the search results, I can now provide a final response since I have found detailed information about Fragment Mine distribution in the documentation and code:
Fragment Mines are discovered through exploration with random distribution
Fragment Mines are discovered through world map exploration with the following characteristics:
- They have a small random chance of being discovered when exploring undiscovered hexes
- There is no predetermined pattern - they are randomly scattered across the map
- They are the only source of Ancient Fragments in the game
- When discovered, they have no immunity period, making them immediately contestable
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Fragment Mine distribution patterns in the codebase
rg -A 3 "FragmentMine" --type ts --type tsx
Length of output: 71
Script:
#!/bin/bash
# Search for Fragment Mine distribution patterns in documentation and code
rg -i "fragment.?mine" --type-add 'docs:*.{md,mdx}' --type docs -A 3
Length of output: 1634
Script:
#!/bin/bash
# Search for any mentions of fragment mines in the codebase
rg -i "fragment.?mine" -A 3
Length of output: 39718
Script:
#!/bin/bash
# Look for files that might contain game mechanics or map information
fd -e ts -e tsx -e md -e mdx -e json . | grep -i "map\|mechanic\|spawn\|mine"
Length of output: 2292
landing/src/dojo/setup.ts (1)
41-42: Verify bridge-related component dependencies
The added WeightConfig and CapacityConfig components are marked as needed for the bridge functionality. Please verify that all necessary bridge-related components are included.
✅ Verification successful
Let me try a different search strategy to verify the bridge-related components.
Let me check the bridge-related components and their usage in the bridge files.
Let me check the bridge component dependencies and their usage.
Let me try one more search to verify the component dependencies.
Based on the analysis of the code, I can now generate the final response:
Bridge components are properly integrated and used
The added WeightConfig and CapacityConfig components are correctly integrated with the bridge system and used alongside other required components:
- The components are used in weight calculations for resource transfers via
getTotalResourceWeightandcalculateDonkeysNeededfunctions - They work together with other bridge-related components like
Position,ArrivalTime,EntityOwner, etc., to handle resource transfers - The bridge functionality is properly implemented in both bridge-in and bridge-out operations, with all necessary component dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for bridge-related component dependencies
echo "Searching for bridge component references..."
rg -A 5 "WeightConfig|CapacityConfig" --type ts --type tsx
echo "Searching for bridge-related imports..."
ast-grep --pattern 'import { $$$, WeightConfig, CapacityConfig, $$$ } from "$_"'
Length of output: 359
Script:
#!/bin/bash
# Search for WeightConfig and CapacityConfig definitions and usages
echo "Searching for config definitions..."
rg -A 5 "WeightConfig|CapacityConfig" --type typescript
echo "Searching for bridge-related files..."
fd -e ts -e tsx bridge
echo "Checking setup file content..."
cat landing/src/dojo/setup.ts
Length of output: 2983
Script:
#!/bin/bash
# Check bridge component usage in bridge-related files
echo "Checking bridge-in component usage..."
cat landing/src/components/modules/bridge-in.tsx
echo -e "\nChecking bridge-out-step-1 component usage..."
cat landing/src/components/modules/bridge-out-step-1.tsx
echo -e "\nChecking bridge-out-step-2 component usage..."
cat landing/src/components/modules/bridge-out-step-2.tsx
echo -e "\nChecking main bridge component usage..."
cat landing/src/components/modules/bridge.tsx
Length of output: 37400
Script:
#!/bin/bash
# Search for weight and capacity related code
echo "Searching for weight calculations..."
rg -A 5 "getTotalResourceWeight|calculateDonkeysNeeded" --type typescript
echo -e "\nSearching for Position component usage..."
rg -A 5 "Position" --type typescript
echo -e "\nSearching for ArrivalTime component usage..."
rg -A 5 "ArrivalTime" --type typescript
echo -e "\nChecking bridge hooks..."
cat landing/src/hooks/useBridge.tsx
Length of output: 3634
Script:
#!/bin/bash
# Search for component usage in bridge files with .ts extension
echo "Searching for weight and capacity calculations..."
rg -A 5 "getTotalResourceWeight|calculateDonkeysNeeded" --type ts
echo -e "\nSearching for Position component usage..."
rg -A 5 "Position" --type ts
echo -e "\nSearching for bridge-related components in setup..."
rg -A 5 "WeightConfig|CapacityConfig|Position|ArrivalTime|EntityOwner|OwnedResourcesTracker|Weight|Resource" landing/src/dojo/setup.ts
Length of output: 68385
eternum-docs/docs/pages/mechanics/realm/realm.mdx (1)
59-68: Verify realm upgrade costs accuracy
Please verify that the upgrade costs displayed by the RealmUpgradeCosts component match the current game configuration.
✅ Verification successful
Let me try a different search strategy to find the realm upgrade configuration and costs.
Let me search for the actual upgrade costs configuration to verify against the documentation.
Based on the search results, I can now generate the final response since I found the actual realm upgrade costs configuration in sdk/packages/eternum/src/constants/realmLevels.ts.
Realm upgrade costs match the configuration
The buildable hex counts in the documentation match the game configuration, and the upgrade costs are correctly implemented:
- Settlement (6 hexes): No upgrade cost (starting level)
- City (18 hexes): 3000 Wheat and 3000 Fish
- Kingdom (36 hexes): 600 each of Cold Iron, Hartwood, Diamonds, Sapphire, Deep Crystal, plus 5000 each of Wheat and Fish
- Empire (60 hexes): Costs shown in the RealmUpgradeCosts component will be pulled from the same configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for realm upgrade configuration
echo "Searching for realm upgrade costs configuration..."
rg -A 10 "RealmLevels" --type ts --type tsx
echo "Searching for realm upgrade component implementation..."
ast-grep --pattern 'const RealmUpgradeCosts = $_'
Length of output: 776
Script:
#!/bin/bash
# Search for realm levels and costs in the codebase
echo "Searching for realm levels..."
rg -A 5 "enum.*Realm"
echo "Searching for RealmUpgradeCosts component..."
rg -A 10 "RealmUpgradeCosts"
echo "Searching for realm upgrade costs configuration..."
rg -A 10 "buildable.*hex"
Length of output: 13228
Script:
#!/bin/bash
# Search for REALM_UPGRADE_COSTS constant and its implementation
echo "Searching for realm upgrade costs constant..."
rg -A 15 "REALM_UPGRADE_COSTS.*="
# Search for any realm cost related constants
echo "Searching for realm cost related files..."
fd -e ts -e tsx -e js -e jsx | xargs rg -l "realm.*cost"
Length of output: 4980
eternum-docs/docs/pages/mechanics/trading.mdx (1)
1-2: LGTM! Documentation now references configurable donkey speed
The changes properly address the previous feedback by:
- Importing DONKEY_SPEED from the SDK
- Using the configurable value instead of hardcoding
Also applies to: 40-42
eternum-docs/docs/pages/mechanics/realm/buildings.mdx (1)
47-60: LGTM! New building cards are well-documented
The new Fishing Village and Market building cards:
- Follow consistent documentation structure
- Provide clear descriptions of functionality
- Properly integrate with existing building types
landing/src/components/modules/bridge-out-step-1.tsx (2)
4-4: LGTM: Resource balance handling is well implemented
The implementation correctly retrieves and memoizes the donkey balance, with proper dependency tracking in the useMemo hook.
Also applies to: 7-7, 87-94
126-128: LGTM: Proper precision handling for resource amounts
The addition of multiplyByPrecision ensures correct scaling of resource amounts before weight calculation.
landing/src/components/modules/bridge-in.tsx (1)
4-4: LGTM: Consistent implementation with bridge-out-step-1
The resource balance handling follows the same pattern as bridge-out-step-1, maintaining consistency across components.
Also applies to: 7-7, 154-161
landing/src/dojo/modelManager/ConfigManager.ts (2)
312-322: LGTM: Well-structured immunity tick handling
The implementation properly differentiates immunity ticks based on structure type while maintaining backward compatibility through the default case.
502-502: Verify the impact of hyperstructure resource retrieval change
The switch from GET_RESOURCES_PER_TIER to GET_HYPERSTRUCTURE_RESOURCES_PER_TIER might affect resource calculations.
✅ Verification successful
Let me check the implementation of these functions to understand if the change is consistent.
Change to hyperstructure resource retrieval is intentional and correct
The switch from GET_RESOURCES_PER_TIER to GET_HYPERSTRUCTURE_RESOURCES_PER_TIER is a deliberate change that introduces different behavior for hyperstructure resources:
- Military, Transport, and Food tiers return empty arrays
- Lords tier filters out the Lords resource when hyperstructure is true
- Other tiers (Common, Uncommon, Rare, Unique, Mythic) maintain the same behavior
The change has been consistently applied across the codebase in all relevant files (client and landing).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other usages of GET_RESOURCES_PER_TIER to ensure consistent updates
# Search for any remaining usage of GET_RESOURCES_PER_TIER
rg "GET_RESOURCES_PER_TIER"
# Search for all usages of GET_HYPERSTRUCTURE_RESOURCES_PER_TIER
rg "GET_HYPERSTRUCTURE_RESOURCES_PER_TIER"
Length of output: 1030
Script:
#!/bin/bash
# Get the implementation of both functions to verify they are related
ast-grep --pattern 'export const GET_RESOURCES_PER_TIER = $_'
ast-grep --pattern 'export const GET_HYPERSTRUCTURE_RESOURCES_PER_TIER = $_'
Length of output: 5290
contracts/manifest_prod.json (1)
5340-5340: LGTM: Class hash update matches hyperstructure contract changes.
The updated class hash reflects the modifications made to the hyperstructure contract.
| export const formatAmount = (amount: number) => { | ||
| if (amount < 1) { | ||
| return `${amount * 1000}`; | ||
| } else if (amount < 1000) { | ||
| return `${amount.toFixed(amount % 1 === 0 ? 0 : (amount % 1) % 0.1 === 0 ? 1 : 2)}K`; | ||
| } else { | ||
| return `${(amount / 1000).toFixed(amount % 1000 === 0 ? 0 : (amount % 1000) % 10 === 0 ? 1 : 2)}M`; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify amount formatting logic and add input validation
The current implementation has several potential issues:
- Complex decimal place logic that's hard to maintain
- No validation for negative numbers
- Potential floating-point precision issues with modulo operations
Consider this more maintainable approach:
export const formatAmount = (amount: number) => {
+ if (!Number.isFinite(amount) || amount < 0) {
+ return '0';
+ }
+
if (amount < 1) {
- return `${amount * 1000}`;
+ return (amount * 1000).toFixed(0);
} else if (amount < 1000) {
- return `${amount.toFixed(amount % 1 === 0 ? 0 : (amount % 1) % 0.1 === 0 ? 1 : 2)}K`;
+ const decimals = amount % 1 === 0 ? 0 : 1;
+ return `${amount.toFixed(decimals)}K`;
} else {
- return `${(amount / 1000).toFixed(amount % 1000 === 0 ? 0 : (amount % 1000) % 10 === 0 ? 1 : 2)}M`;
+ const inMillions = amount / 1000;
+ const decimals = inMillions % 1 === 0 ? 0 : 1;
+ return `${inMillions.toFixed(decimals)}M`;
}
};📝 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.
| export const formatAmount = (amount: number) => { | |
| if (amount < 1) { | |
| return `${amount * 1000}`; | |
| } else if (amount < 1000) { | |
| return `${amount.toFixed(amount % 1 === 0 ? 0 : (amount % 1) % 0.1 === 0 ? 1 : 2)}K`; | |
| } else { | |
| return `${(amount / 1000).toFixed(amount % 1000 === 0 ? 0 : (amount % 1000) % 10 === 0 ? 1 : 2)}M`; | |
| } | |
| export const formatAmount = (amount: number) => { | |
| if (!Number.isFinite(amount) || amount < 0) { | |
| return '0'; | |
| } | |
| if (amount < 1) { | |
| return (amount * 1000).toFixed(0); | |
| } else if (amount < 1000) { | |
| const decimals = amount % 1 === 0 ? 0 : 1; | |
| return `${amount.toFixed(decimals)}K`; | |
| } else { | |
| const inMillions = amount / 1000; | |
| const decimals = inMillions % 1 === 0 ? 0 : 1; | |
| return `${inMillions.toFixed(decimals)}M`; | |
| } | |
| }; |
| const [isLoading, setIsLoading] = useState(false); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Separate loading states for different actions
Using a single loading state for both registration and claiming could lead to race conditions. Consider using separate states for each action.
- const [isLoading, setIsLoading] = useState(false);
+ const [isRegistering, setIsRegistering] = useState(false);
+ const [isClaiming, setIsClaiming] = useState(false);Then update the button's loading state:
- isLoading={isLoading}
+ isLoading={registrationClosed ? isClaiming : isRegistering}Committable suggestion skipped: line range outside the PR's diff.
| @@ -12,7 +12,7 @@ use s0_eternum::{ | |||
| }; | |||
| use starknet::ContractAddress; | |||
|
|
|||
| const LEADERBOARD_REGISTRATION_PERIOD: u64 = 604800; // one week | |||
| const LEADERBOARD_REGISTRATION_PERIOD: u64 = 3600; // one week | |||
There was a problem hiding this comment.
💡 Codebase verification
Based on the search results, I can now provide a final response as I have sufficient information about how the registration period is used in the codebase:
Reducing registration period from 1 week to 1 hour requires careful consideration
The registration period is a critical timing window that affects several key gameplay mechanics:
- Players must register within this window to participate in the leaderboard (
register_to_leaderboardfunction) - The system enforces strict checks to prevent registration after this period (
"Registration period is over"error) - Reward claiming is only possible after this period ends (
"Claiming period hasn't started yet"error) - Each address can only register once during this period to prevent point inflation
Reducing this window from 1 week to 1 hour (3600 seconds) significantly limits the time players have to:
- Complete their contributions
- Register their points
- Participate in the competitive gameplay
This could negatively impact player engagement and fairness, especially for players in different time zones or with limited playing time availability.
🔗 Analysis chain
Verify impact of reduced leaderboard registration period.
The registration period has been significantly reduced from 1 week to 1 hour, which could impact user experience and fairness.
Let me generate additional scripts to better understand the impact of this change by looking for any time-dependent logic around leaderboard registration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for dependencies on LEADERBOARD_REGISTRATION_PERIOD
# Expected: Find any code that relies on this timing window
# Search for direct references
echo "Searching for direct references to LEADERBOARD_REGISTRATION_PERIOD..."
rg -A 3 "LEADERBOARD_REGISTRATION_PERIOD"
# Search for leaderboard registration related code
echo "Searching for leaderboard registration logic..."
ast-grep --pattern 'registration_end_timestamp = $$$LEADERBOARD_REGISTRATION_PERIOD$$$'
Length of output: 1909
Script:
#!/bin/bash
# Search for leaderboard registration time-related logic
echo "Searching for registration timestamp checks..."
rg -A 5 "registration_end_timestamp"
# Search for any leaderboard-related structs and their usage
echo "Searching for leaderboard struct definition..."
ast-grep --pattern 'struct Leaderboard {
$$$
}'
# Look for any registration-related functions
echo "Searching for registration-related functions..."
rg -A 10 "register|registration" -g "*.cairo"
Length of output: 30463
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
eternum-docs/src/components/BuildingCard.tsx (1)
27-33: Improvement: Consider responsive image sizingThe fixed width of 250px might not be optimal for all screen sizes.
Consider using responsive classes:
-<img key={index} src={img.src} alt={img.alt} width="250" /> +<img key={index} src={img.src} alt={img.alt} className="w-[250px] md:w-[300px] lg:w-[350px]" />
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
eternum-docs/docs/public/buildings/dragonhide.pngis excluded by!**/*.png
📒 Files selected for processing (2)
eternum-docs/docs/pages/mechanics/realm/buildings.mdx(1 hunks)eternum-docs/src/components/BuildingCard.tsx(2 hunks)
🔇 Additional comments (5)
eternum-docs/src/components/BuildingCard.tsx (2)
1-1: LGTM: Clean implementation of population capacity feature
The addition of BUILDING_CAPACITY import and its usage with fallback to 0 is well implemented.
Also applies to: 20-20
36-49: LGTM: Well-structured population stats display
The conditional rendering logic for population and capacity is clean and maintainable.
eternum-docs/docs/pages/mechanics/realm/buildings.mdx (3)
47-52: LGTM: Clear documentation for Fishing Village
The building description is concise and follows the established documentation pattern.
66-66: LGTM: Resource Facility images updated consistently
The image order follows the logical progression of resource types.
Also applies to: 68-68
54-59: Verify Market building's $LORDS consumption mechanism
While the documentation is clear, we should verify if the $LORDS consumption mechanism is implemented in the game logic.
✅ Verification successful
Based on the search results, I can now generate a final response as we have sufficient evidence about the Market's $LORDS consumption mechanism.
Market's $LORDS consumption mechanism is correctly implemented
The codebase shows that the Market building's $LORDS consumption is properly implemented through a sophisticated AMM (Automated Market Maker) system:
- The Market contract maintains $LORDS reserves (
lords_amount) and handles token exchanges - $LORDS consumption occurs through:
- Direct swaps where players spend $LORDS to get resources
- Liquidity provision where $LORDS are locked in the market
- Trading fees in $LORDS that are collected from transactions
The implementation aligns with the documentation's claim about $LORDS consumption, with additional sophistication through an AMM model that ensures price stability and liquidity.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for Market building's $LORDS consumption implementation
ast-grep --pattern 'function $_($$_) {
$$$
LORDS$$$
$$$
Market$$$
$$$
}'
Length of output: 84
Script:
#!/bin/bash
# Let's try a broader search for Market and LORDS related code
rg -i "market.*lords" -A 5 -B 5
# Also search for any Market-related contract or interface definitions
ast-grep --pattern 'contract $_Market {
$$$
}'
# Search for Market-related functions with a more relaxed pattern
ast-grep --pattern 'function $_ {
$$$
Market$$$
}'
# Look for Market configuration or constants
rg "Market.*cost|MARKET.*COST" -i
Length of output: 41470
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores