feat: update production #2670
feat: update production #2670
Conversation
|
Hi @credence0x! You need to be added as a user to interact with me. Please ask @ponderingdemocritus to add you on the settings page. You are receiving this comment because I am set to review all PRs. That is configurable here. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis pull request introduces a comprehensive overhaul of resource production and labor management across multiple components of the Eternum game system. The changes focus on replacing the existing labor-based resource production model with a more flexible and nuanced approach that allows for scaling resource production through labor parameters. Key modifications include renaming constants, updating configuration structures, and implementing new strategies for resource burning and production. Changes
Sequence DiagramsequenceDiagram
participant ResourceManager
participant ProductionConfig
participant LaborStrategy
participant ResourceProduction
ResourceManager->>ProductionConfig: Set production parameters
ProductionConfig->>LaborStrategy: Define labor burn strategy
LaborStrategy-->>ProductionConfig: Configure resource conversion
ProductionConfig->>ResourceProduction: Apply production configuration
ResourceProduction-->>ResourceManager: Update resource output
Possibly related PRs
Suggested Reviewers
Poem
✨ Finishing Touches
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: 5
🧹 Nitpick comments (20)
contracts/game/src/systems/production/contracts.cairo (2)
22-24: Clarify naming forburn_other_resource_for_labor_production.
The name clearly indicates its purpose; however, consider whether "other" is necessary. If the function will always burn any resource (besides labor) to produce labor, you could shorten it toburn_resource_for_labor_production.
141-153: Deprecation and realm checks inburn_labor_resource_for_other_production.
The code ensures the callee is a realm structure and checks for resource rarity. Make sure depreciation logic (e.g., after repeated conversions) is tested thoroughly to avoid exploits.contracts/game/src/models/resource/production/production.cairo (4)
10-12: Confirm resource import usage.
Ensure each import (Resource,RESOURCE_PRECISION,ResourceImpl, etc.) is required. If not, consider removing to keep the code clean.
56-57: Typo inincrease_output_amout_left.
Consider fixing the spelling of "amount" in the method name to ensure internal consistency.-fn increase_output_amout_left(ref self: Production, amount: u128) { +fn increase_output_amount_left(ref self: Production, amount: u128) {
95-103: Revised harvest logic limiting production byoutput_amount_left.
This helps ensure we don't exceed the intended maximum. Confirm that out-of-range edge cases (e.g., stale production rate or large time jumps) are handled gracefully.
187-209: Integer rounding in depreciation.
When computing(labor_amount * (denom - num) / produced_resource_rarity / denom), watch for unintended rounding if the order of operations shrinks the result. It may be correct but worth verifying.config/deployer/config.ts (1)
379-379: Address code formatting warnings.The pipeline's lint stage flagged formatting issues. Consider running Prettier or the project's recommended linter to fix these.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
packages/core/src/dojo/contract-components.ts (2)
1054-1054: Newoutput_amount_leftfieldAdding
output_amount_lefthelps track leftover outputs per tick. Consider adding test coverage to ensure this field updates correctly and doesn't overflow with large resource values.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
Line range hint
868-1062: Fix Prettier formattingThe lint pipeline shows a formatting discrepancy. Please run the project’s Prettier command (e.g.,
npm run formator equivalent) to resolve these warnings.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
packages/core/src/constants/buildings.ts (1)
25-25: Fix Prettier formattingAs indicated by the pipeline lint warnings, run Prettier to correct any style conflicts. This ensures consistent formatting across the codebase.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
client/apps/game/src/ui/config.tsx (1)
71-71: Consider updating the image path for Labor building.The Labor building is currently using the image meant for walls (
walls.png). This might confuse users as it doesn't visually represent the building's purpose. Consider creating and using a more appropriate image that reflects labor/work-related activities.config/environments/utils/building.ts (1)
117-120: Fix comment formatting.The comment about resource associations needs proper formatting.
Apply this diff to improve the comment:
-// Note: ensure that no resource associated with some other building -// is also associated with a resource building cost -// (e.g. dont add Labor resource here again because it is -// already associated with the Labor building) +// Note: Ensure that no resource associated with some other building +// is also associated with a resource building cost. +// (e.g., don't add Labor resource here again because it is +// already associated with the Labor building)🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
packages/core/src/utils/resources.ts (1)
170-171: Fix formatting: Remove extra empty lines.The code has extra empty lines that don't match Prettier standards.
Apply this diff to fix the formatting:
- - +🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
client/apps/game/src/three/scenes/constants.ts (1)
58-58: Consider creating a dedicated model for the Labor building.Currently, the Labor building reuses the market model (
market.glb). For better visual distinction and user experience, consider creating a unique model that better represents the Labor building's function.packages/core/src/provider/index.ts (1)
1923-1931: Consider improving calldata organization.The calldata construction could be more maintainable by:
- Using a helper function to construct the calldata
- Adding comments to document the parameter order
- call.resource_type, - call.amount_per_building_per_tick, - call.labor_burn_strategy.resource_rarity, - call.labor_burn_strategy.depreciation_percent_num, - call.labor_burn_strategy.depreciation_percent_denom, - call.labor_burn_strategy.wheat_burn_per_labor, - call.labor_burn_strategy.fish_burn_per_labor, - call.predefined_resource_burn_cost.length, - ...call.predefined_resource_burn_cost.flatMap(({ resource, amount }) => [resource, amount]), + // Resource configuration + call.resource_type, + call.amount_per_building_per_tick, + + // Labor burn strategy + ...constructLaborBurnStrategyCalldata(call.labor_burn_strategy), + + // Predefined resource burn costs + ...constructResourceBurnCostCalldata(call.predefined_resource_burn_cost),🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
contracts/common/addresses/local.json (1)
1-37: Fix code formatting to match Prettier standards.The pipeline indicates formatting issues. Please run Prettier to format this file according to the project's standards.
{ - "seasonPass": "0x6a7493de87c995a51e7d3373b7c9bb0b9908d1d095703c951fc50b66b2b76e4", - "realms": "0x14a9f2c29a9c316ac4594abe33bf62e030c10cb5eab12db0ccee4e09d3b19c8", - "lords": "0x97d987bcb1f9e38918f9fed2e4cffc44e36151600bc252303469d2271b031", + "seasonPass": "0x6a7493de87c995a51e7d3373b7c9bb0b9908d1d095703c951fc50b66b2b76e4", + "realms": "0x14a9f2c29a9c316ac4594abe33bf62e030c10cb5eab12db0ccee4e09d3b19c8", + "lords": "0x97d987bcb1f9e38918f9fed2e4cffc44e36151600bc252303469d2271b031", "resources": { - "STONE": [1,"0x493d207f45b37e4bdbfe5dd470e2e6647e06a7d9f46f7303108c1e5bcdcea8c"], + "STONE": [1, "0x493d207f45b37e4bdbfe5dd470e2e6647e06a7d9f46f7303108c1e5bcdcea8c"], // Apply similar spacing fixes to other resource entries } }🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
contracts/game/manifest_local.json (2)
Line range hint
6615-6648: LGTM! Labor conversion functions look well-designed.The new labor conversion functions provide a clear interface for:
- Converting resources to labor
- Converting labor back to resources
Consider adding events to track these conversions for analytics and debugging.
3297-3297: Architectural change: Walls replaced with Labor system.The replacement of Walls with Labor represents a significant architectural change that affects both building categories and production systems. The integration looks complete across:
- Building categories enum
- Production system functions
- Resource conversion strategies
Consider documenting this architectural change in the project's technical documentation to help other developers understand the new labor-based economy.
Also applies to: 6798-6800
config/environments/utils/resource.ts (1)
Line range hint
1-428: Fix code formatting to match Prettier standards.The pipeline indicates formatting issues. Please run Prettier to fix the formatting.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
contracts/game/src/models/resource/resource.cairo (1)
88-88: Add more context to the TODO comment.The TODO comment should explain why using
felt252would be better than the current implementation and include a ticket reference for tracking.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
client/apps/game/src/three/scenes/constants.ts(2 hunks)client/apps/game/src/ui/components/construction/select-preview-building.tsx(1 hunks)client/apps/game/src/ui/config.tsx(1 hunks)config/deployer/config.ts(3 hunks)config/environments/_shared_.ts(3 hunks)config/environments/utils/building.ts(5 hunks)config/environments/utils/resource.ts(4 hunks)contracts/common/addresses/local.json(1 hunks)contracts/game/manifest_local.json(28 hunks)contracts/game/src/constants.cairo(4 hunks)contracts/game/src/models.cairo(0 hunks)contracts/game/src/models/config.cairo(1 hunks)contracts/game/src/models/quest.cairo(1 hunks)contracts/game/src/models/resource/production/building.cairo(7 hunks)contracts/game/src/models/resource/production/labor.cairo(0 hunks)contracts/game/src/models/resource/production/production.cairo(4 hunks)contracts/game/src/models/resource/resource.cairo(4 hunks)contracts/game/src/systems/config/contracts.cairo(4 hunks)contracts/game/src/systems/map/map_generation.cairo(3 hunks)contracts/game/src/systems/production/contracts.cairo(4 hunks)contracts/game/src/systems/realm/contracts.cairo(0 hunks)packages/core/src/constants/buildings.ts(1 hunks)packages/core/src/constants/index.ts(1 hunks)packages/core/src/constants/structures.ts(4 hunks)packages/core/src/dojo/contract-components.ts(2 hunks)packages/core/src/managers/config-manager.ts(3 hunks)packages/core/src/managers/resource-manager.ts(1 hunks)packages/core/src/provider/index.ts(1 hunks)packages/core/src/types/common.ts(3 hunks)packages/core/src/types/provider.ts(1 hunks)packages/core/src/utils/resources.ts(2 hunks)
💤 Files with no reviewable changes (3)
- contracts/game/src/systems/realm/contracts.cairo
- contracts/game/src/models.cairo
- contracts/game/src/models/resource/production/labor.cairo
✅ Files skipped from review due to trivial changes (1)
- contracts/game/src/models/quest.cairo
🧰 Additional context used
🪛 GitHub Actions: lint
packages/core/src/managers/resource-manager.ts
[warning] Code formatting does not match Prettier standards
packages/core/src/constants/buildings.ts
[warning] Code formatting does not match Prettier standards
config/deployer/config.ts
[warning] Code formatting does not match Prettier standards
packages/core/src/utils/resources.ts
[warning] Code formatting does not match Prettier standards
config/environments/utils/resource.ts
[warning] Code formatting does not match Prettier standards
packages/core/src/provider/index.ts
[warning] Code formatting does not match Prettier standards
packages/core/src/dojo/contract-components.ts
[warning] Code formatting does not match Prettier standards
config/environments/utils/building.ts
[warning] Code formatting does not match Prettier standards
contracts/common/addresses/local.json
[warning] Code formatting does not match Prettier standards
🔇 Additional comments (64)
contracts/game/src/systems/production/contracts.cairo (7)
4-4: Confirm usage of newly importedProductionStrategyImpl.
It's good to see the explicit import ofProductionStrategyImpl, which is utilized in the new methods.
25-25: No changes in this blank line.
26-28: Ensure consistent parameter ordering inburn_labor_resource_for_other_production.
Review the ordering of parameters (from_entity_id,labor_amount,produced_resource_type) to confirm it matches usage patterns in the codebase.
30-32: Confirm coverage ofburn_other_predefined_resources_for_resource.
This newly added method has a distinctive name. Ensure tests or usage logs exist to confirm all required resources are appropriately burned.
45-45: New import ofResourceCost.
This addition helps track resource costs directly. Good addition if used within new burning methods.
48-48: Reference toProductionStrategyImpl.
Thank you for consistently referencing the new implementation. This helps maintain clarity regarding resource conversions.
125-137: Validate resource type checks inburn_other_resource_for_labor_production.
The logic checksresource_type.is_non_zero(), but consider verifying that the resource_type is known or whitelisted. A resource type like0xFFmight pass.is_non_zero()but be invalid.contracts/game/src/models/resource/production/production.cairo (7)
8-8: Check references for new strategy imports.
The added imports forLaborBurnPrStrategyandMultipleResourceBurnPrStrategyare used below. Looks consistent.
23-25: Addedoutput_amount_leftfield inProduction.
This field replaces labor-based logic with output-based. Excellent approach for more flexible resource production.
61-62: Double-check underflow potential indecrease_output_amout_left.
Ensure upstream logic guaranteesoutput_amount_left >= amount; otherwise, consider protective checks to avoid negative values.
115-115: NewProductionStrategyImpl.
Introduction of this implementation centralizes resource conversion strategies, which is helpful for maintainability.
118-137:burn_other_resource_for_labor_productionfunctionality.
Balance changes can be drastic ifresource_rarityis large. Ensure the game economy does not allow quick resource inflation.
142-153:burn_labor_resource_for_other_productiondepreciation flow.
The approach subtracts a percentage from the potential output. Confirm that repeated conversions won't yield fractional remainders that cause integer rounding issues.
178-186: Validate fish/wheat payment inResourceFoodImpl::pay.
Check if there's a scenario in which fish or wheat is insufficient, possibly leading to a partial burn or failing transaction.packages/core/src/types/provider.ts (2)
469-471: Renamed and reorganized production props.
Replacingamountwithamount_per_building_per_tickclarifies the per-building logic. Includinglabor_burn_strategyandpredefined_resource_burn_costprovides flexibility for multiple resource mechanics.
475-481:LaborBurnProductionStrategystruct.
These new fields—resource_rarity,depreciation_percent_(num|denom),wheat_burn_per_labor,fish_burn_per_labor—reflect the new production mechanics. Confirm all are defined as big numbers in actual usage.contracts/game/src/models/config.cairo (3)
532-538: Check references to newly added fields.Ensure that any old references (e.g.,
produced_amount,labor_cost) have been replaced with the new fieldsamount_per_building_per_tick,labor_burn_strategy, andmultiple_resource_burn_strategy.✅ Verification successful
Migration to new fields is complete ✅
The codebase shows proper integration of the new fields (
amount_per_building_per_tick,labor_burn_strategy, andmultiple_resource_burn_strategy), and there are no remaining references to the old fields that need to be updated. The occurrences of "produced" in the codebase are either in manifest files or are different local variables, not related to the old field names.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify there are no remaining references to old fields rg -A 5 'produced_amount|labor_cost'Length of output: 2605
Script:
#!/bin/bash # Search for struct definitions and field accesses ast-grep --pattern 'struct $_ { $$$ produced_amount: $_ $$$ }' ast-grep --pattern 'struct $_ { $$$ labor_cost: $_ $$$ }' # Search for new field references to confirm migration rg -A 5 'amount_per_building_per_tick|labor_burn_strategy|multiple_resource_burn_strategy' # Search for any remaining references with different patterns rg -i '\b(produced.*amount|labor.*cost)\b'Length of output: 21876
540-599: Validate resource_rarity to avoid potential overflow.The
resource_rarityfield is an unboundedu128. Large values could cause multiplications leading to unexpected overflow or inefficiencies. Ensure there's a sensible cap or validation in place.
600-620: Enforce limits forrequired_resources_count.
required_resources_countis declared asu8. The code should gracefully handle or prevent scenarios where more than 255 resources are required for a single production.contracts/game/src/systems/config/contracts.cairo (5)
6-7: New imports look correct.No issues here. The new strategies are being properly imported.
135-141: Consider default scenarios.If
labor_burn_strategyorpredefined_resource_burn_costis empty, ensure the system gracefully handles production.
234-235: Minimal import changes.No issues noted.
239-240: Minimal import changes.No concerns about reorganization here.
607-643: Production configuration updates are solid, but verify naming consistency.The code references
ProductionConfig, while the summary mentionsResourceFactoryConfig. This inconsistency may cause confusion. Consider synchronizing naming to improve clarity.
[refactor_suggestion_essential, inconsistent_summary]- let mut resource_production_config: ProductionConfig = world.read_model(resource_type); + let mut resource_factory_config: ResourceFactoryConfig = world.read_model(resource_type);config/deployer/config.ts (5)
16-16: Import statement is correct.The new function
scaleResourceProductionByLaborParamsis properly imported.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
200-202: Ensure consistent indexing among arrays.
scaledResourceOutputs,scaledResourceInputs, andscaledResourceProductionByLaborParamsshould share matching keys to avoid mismatches.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
205-207: Calldata composition looks good.Merging
amount_per_building_per_tick,predefined_resource_burn_cost, andlabor_burn_strategyinto a single object is well-structured.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
215-216: Console logs are consistent.No issues with the new logging statements.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
194-198: Provide fallback handling.If
resourceProductionByLaborParamslacks data for certain resources, it may cause errors. Consider adding checks or fallbacks for missing entries.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
packages/core/src/dojo/contract-components.ts (5)
868-868: New ProductionConfig component addedThe addition of the
ProductionConfigcomponent is aligned with the new strategies for resource production as described in the PR objectives. Ensure all references to legacy production logic have been removed or updated to avoid undefined behavior.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
874-881: Validate consistency ofLaborBurnPrStrategyfieldsThese nested fields correctly match the PR’s new strategy specification. Confirm that each field (e.g.,
resource_rarity,depreciation_percent_num, etc.) aligns with the Cairo contract definitions to avoid schema mismatches.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
882-884: Ensure correct usage ofMultipleResourceBurnPrStrategyThe second strategy is well-defined. Verify that system calls correctly reference
required_resources_idandrequired_resources_countin the game logic to prevent resource ID mismatch or off-by-one errors.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
890-892: Check field order intypesarrayThe
typesarray must match the exact order of the defined fields. Double-check that each subfield mapping is accurate, to prevent runtime deserialization errors.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
1062-1062: Maintain consistent data layoutThe new
typesentry correctly includes theoutput_amount_leftfield asu128. Verify that downstream code readingResource.productionuses the correct index for this new field.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
packages/core/src/constants/index.ts (1)
37-37: ReplacingDemonhidewithLaborChanging the numeric ID from
DemonhidetoLabormay cause backward incompatibility if any existing data still relies on resource ID23. Confirm all references and data migrations are updated to avoid orphaned references toDemonhide.packages/core/src/constants/buildings.ts (1)
25-25: IntroducedLaborentryReplacing
WallswithLaboraligns with the PR’s shift toward labor-based production. Verify any references toWallsacross the codebase are removed, ensuring no inconsistent building references remain.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
packages/core/src/managers/resource-manager.ts (2)
101-103: LGTM! Production calculation logic looks good.The change from labor units to output-based calculation aligns well with the new resource production strategy introduced in this PR.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
116-119: LGTM! Production end calculation is correct.The calculation now properly uses output_amount_left while maintaining special handling for food resources.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
packages/core/src/constants/structures.ts (1)
30-30: LGTM! Consistent replacement of Walls with Labor.The changes maintain consistency across the enum, string mapping, and related functions while preserving the numeric value (12).
Also applies to: 49-49, 81-82, 118-119
config/environments/utils/building.ts (1)
18-18: Verify Labor building configuration values.Please confirm if these values are appropriate for the Labor building:
- Capacity: 0 (no storage)
- Population requirement: 2 workers
- Produces: ResourcesIds.Labor
These settings will affect the game's economic balance.
Also applies to: 37-37, 56-56
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
packages/core/src/utils/resources.ts (1)
172-183: LGTM! The scaling function is well-implemented.The function correctly scales the labor-related values while preserving other properties, aligning with the new
LaborBurnPrStrategyrequirements.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
contracts/game/src/systems/map/map_generation.cairo (2)
40-40: LGTM! Import changes align with the new production-based approach.The replacement of
LaborImplwithProductionandProductionTraitcorrectly reflects the transition from labor-based to production-based resource management.
196-205: LGTM! Resource production update is correctly implemented.The changes effectively transition from labor-based to production-based resource management:
- Correctly retrieves the shards resource
- Updates the production output amount directly
- Properly saves the updated resource state
client/apps/game/src/three/scenes/constants.ts (1)
129-129: LGTM! Building model path is correctly mapped.The Labor building type is properly mapped to its model path.
config/environments/_shared_.ts (2)
29-29: LGTM! Import added for labor-based production configuration.The import of
RESOURCE_PRODUCTION_THROUGH_LABORcorrectly supports the new labor-based production strategy.
160-160: LGTM! Configuration structure improvements.The changes enhance the configuration structure by:
- Adding labor-based production parameters
- Clearly separating resource and non-resource building costs
- Maintaining backward compatibility
Also applies to: 249-250
packages/core/src/managers/config-manager.ts (1)
116-116: LGTM! Consistent refactoring of production amount property.The changes consistently replace
produced_amountwithamount_per_building_per_tickacross the codebase, improving clarity around how resource production is calculated.Also applies to: 504-504
contracts/game/src/models/resource/production/building.cairo (4)
67-67: LGTM! Consistent enum and implementation changes.The BuildingCategory enum and its implementation in BuildingCategoryIntoFelt252 consistently replace Walls with Labor while maintaining the same numeric value (12).
Also applies to: 86-86
117-117: LGTM! Consistent building type behavior changes.The produced_resource and boost_adjacent_building_production_by methods have been updated to handle the new Labor building type appropriately:
- Labor produces ResourceTypes::LABOR
- Labor provides 0 production boost to adjacent buildings
Also applies to: 136-136
220-223: LGTM! Production amount calculation updated.The production_amount calculation has been updated to use amount_per_building_per_tick, consistent with the changes in config-manager.ts.
290-291: LGTM! Consistent method chaining.The method chaining syntax has been maintained consistently throughout the code.
Also applies to: 302-303
client/apps/game/src/ui/components/construction/select-preview-building.tsx (1)
60-60: LGTM! UI filtering updated for new building type.The building type filtering has been updated to exclude "Labor" instead of "Walls", maintaining consistency with the backend changes.
packages/core/src/provider/index.ts (1)
1914-1936: Verify error handling in the new production config method.While the implementation looks solid, consider adding parameter validation and error handling for:
- Invalid resource types
- Zero or negative amounts
- Invalid labor burn strategy parameters
Run this script to check for existing error handling patterns in similar config methods:
✅ Verification successful
Existing error handling is sufficient ✅
The implementation already has multiple layers of validation:
- Type safety through TypeScript interfaces
- Contract-level validation through reverts
- Transaction monitoring and error reporting
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for error handling patterns in config-related methods rg -A 5 "set_.*_config.*{" packages/core/src/Length of output: 143
Script:
#!/bin/bash # Search for config setter methods rg -A 5 "set.*config" packages/core/src/ # Search for validation patterns rg -B 2 -A 5 "validate|check|assert" packages/core/src/ # Look for error handling utilities rg -B 2 -A 5 "throw|error|invalid" packages/core/src/provider/Length of output: 29723
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
contracts/common/addresses/local.json (2)
6-35: Verify resource index sequence and uniqueness.There appears to be a gap in the resource indices (jumps from 22 to 24). Please verify:
- The missing index 23 is intentional
- All resource indices are unique
- The sequence aligns with the game's resource hierarchy
✅ Verification successful
Resource indices verified - gap is intentional and indices are unique
The gap between indices 22 and 24 appears to be an intentional design choice that separates crafting materials from special items. All indices are unique and follow a logical progression aligned with the game's resource hierarchy.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify resource indices are unique and properly sequenced echo "Checking resource indices..." jq -r '.resources | to_entries[] | [.value[0], .key] | @tsv' contracts/common/addresses/local.json | \ sort -n | \ awk ' BEGIN { prev=0 } { if ($1 != prev + 1) { printf "Gap found between indices %d and %d (resource: %s)\n", prev, $1, $2 } prev=$1 } END { print "\nResource index mapping:" print "----------------------" }' # Check for duplicate indices echo -e "\nChecking for duplicate indices..." jq -r '.resources | to_entries[] | [.value[0], .key] | @tsv' contracts/common/addresses/local.json | \ sort -n | \ awk '{ if (indices[$1]++) { printf "Duplicate index %d found for resource: %s\n", $1, $2 } else { printf "%d\t%s\n", $1, $2 } }'Length of output: 1227
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
2-4: Verify contract address validity and consistency.The
lordsaddress appears in both the main contracts section and the resources section. Please verify:
- The addresses are valid on the target network
- The
lordsaddress is consistent between line 4 and line 35✅ Verification successful
Contract addresses are consistent and properly formatted
The
lordscontract address is consistent between the main contracts section and resources section. All addresses follow the correct format for StarkNet addresses.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check lords address consistency and verify contract addresses exist on network # Check lords address consistency LORDS_MAIN=$(jq -r '.lords' contracts/common/addresses/local.json) LORDS_RESOURCE=$(jq -r '.resources.LORDS[1]' contracts/common/addresses/local.json) if [ "$LORDS_MAIN" = "$LORDS_RESOURCE" ]; then echo "✓ Lords address is consistent" else echo "✗ Lords address mismatch:" echo "Main: $LORDS_MAIN" echo "Resource: $LORDS_RESOURCE" fi # List all contract addresses for manual verification echo -e "\nContract addresses to verify:" jq -r '. | to_entries | .[] | select(.key != "resources") | "\(.key): \(.value)"' contracts/common/addresses/local.jsonLength of output: 909
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
contracts/game/manifest_local.json (1)
2983-3008: Verify economic parameters in LaborBurnPrStrategy.The new LaborBurnPrStrategy structure introduces critical economic parameters that will affect the game's resource conversion mechanics. Please ensure these parameters have been properly balanced and tested:
- resource_rarity: Affects conversion rates
- depreciation_percent_num/denom: Controls value loss
- wheat/fish_burn_per_labor: Determines food costs
Consider adding parameter constraints or validation in the implementation to prevent economic exploits.
contracts/game/src/constants.cairo (3)
106-106: LGTM! Consistent renaming from DEMONHIDE to LABOR.The renaming is applied consistently in both the constant definition and the corresponding string representation.
Also applies to: 164-164
268-268: LGTM! LABOR resource added to the list.The LABOR resource is correctly added to the list of resources, maintaining consistency with the renaming changes.
Line range hint
1-290: Verify the impact of removing resource ID range constants.The removal of
FIRST_REGULAR_RESOURCE_ID,LAST_REGULAR_RESOURCE_ID,FIRST_LABOR_RESOURCE_ID, andLAST_LABOR_RESOURCE_IDconstants simplifies the code but may affect systems that relied on these for resource validation.Let's verify the usage of these constants across the codebase:
✅ Verification successful
Removal of resource ID range constants is safe
The codebase has adopted more robust resource validation mechanisms through explicit enumeration, type-safe constants, and function-based validation, making the removed range constants obsolete.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the removed constants # that might cause issues. echo "Searching for references to removed constants..." rg -l "FIRST_REGULAR_RESOURCE_ID|LAST_REGULAR_RESOURCE_ID|FIRST_LABOR_RESOURCE_ID|LAST_LABOR_RESOURCE_ID" echo "Searching for potential resource ID validation patterns..." ast-grep --pattern 'if $resource_id $op $range { $$$; }'Length of output: 400
Script:
#!/bin/bash # Broader search for resource validation patterns and usage echo "Searching for resource validation patterns..." rg -A 2 "resource.*id.*=|resource.*type.*=" echo -e "\nChecking uses of all_resource_ids..." rg -A 2 "all_resource_ids\(\)" echo -e "\nChecking for resource type validations..." rg -A 2 "resource.*type.*valid|invalid.*resource"Length of output: 66469
config/environments/utils/resource.ts (2)
27-27: LGTM! Appropriate weight assigned to LABOR resource.The weight of 1000 grams is consistent with other resource weights in the system.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
69-69: Verify if the low production rate for LABOR is intentional.The production rate of 0.005 for LABOR is significantly lower than other resources (which are typically 30). While this might be intentional to make labor more scarce, please confirm if this aligns with the game's economic balance.
✅ Verification successful
The low production rate for LABOR (0.005) is verified as intentional
This aligns with the game's economic balance where military units have similarly low rates (0.04) and basic resources have higher rates (30-50). Making labor the scarcest resource (0.005) appears to be a deliberate design choice to create meaningful economic decisions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Compare LABOR production rate with other resources # to highlight the significant difference echo "Analyzing resource production rates..." rg -A 1 "ResourcesIds\.[A-Za-z]+\]: \d+(\.\d+)?" config/environments/utils/resource.ts | sort -k3 -nLength of output: 2166
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
packages/core/src/types/common.ts (1)
415-423: LGTM! Well-structured interface for labor production parameters.The
ProductionByLaborParamsinterface properly defines all necessary parameters for the labor-based production system.contracts/game/src/models/resource/resource.cairo (2)
177-177: LGTM! Simplified resource validation.The validation has been simplified to only check for non-zero resource type, which is sufficient given the removal of resource ID range constants.
275-281: LGTM! Improved production update efficiency.The code now only limits balance by storehouse capacity when there's a non-zero harvest, improving efficiency.
| // Burn other predefined resources for resource | ||
| // e.g. Wood, Stone, Coal for Gold | ||
| fn burn_other_predefined_resources_for_resource( | ||
| ref self: ContractState, from_entity_id: ID, produced_resource_type: u8, production_tick_count: u128 | ||
| ) { | ||
| let mut world: WorldStorage = self.world(DEFAULT_NS()); | ||
| SeasonImpl::assert_season_is_not_over(world); | ||
|
|
||
| let entity_owner: EntityOwner = world.read_model(from_entity_id); | ||
| entity_owner.assert_caller_owner(world); | ||
|
|
||
| ProductionStrategyImpl::burn_other_predefined_resources_for_resource( | ||
| ref world, from_entity_id, produced_resource_type, production_tick_count |
There was a problem hiding this comment.
Review production tick logic in burn_other_predefined_resources_for_resource.
This method calculates resource consumption over multiple ticks (production_tick_count). Verify users can't push unrealistic tick counts to bypass normal resource limitations.
|
|
||
| // burn multiple other predefined resources for production of one resource | ||
| // e.g burn stone, coal and copper for production of gold | ||
| fn burn_other_predefined_resources_for_resource( | ||
| ref world: WorldStorage, from_entity_id: ID, produced_resource_type: u8, production_tick_count: u128 | ||
| ) { | ||
| assert!(produced_resource_type.is_non_zero(), "wrong resource type"); | ||
| assert!(production_tick_count.is_non_zero(), "zero production tick count"); | ||
| assert!(from_entity_id.is_non_zero(), "zero entity id"); | ||
|
|
||
| // ensure entity is a structure | ||
| let from_entity_structure: Structure = world.read_model(from_entity_id); | ||
| assert!(from_entity_structure.category == StructureCategory::Realm, "structure is not a realm"); | ||
|
|
||
|
|
||
| // ensure there is a config for this labor resource | ||
| let produced_resource_production_config: ProductionConfig = world.read_model(produced_resource_type); | ||
| let produced_resource_multiple_resource_burn_strategy: MultipleResourceBurnPrStrategy = | ||
| produced_resource_production_config | ||
| .multiple_resource_burn_strategy; | ||
| let other_resources_count = produced_resource_multiple_resource_burn_strategy.required_resources_count; | ||
| let other_resources_id = produced_resource_multiple_resource_burn_strategy.required_resources_id; | ||
| assert!(other_resources_count.is_non_zero(), "specified resource can't be produced from other resources"); | ||
|
|
||
| for i in 0 | ||
| ..other_resources_count { | ||
| let other_resource_cost: ResourceCost = world.read_model((other_resources_id, i)); | ||
| let other_resource_type = other_resource_cost.resource_type; | ||
| let other_resource_amount = other_resource_cost.amount; | ||
| assert!(other_resource_amount.is_non_zero(), "specified resource cost is 0"); | ||
|
|
||
| // make payment for produced resource | ||
| let mut other_resource = ResourceImpl::get(ref world, (from_entity_id, other_resource_type)); | ||
| other_resource.burn(other_resource_amount * production_tick_count); | ||
| other_resource.save(ref world); | ||
| }; | ||
|
|
||
| // add produced resource amount to factory | ||
| let mut produced_resource = ResourceImpl::get(ref world, (from_entity_id, produced_resource_type)); | ||
| let mut produced_resource_production = produced_resource.production; | ||
| produced_resource_production.increase_output_amout_left(production_tick_count * RESOURCE_PRECISION); | ||
| produced_resource.production = produced_resource_production; | ||
| produced_resource.save(ref world); | ||
| // todo add event here | ||
| } |
There was a problem hiding this comment.
burn_other_predefined_resources_for_resource multi-resource burn.
The code loops over other_resource_cost, multiplying by production_tick_count. Watch for integer overflow if other_resources_count or production_tick_count becomes extremely large.
| @@ -525,27 +525,102 @@ pub struct LevelingConfig { | |||
|
|
|||
| #[derive(IntrospectPacked, Copy, Drop, Serde)] | |||
| #[dojo::model] | |||
| // rename to ResourceFactoryConfig | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Rename the struct to align with the comment.
The code still references ProductionConfig while the comment suggests renaming it to ResourceFactoryConfig. To avoid confusion, consider renaming the struct or removing the comment if no longer valid.
-// rename to ResourceFactoryConfig
-pub struct ProductionConfig {
+pub struct ResourceFactoryConfig {Committable suggestion skipped: line range outside the PR's diff.
| if (!production || production.building_count == 0) return 0n; | ||
|
|
||
| let totalAmountProduced = BigInt(currentTick - production.last_updated_tick) * BigInt(production.production_rate); | ||
| if (!this.isFood() && totalAmountProduced > production.output_amount_left) { | ||
| totalAmountProduced = production.output_amount_left; | ||
| } | ||
|
|
||
| return laborUnitsBurned; | ||
| return totalAmountProduced; |
There was a problem hiding this comment.
Fix code formatting.
The code needs formatting to match Prettier standards. There are extra blank lines and inconsistent spacing.
Apply this diff to fix the formatting:
if (!production || production.building_count == 0) return 0n;
-
let totalAmountProduced = BigInt(currentTick - production.last_updated_tick) * BigInt(production.production_rate);
if (!this.isFood() && totalAmountProduced > production.output_amount_left) {
totalAmountProduced = production.output_amount_left;
}
-
return totalAmountProduced;📝 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.
| if (!production || production.building_count == 0) return 0n; | |
| let totalAmountProduced = BigInt(currentTick - production.last_updated_tick) * BigInt(production.production_rate); | |
| if (!this.isFood() && totalAmountProduced > production.output_amount_left) { | |
| totalAmountProduced = production.output_amount_left; | |
| } | |
| return laborUnitsBurned; | |
| return totalAmountProduced; | |
| if (!production || production.building_count == 0) return 0n; | |
| let totalAmountProduced = BigInt(currentTick - production.last_updated_tick) * BigInt(production.production_rate); | |
| if (!this.isFood() && totalAmountProduced > production.output_amount_left) { | |
| totalAmountProduced = production.output_amount_left; | |
| } | |
| return totalAmountProduced; |
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
| }, | ||
| [ResourcesIds.TwilightQuartz]: { | ||
| resource_rarity: 4518, | ||
| depreciation_percent_num: 10, | ||
| depreciation_percent_denom: 100, | ||
| wheat_burn_per_labor: 250_000, | ||
| fish_burn_per_labor: 300_000, | ||
| }, | ||
| [ResourcesIds.AlchemicalSilver]: { | ||
| resource_rarity: 5392, | ||
| depreciation_percent_num: 10, | ||
| depreciation_percent_denom: 100, | ||
| wheat_burn_per_labor: 250_000, | ||
| fish_burn_per_labor: 300_000, | ||
| }, | ||
| [ResourcesIds.Adamantine]: { | ||
| resource_rarity: 9120, | ||
| depreciation_percent_num: 10, | ||
| depreciation_percent_denom: 100, | ||
| wheat_burn_per_labor: 250_000, | ||
| fish_burn_per_labor: 300_000, | ||
| }, | ||
| [ResourcesIds.Mithral]: { | ||
| resource_rarity: 13553, | ||
| depreciation_percent_num: 10, | ||
| depreciation_percent_denom: 100, | ||
| wheat_burn_per_labor: 250_000, | ||
| fish_burn_per_labor: 300_000, | ||
| }, | ||
| [ResourcesIds.Dragonhide]: { | ||
| resource_rarity: 21792, | ||
| depreciation_percent_num: 10, | ||
| depreciation_percent_denom: 100, | ||
| wheat_burn_per_labor: 250_000, | ||
| fish_burn_per_labor: 300_000, | ||
| }, | ||
| [ResourcesIds.AncientFragment]: { | ||
| resource_rarity: 2098, | ||
| depreciation_percent_num: 10, | ||
| depreciation_percent_denom: 100, | ||
| wheat_burn_per_labor: 250_000, | ||
| fish_burn_per_labor: 300_000, | ||
| }, | ||
| [ResourcesIds.Donkey]: { | ||
| resource_rarity: 0, | ||
| depreciation_percent_num: 0, | ||
| depreciation_percent_denom: 0, | ||
| wheat_burn_per_labor: 0, | ||
| fish_burn_per_labor: 0, | ||
| }, | ||
| [ResourcesIds.Knight]: { | ||
| resource_rarity: 0, | ||
| depreciation_percent_num: 0, | ||
| depreciation_percent_denom: 0, | ||
| wheat_burn_per_labor: 0, | ||
| fish_burn_per_labor: 0, | ||
| }, | ||
| [ResourcesIds.Crossbowman]: { | ||
| resource_rarity: 0, | ||
| depreciation_percent_num: 0, | ||
| depreciation_percent_denom: 0, | ||
| wheat_burn_per_labor: 0, | ||
| fish_burn_per_labor: 0, | ||
| }, | ||
| [ResourcesIds.Paladin]: { | ||
| resource_rarity: 0, | ||
| depreciation_percent_num: 0, | ||
| depreciation_percent_denom: 0, | ||
| wheat_burn_per_labor: 0, | ||
| fish_burn_per_labor: 0, | ||
| }, | ||
| [ResourcesIds.Wheat]: { | ||
| resource_rarity: 0, | ||
| depreciation_percent_num: 0, | ||
| depreciation_percent_denom: 0, | ||
| wheat_burn_per_labor: 0, | ||
| fish_burn_per_labor: 0, | ||
| }, | ||
| [ResourcesIds.Fish]: { | ||
| resource_rarity: 0, | ||
| depreciation_percent_num: 0, | ||
| depreciation_percent_denom: 0, | ||
| wheat_burn_per_labor: 0, | ||
| fish_burn_per_labor: 0, | ||
| }, | ||
| [ResourcesIds.Lords]: { | ||
| resource_rarity: 0, | ||
| depreciation_percent_num: 0, | ||
| depreciation_percent_denom: 0, | ||
| wheat_burn_per_labor: 0, | ||
| fish_burn_per_labor: 0, | ||
| }, | ||
| [ResourcesIds.Labor]: { | ||
| resource_rarity: 0, | ||
| depreciation_percent_num: 0, | ||
| depreciation_percent_denom: 0, | ||
| wheat_burn_per_labor: 0, | ||
| fish_burn_per_labor: 0, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Review the resource production parameters for balance.
Several potential issues in the configuration:
- Wood has 100% depreciation while other resources have 10%
- Food costs (wheat_burn_per_labor and fish_burn_per_labor) are identical across all resources
- Military units and food resources have all values set to 0
Consider:
- Adjusting Wood's depreciation to match other resources
- Varying food costs based on resource rarity
- Documenting why military/food resources have 0 values
Apply this diff to fix Wood's depreciation:
[ResourcesIds.Wood]: {
resource_rarity: 100,
- depreciation_percent_num: 100,
+ depreciation_percent_num: 10,
depreciation_percent_denom: 100,
wheat_burn_per_labor: 250_000,
fish_burn_per_labor: 300_000,
},📝 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 RESOURCE_PRODUCTION_THROUGH_LABOR: ProductionByLaborParams = { | |
| [ResourcesIds.Wood]: { | |
| resource_rarity: 100, | |
| depreciation_percent_num: 100, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Stone]: { | |
| resource_rarity: 127, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Coal]: { | |
| resource_rarity: 131, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Copper]: { | |
| resource_rarity: 190, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Obsidian]: { | |
| resource_rarity: 226, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Silver]: { | |
| resource_rarity: 288, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Ironwood]: { | |
| resource_rarity: 425, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.ColdIron]: { | |
| resource_rarity: 524, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Gold]: { | |
| resource_rarity: 549, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Hartwood]: { | |
| resource_rarity: 844, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Diamonds]: { | |
| resource_rarity: 1672, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Sapphire]: { | |
| resource_rarity: 2030, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Ruby]: { | |
| resource_rarity: 2098, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.DeepCrystal]: { | |
| resource_rarity: 2098, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Ignium]: { | |
| resource_rarity: 2915, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.EtherealSilica]: { | |
| resource_rarity: 3095, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.TrueIce]: { | |
| resource_rarity: 3606, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.TwilightQuartz]: { | |
| resource_rarity: 4518, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.AlchemicalSilver]: { | |
| resource_rarity: 5392, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Adamantine]: { | |
| resource_rarity: 9120, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Mithral]: { | |
| resource_rarity: 13553, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Dragonhide]: { | |
| resource_rarity: 21792, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.AncientFragment]: { | |
| resource_rarity: 2098, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Donkey]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Knight]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Crossbowman]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Paladin]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Wheat]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Fish]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Lords]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Labor]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| }; | |
| export const RESOURCE_PRODUCTION_THROUGH_LABOR: ProductionByLaborParams = { | |
| [ResourcesIds.Wood]: { | |
| resource_rarity: 100, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Stone]: { | |
| resource_rarity: 127, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Coal]: { | |
| resource_rarity: 131, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Copper]: { | |
| resource_rarity: 190, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Obsidian]: { | |
| resource_rarity: 226, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Silver]: { | |
| resource_rarity: 288, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Ironwood]: { | |
| resource_rarity: 425, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.ColdIron]: { | |
| resource_rarity: 524, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Gold]: { | |
| resource_rarity: 549, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Hartwood]: { | |
| resource_rarity: 844, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Diamonds]: { | |
| resource_rarity: 1672, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Sapphire]: { | |
| resource_rarity: 2030, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Ruby]: { | |
| resource_rarity: 2098, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.DeepCrystal]: { | |
| resource_rarity: 2098, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Ignium]: { | |
| resource_rarity: 2915, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.EtherealSilica]: { | |
| resource_rarity: 3095, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.TrueIce]: { | |
| resource_rarity: 3606, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.TwilightQuartz]: { | |
| resource_rarity: 4518, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.AlchemicalSilver]: { | |
| resource_rarity: 5392, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Adamantine]: { | |
| resource_rarity: 9120, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Mithral]: { | |
| resource_rarity: 13553, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Dragonhide]: { | |
| resource_rarity: 21792, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.AncientFragment]: { | |
| resource_rarity: 2098, | |
| depreciation_percent_num: 10, | |
| depreciation_percent_denom: 100, | |
| wheat_burn_per_labor: 250_000, | |
| fish_burn_per_labor: 300_000, | |
| }, | |
| [ResourcesIds.Donkey]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Knight]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Crossbowman]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Paladin]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Wheat]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Fish]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Lords]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| [ResourcesIds.Labor]: { | |
| resource_rarity: 0, | |
| depreciation_percent_num: 0, | |
| depreciation_percent_denom: 0, | |
| wheat_burn_per_labor: 0, | |
| fish_burn_per_labor: 0, | |
| }, | |
| }; |
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not match Prettier standards
| @@ -55,7 +55,7 @@ enum BuildingFilenames { | |||
| Stable = "stable.glb", | |||
| Storehouse = "storehouse.glb", | |||
| TradingPost = "market.glb", | |||
| Walls = "market.glb", | |||
| Labor = "market.glb", | |||
There was a problem hiding this comment.
is this needed ? Labor should be the castle
| @@ -126,7 +126,7 @@ export const buildingModelPaths = { | |||
| [BuildingType.Stable]: BUILDINGS_MODELS_PATH + BuildingFilenames.Stable, | |||
| [BuildingType.Storehouse]: BUILDINGS_MODELS_PATH + BuildingFilenames.Storehouse, | |||
| [BuildingType.TradingPost]: BUILDINGS_MODELS_PATH + BuildingFilenames.TradingPost, | |||
| [BuildingType.Walls]: BUILDINGS_MODELS_PATH + BuildingFilenames.Walls, | |||
| [BuildingType.Labor]: BUILDINGS_MODELS_PATH + BuildingFilenames.Labor, | |||
| /// Make production labor resource from input resources | ||
| fn make_production_labor(ref self: ContractState, entity_id: ID, resource_type: u8, labor_amount: u128) { | ||
| /// Burn other resource for production of labor | ||
| fn burn_other_resource_for_labor_production( |
There was a problem hiding this comment.
so we'll need to multicall if we want to burn multiple resources in one go ?
| this.buildingCosts = Object.fromEntries( | ||
| Object.entries(this.config.buildings.buildingCosts).map(([key, costs]) => [ | ||
| Object.entries(this.config.buildings.nonResourceBuildingCosts).map(([key, costs]) => [ |
There was a problem hiding this comment.
can you rename into otherBuildingCosts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
contracts/game/src/models/resource/production/production.cairo (1)
229-233:⚠️ Potential issueAdd safeguards against resource drain attacks.
The resource burning loop could potentially drain more resources than intended if
production_tick_countis manipulated.Add maximum resource burn limits:
// make payment for produced resource let mut other_resource = ResourceImpl::get(ref world, (from_entity_id, other_resource_type)); +// Add maximum burn limit +let max_burn_amount = other_resource_amount * MAX_PRODUCTION_TICKS; +assert!(other_resource_amount * production_tick_count <= max_burn_amount, "exceeds maximum burn limit"); other_resource.burn(other_resource_amount * production_tick_count); other_resource.save(ref world);
🧹 Nitpick comments (4)
packages/core/src/types/common.ts (1)
415-423: Well-structured interface for labor-based production parameters!The interface effectively captures the requirements for the labor burn strategy, with clear parameter naming and appropriate types.
Consider adding JSDoc comments to document:
- The valid ranges for each parameter
- The relationship between
depreciation_percent_numanddepreciation_percent_denom- The units for food burn parameters
config/deployer/config.ts (2)
215-234: Well-structured console output with good error handling!The console output is informative and handles all cases appropriately, including when labor production is not available.
Consider adding units to the output for clarity:
- │ ${chalk.gray(`Wheat Burn Per Labor:`)} ${chalk.white(inGameAmount(calldata.labor_burn_strategy.wheat_burn_per_labor, config.config))} - │ ${chalk.gray(`Fish Burn Per Labor:`)} ${chalk.white(inGameAmount(calldata.labor_burn_strategy.fish_burn_per_labor, config.config))} + │ ${chalk.gray(`Wheat Burn Per Labor Unit:`)} ${chalk.white(inGameAmount(calldata.labor_burn_strategy.wheat_burn_per_labor, config.config))} wheat + │ ${chalk.gray(`Fish Burn Per Labor Unit:`)} ${chalk.white(inGameAmount(calldata.labor_burn_strategy.fish_burn_per_labor, config.config))} fish🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
Line range hint
1-1127: Fix Prettier formatting issues.The pipeline indicates formatting issues. Run Prettier with the --write flag to automatically fix the formatting:
prettier --write "config/deployer/config.ts"🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
contracts/game/src/systems/production/contracts.cairo (1)
129-154: Enhance input validation for resource burning.The function performs basic entity ownership and structure category validation but could benefit from additional checks.
Consider adding these validations:
fn burn_other_resources_for_labor_production( ref self: ContractState, entity_id: ID, resource_types: Span<u8>, resource_amounts: Span<u128>, ) { let mut world: WorldStorage = self.world(DEFAULT_NS()); SeasonImpl::assert_season_is_not_over(world); let entity_owner: EntityOwner = world.read_model(entity_id); entity_owner.assert_caller_owner(world); let entity_structure: Structure = world.read_model(entity_id); assert!(entity_structure.category == StructureCategory::Realm, "structure is not a realm"); + // Validate non-zero inputs + assert!(resource_types.len() > 0, "empty resource types"); + assert!(resource_amounts.len() > 0, "empty resource amounts"); + assert!( resource_types.len() == resource_amounts.len(), "resource types and resource amounts must be the same length" );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
client/apps/game/src/three/scenes/constants.ts(2 hunks)config/deployer/config.ts(3 hunks)config/environments/_shared_.ts(4 hunks)config/environments/utils/building.ts(4 hunks)contracts/common/addresses/local.json(1 hunks)contracts/game/manifest_local.json(28 hunks)contracts/game/src/models/resource/production/building.cairo(4 hunks)contracts/game/src/models/resource/production/production.cairo(4 hunks)contracts/game/src/systems/production/contracts.cairo(4 hunks)contracts/game/src/systems/realm/contracts.cairo(2 hunks)packages/core/src/constants/structures.ts(0 hunks)packages/core/src/managers/config-manager.ts(3 hunks)packages/core/src/types/common.ts(3 hunks)
💤 Files with no reviewable changes (1)
- packages/core/src/constants/structures.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- contracts/game/src/systems/realm/contracts.cairo
- packages/core/src/managers/config-manager.ts
- client/apps/game/src/three/scenes/constants.ts
- config/environments/shared.ts
- contracts/game/src/models/resource/production/building.cairo
- config/environments/utils/building.ts
🧰 Additional context used
🪛 GitHub Actions: lint
config/deployer/config.ts
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
contracts/common/addresses/local.json
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
🔇 Additional comments (14)
packages/core/src/types/common.ts (2)
440-441: LGTM! Type-safe configuration for resource production parameters.The additions to the Config interface properly integrate the labor production parameters and resource rarity mapping.
552-553: Good naming improvement for better clarity!The renaming of
buildingCoststootherBuildingCostshelps distinguish between different types of building costs while maintaining the existingresourceBuildingCosts.config/deployer/config.ts (3)
16-16: LGTM! Appropriate import for labor parameters scaling.The addition of
scaleResourceProductionByLaborParamsis consistent with the new labor production strategy.🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
194-198: LGTM! Consistent scaling implementation.The scaling of labor production parameters follows the same pattern as other resource scaling operations, maintaining consistency in the codebase.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
200-207: Good variable renaming for improved clarity!The variable names are now more descriptive and better reflect their purpose:
outputAmountPerBuildingPerTickclearly indicates per-building, per-tick productionpredefinedResourceBurnCostandresourceProductionByLaborParamsaccurately describe their roles🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
contracts/common/addresses/local.json (1)
2-4: Verify contract address changes.The core contract addresses (seasonPass, realms, lords) have been updated. Ensure these new addresses are correctly deployed and configured in the target environment.
✅ Verification successful
Local environment addresses verified
The contract addresses are properly configured across local environment files. No further verification needed as these are development environment addresses.
- Found in both
contracts/common/addresses/local.jsonandconfig/environments/data/local.json- Deployment scripts present in the repository for local setup
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify contract addresses exist and are deployed # Note: Using curl to check contract existence. Actual implementation may vary based on your blockchain explorer API for addr in "0x1b39ac97dc0e388d3bae0131b450913a87674cad99f735d79be5c838a8cd7e5" "0x684cff4e32074d596bd5f218da91277607dab5e93653c20797cd41268002c99" "0x3582347b80946fce439e5ae5d6b8fc10de6c2ee4d74181af35cfc8efa95cf1b"; do echo "Checking address: $addr" curl -s "https://your-blockchain-explorer/api/contract/$addr" doneLength of output: 1550
Script:
#!/bin/bash # Search for contract addresses across the codebase echo "Searching for seasonPass contract references..." rg -l "0x1b39ac97dc0e388d3bae0131b450913a87674cad99f735d79be5c838a8cd7e5" echo -e "\nSearching for realms contract references..." rg -l "0x684cff4e32074d596bd5f218da91277607dab5e93653c20797cd41268002c99" echo -e "\nSearching for lords contract references..." rg -l "0x3582347b80946fce439e5ae5d6b8fc10de6c2ee4d74181af35cfc8efa95cf1b" # Look for deployment scripts echo -e "\nChecking for deployment related files..." fd -t f "deploy|migration" -e ts -e js -e shLength of output: 1139
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
contracts/game/src/systems/production/contracts.cairo (2)
22-24: LGTM! Input validation is properly implemented.The method signature correctly accepts resource types and amounts as spans, allowing for batch processing of resources.
185-213: Review production tick count validation.The function
burn_other_predefined_resources_for_resourcesprocesses resources over multiple ticks. This could potentially be exploited if unrealistic tick counts are provided.This is a duplicate of a previous review comment. The concern about tick count validation remains valid. Consider adding maximum tick count validation to prevent resource exploitation.
contracts/game/src/models/resource/production/production.cairo (2)
95-103: LGTM! Resource production calculation is properly implemented.The production calculation correctly:
- Calculates total production based on ticks
- Limits production by output_amount_left
- Handles free production resources separately
171-193:⚠️ Potential issuePrevent potential overflow in resource calculations.
The resource conversion calculations involve multiple multiplications and divisions that could potentially overflow.
Consider adding overflow checks:
let wheat_burn_amount: u128 = labor_amount / RESOURCE_PRECISION * produced_resource_labor_burn_strategy.wheat_burn_per_labor; +// Add overflow check +assert!(wheat_burn_amount >= labor_amount / RESOURCE_PRECISION, "wheat burn amount overflow"); let fish_burn_amount: u128 = labor_amount / RESOURCE_PRECISION * produced_resource_labor_burn_strategy.fish_burn_per_labor; +// Add overflow check +assert!(fish_burn_amount >= labor_amount / RESOURCE_PRECISION, "fish burn amount overflow");Likely invalid or redundant comment.
contracts/game/manifest_local.json (4)
3297-3297: Addition of Labor building type aligns with PR objectives.The addition of the
Laborvariant to theBuildingCategoryenum enables the new labor-based production strategy.
2983-3008: LaborBurnPrStrategy structure implements required mechanics.The structure includes all components mentioned in PR objectives:
- Resource rarity for value/conversion rate
- Depreciation mechanics via num/denom
- Food costs per labor (wheat/fish)
Line range hint
6615-6632: Production systems contract updated with new burn methods.Three new methods added to support the labor burn strategy:
burn_other_resource_for_labor_production: Converts resources to laborburn_labor_resource_for_other_production: Converts labor back to resourcesburn_other_predefined_resources_for_resource: Handles predefined resource conversionsThe implementation follows a clear pattern of resource transformation while maintaining economic balance.
Also applies to: 6633-6654, 6655-6671
Line range hint
1-9583: Verify manifest structure and contract dependencies.The manifest contains a complex web of contracts, models, and events. Let's verify the dependencies are correctly configured.
✅ Verification successful
Manifest structure and dependencies verified
The manifest is well-structured with:
- 24 unique contracts with proper implementations
- 84 unique models with proper definitions
- 26 unique events
- No duplicate tags or conflicting definitions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify contract dependencies and references # Check if all referenced contracts exist echo "Checking contract references..." for contract in $(jq -r '.contracts[].tag' contracts/game/manifest_local.json); do echo "Verifying contract: $contract" # Search for contract implementations rg -l "$contract" . --type cairo done # Check if all models are referenced echo "Checking model references..." for model in $(jq -r '.models[].tag' contracts/game/manifest_local.json); do echo "Verifying model: $model" # Search for model usage rg -l "$model" . --type cairo doneLength of output: 448
Script:
#!/bin/bash # Description: Verify contract dependencies and references # Check if all referenced contracts exist echo "Checking contract references..." for contract in $(jq -r '.contracts[].tag' contracts/game/manifest_local.json); do echo "Verifying contract: $contract" # Search for contract implementations rg -l "$contract" . --type cairo || true done # Check if all models are referenced echo "Checking model references..." for model in $(jq -r '.models[].tag' contracts/game/manifest_local.json); do echo "Verifying model: $model" # Search for model usage rg -l "$model" . --type cairo || true done # Print manifest structure echo "Manifest structure:" jq -r '.contracts | length' contracts/game/manifest_local.json jq -r '.models | length' contracts/game/manifest_local.json jq -r '.events | length' contracts/game/manifest_local.jsonLength of output: 28355
Script:
#!/bin/bash # Description: Verify contract and model implementations echo "Checking contract implementations..." for contract in $(jq -r '.contracts[].tag' contracts/game/manifest_local.json); do echo "Verifying contract: $contract" # Search for contract implementations without file type restriction rg -l "impl.*$contract" || true done echo "Checking model definitions..." for model in $(jq -r '.models[].tag' contracts/game/manifest_local.json); do echo "Verifying model: $model" # Search for model definitions without file type restriction rg -l "struct.*$model" || true done # Print manifest structure and validate relationships echo "Analyzing manifest structure..." echo "Contracts count: $(jq -r '.contracts | length' contracts/game/manifest_local.json)" echo "Models count: $(jq -r '.models | length' contracts/game/manifest_local.json)" echo "Events count: $(jq -r '.events | length' contracts/game/manifest_local.json)" # Check for any duplicate tags echo "Checking for duplicate tags..." jq -r '.contracts[].tag' contracts/game/manifest_local.json | sort | uniq -d jq -r '.models[].tag' contracts/game/manifest_local.json | sort | uniq -d jq -r '.events[].tag' contracts/game/manifest_local.json | sort | uniq -dLength of output: 24994
| "STONE": [1,"0x764bfa8054e01987c63c8c4de249d87cb73bb894d30eec980ef7e14354b499"], | ||
| "COAL": [2,"0x291d337d97f440979b82e953529afa3e49cc9a7429b080fac21f7e46d0937c0"], | ||
| "WOOD": [3,"0x3d76be487d3ccd832eece5c9a1dca8a4e730a8e95980d39b5b4225609ff10f2"], | ||
| "COPPER": [4,"0x201b62f8fb45abb45a71f30e4d67213ec649eb548c93f58fa6a5cf7581b2af8"], | ||
| "IRONWOOD": [5,"0xa4b6768aa7d934f58916c0aebf3c20a2cc72c6a1dcf36992f452399944aa43"], | ||
| "OBSIDIAN": [6,"0x64113e7a151e22c41aed43b3c2b9917c53999233e9199735aa11e485b18d4bb"], | ||
| "GOLD": [7,"0x663d6c0f1d9c5d1db9a21bcffcfbff67f612fff1822f1a2a10ea2ce7381e89c"], | ||
| "SILVER": [8,"0x7256d097abcd84da4c5cdddc0effec6eaff8af1a27355e2b58dcd75b4195aaa"], | ||
| "MITHRAL": [9,"0x4517035fe9b1117f9e2ff7afcb7fb22473471f920ced8cbe8181fe18a143365"], | ||
| "ALCHEMICALSILVER": [10,"0x35002c524ee8610b126fbeea42d0008a761becf61543b7b5b3bd0de88eecf6c"], | ||
| "COLDIRON": [11,"0x7bfac4aeda636370487f1f00e81ce60e989b78c7c75aeb887a1572afd44549e"], | ||
| "DEEPCRYSTAL": [12,"0x1ef219c0c696040fad7a1454de9b0e60e8d8139e0fc9abd03aef6833c6006aa"], | ||
| "RUBY": [13,"0x1f2ec80025c3316f72273a6fff795b77486b569ae9b2705332413d65cd2e9d9"], | ||
| "DIAMONDS": [14,"0x68c3cd61accd2f6fc53b604c26b13324a6422934ea188c528c9fe368f92a0a6"], | ||
| "HARTWOOD": [15,"0x7725e1d3d76766ab5d932773db10f0253fb6150dec166c02d3182ff1c18307c"], | ||
| "IGNIUM": [16,"0x1c682133d704c3e60da312d0b7fcf996ec0734c6cf837fc9721b334dbade9e4"], | ||
| "TWILIGHTQUARTZ": [17,"0x251336dc8d38d28817642abdc84284211b3c3018f0de71f26c0d3f7dfd8e5ed"], | ||
| "TRUEICE": [18,"0x44acfcb03aaff31a83ce8f9950ba43c81869d7381666c754718046262cbe14f"], | ||
| "ADAMANTINE": [19,"0x237e8ba7dfcde8ff1a52049d068dd72eea91e82fbd4e6a33b8a57d3af9d596e"], | ||
| "SAPPHIRE": [20,"0x474ffff5b46caa1ff53065ffdd8659cc3a24c10a83d959ebb3fa461a04137e0"], | ||
| "ETHEREALSILICA": [21,"0x3bab20c62145e6c2811e79c4740579fc13a222168ca1ff711b7c6ff9106709f"], | ||
| "DRAGONHIDE": [22,"0x23ca3985af122c50de2bd4dfeeb2a9e15275fb55da095aac955aff87b7428d8"], | ||
| "ANCIENTFRAGMENT": [24,"0x4b62c1c63d6dd8610f648274e3c6221b15497f4bbbcb90f943b6ce8bbbb4481"], | ||
| "DONKEY": [25,"0x4a9797b9ca89087191e6aba1e15818f60b5d273752e2b86fd6cd6e73071d155"], | ||
| "KNIGHT": [26,"0x38e63e49a87d431dee485ad1575fdf026dc7b886f41e23a268a74feeaf0d60a"], | ||
| "CROSSBOWMAN": [27,"0x4a1f92f9897c84cd272d64253657cbc81fd5bc2b891781bcc5f82d1d5eaa1e1"], | ||
| "PALADIN": [28,"0x54ba2ca0958ba7b800e74dcc1ff5911fc29657410ad452d1aa789afd9b7da65"], | ||
| "WHEAT": [29,"0x491e517ae27e3d2c4f12faa8d8c0a962e7b8d3bfd41b726ba99b2f9c57b8955"], | ||
| "FISH": [30,"0x502ec92d38c32f6beb0c5c78d06f9dcbd386589cd974a2d0c3f5204f99bb949"], | ||
| "LORDS": [31,"0x3582347b80946fce439e5ae5d6b8fc10de6c2ee4d74181af35cfc8efa95cf1b"] |
There was a problem hiding this comment.
Fix JSON formatting to meet Prettier standards.
The file needs formatting according to the pipeline failure logs.
Run Prettier with the --write flag to automatically fix the formatting:
{
"seasonPass": "0x1b39ac97dc0e388d3bae0131b450913a87674cad99f735d79be5c838a8cd7e5",
"realms": "0x684cff4e32074d596bd5f218da91277607dab5e93653c20797cd41268002c99",
"lords": "0x3582347b80946fce439e5ae5d6b8fc10de6c2ee4d74181af35cfc8efa95cf1b",
"resources": {
- "STONE": [1,"0x764bfa8054e01987c63c8c4de249d87cb73bb894d30eec980ef7e14354b499"],
+ "STONE": [1, "0x764bfa8054e01987c63c8c4de249d87cb73bb894d30eec980ef7e14354b499"],
// Apply similar spacing fixes to other resource entries
}
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
config/environments/data/mainnet.json(1 hunks)config/environments/data/sepolia.json(1 hunks)config/environments/data/slot.json(1 hunks)config/environments/utils/building.ts(3 hunks)config/utils/utils.ts(1 hunks)contracts/common/addresses/local.json(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- config/environments/data/slot.json
- config/environments/data/sepolia.json
- config/environments/data/mainnet.json
🚧 Files skipped from review as they are similar to previous changes (1)
- config/environments/utils/building.ts
🧰 Additional context used
🪛 GitHub Actions: lint
contracts/common/addresses/local.json
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
🔇 Additional comments (5)
config/utils/utils.ts (1)
Line range hint
1-42: Verify configuration compatibility with new resource production strategies.Given that this PR introduces new resource production strategies (
LaborBurnPrStrategyandMultipleResourceBurnPrStrategy), ensure that:
- The
Configtype properly reflects all new configuration properties required by these strategies- All environment configurations (
sepolia,mainnet,slot) include the necessary parameters for resource productionRun this script to verify the configuration structure:
✅ Verification successful
Type assertions are acceptable for configuration compatibility.
The type assertions (
as any) in the configuration loading are justified because:
- The configuration files follow a consistent structure across all environments
- Resource production is configured through the existing
resourceProductionByLaborParamsstructure- The assertions help maintain backward compatibility while supporting new strategy implementations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify configuration compatibility with new strategies # Search for strategy-related configuration properties echo "Searching for strategy configurations..." rg -l "LaborBurnPrStrategy|MultipleResourceBurnPrStrategy" # Check configuration files for required properties echo "Checking configuration files for required properties..." for file in sepolia mainnet slot; do echo "=== $file configuration ===" jq '.configuration | select(.resourceProduction != null) | .resourceProduction' "../environments/data/$file.json" doneLength of output: 1465
Script:
#!/bin/bash # First, find the correct paths for config files echo "=== Finding config files ===" fd -e json -e ts "local|mainnet|sepolia|slot" config/ # Check content of local.json as reference echo -e "\n=== Examining local config structure ===" cat config/environments/data/local.json # Look for strategy implementation details echo -e "\n=== Examining strategy implementations ===" rg -A 5 "class.*(?:LaborBurnPrStrategy|MultipleResourceBurnPrStrategy)"Length of output: 66327
contracts/common/addresses/local.json (4)
Line range hint
1-37: Fix JSON formatting to meet Prettier standards.The file needs formatting according to the pipeline failure logs.
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
Line range hint
5-35: Verify resource configuration aligns with new strategies.The resource configuration appears structured correctly with [index, address] pairs. However, since these resources will be used in the new
LaborBurnPrStrategyandMultipleResourceBurnPrStrategy, please ensure:
- All resource indexes are correctly mapped for the burn strategies
- The resource hierarchy matches the conversion rates in
LaborBurnPrStrategy✅ Verification successful
Resource configuration correctly aligns with burn strategies
The resource configuration in local.json properly supports both:
- Labor burn strategy with resource rarity and food consumption
- Multiple resource burn strategy with predefined resource combinations
The sequential resource IDs (1-31) are correctly referenced throughout the implementation, maintaining consistency with both strategy patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify resource configuration consistency # Extract and validate resource indexes and addresses # Check for resource usage in strategy implementations echo "Checking resource usage in strategies:" rg -l "LaborBurnPrStrategy|MultipleResourceBurnPrStrategy" # Verify resource indexes are referenced correctly echo "Checking resource index references:" rg -l "required_resources_id|required_resources_count"Length of output: 1122
Script:
#!/bin/bash # Check strategy implementations and resource usage echo "=== Checking strategy implementations ===" rg -A 5 "LaborBurnPrStrategy|MultipleResourceBurnPrStrategy" contracts/game/src/systems/config/contracts.cairo echo -e "\n=== Checking resource configurations ===" rg -A 5 "resource.*id|resource.*count" contracts/game/src/models/config.cairo echo -e "\n=== Checking production model resource usage ===" rg -A 5 "resource" contracts/game/src/models/resource/production/production.cairoLength of output: 12417
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
Line range hint
3-3: Verify LORDS token address duplication.The LORDS token address appears both as a top-level contract address and as a resource. While this might be intentional, please verify:
- If this duplication is necessary
- If both references are required by the new strategies
Also applies to: 35-35
✅ Verification successful
Both LORDS token references are necessary and serve distinct purposes
The duplication of the LORDS token address is intentional and required:
- Top-level
lordsaddress is used for direct contract interactions (minting, attaching)- Resource entry
LORDSis used when handling it as a game resource (trading, bank operations)🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check LORDS token usage patterns # Search for LORDS token address usage LORDS_ADDR="0x3acbe76476cbf8997397878c52ce1acdf90ed1c295907f6505d3ff4c1646afa" echo "Checking LORDS token usage patterns:" rg -B 2 -A 2 "$LORDS_ADDR"Length of output: 3272
Script:
#!/bin/bash # Search for patterns of accessing LORDS token references echo "Searching for lords token access patterns:" rg -B 2 -A 2 "lords['\"]|LORDS['\"]" --type ts --type js echo -e "\nSearching for resource access patterns:" ast-grep --pattern 'resources.$_'Length of output: 16938
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
Line range hint
2-4: Verify contract address validity and deployment.The contract addresses appear to be in the correct format. However, since these are critical system components that will interact with the new resource production strategies, please ensure:
- All addresses are correctly deployed contracts
- The contracts at these addresses implement the required interfaces for the new strategies
✅ Verification successful
Contract addresses are properly integrated and valid
The verification confirms these addresses are:
- Properly integrated into the core type system through the
SeasonAddressesinterface- Used consistently across development and production environments
- Referenced correctly in deployment and configuration files
- Part of the season pass and resource production systems
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify contract addresses exist and are deployed # Note: This is a local environment check for addr in "0x432531b2276e32aa5b82ae095e6694d68f49a2c82f2ee8b406f681ec3826c8b" "0x56502f7cf5847c0a4d4d0d0a37fa02e104d637f278564ce2e85fcd9edd7d27a" "0x3acbe76476cbf8997397878c52ce1acdf90ed1c295907f6505d3ff4c1646afa"; do # Search for contract deployment or usage in the codebase echo "Checking address: $addr" rg -l "$addr" doneLength of output: 1736
Script:
#!/bin/bash # Search for contract interface definitions and usage patterns echo "Searching for seasonPass contract usage..." rg -A 5 "seasonPass" --type cairo echo -e "\nSearching for realms contract usage..." rg -A 5 "realms" --type cairo echo -e "\nSearching for lords contract usage..." rg -A 5 "lords" --type cairoLength of output: 461
Script:
#!/bin/bash # Search for contract usage and initialization patterns echo "Searching for seasonPass patterns..." rg -l "seasonPass" echo -e "\nSearching for contract initialization patterns..." rg -A 5 "contract_address|interface|impl" --type ts --type js echo -e "\nChecking contract deployment files..." fd "deploy|init" --type f --exec rg -l "0x432531b2|0x56502f7|0x3acbe76"Length of output: 67066
🧰 Tools
🪛 GitHub Actions: lint
[warning] Code formatting does not meet Prettier standards. Run Prettier with --write to fix.
| @@ -27,11 +27,11 @@ export function getConfigFromNetwork(chain: Chain): Config { | |||
| try { | |||
| switch (chain) { | |||
| case "sepolia": | |||
| return sepoliaConfig.configuration; | |||
| return sepoliaConfig.configuration as any; // as any to avoid type errors | |||
There was a problem hiding this comment.
Avoid using as any type assertions.
The addition of as any type assertions suggests there might be type mismatches between the configuration files and the Config type. While this fixes the immediate type errors, it bypasses TypeScript's type checking and could hide potential issues.
Consider these alternatives:
- Investigate and fix the type mismatches between the configuration files and the
Configtype - If the configurations have additional properties, extend the
Configtype to include them - Use a more specific type assertion if needed (e.g.,
as Config)
Example approach to investigate type mismatches:
#!/bin/bash
# Description: Analyze the structure of configuration files vs Config type
# Find the Config type definition
echo "Searching for Config type definition..."
rg -A 10 "type Config = {"
# Compare with actual configuration structure
echo "Analyzing configuration structure..."
for file in sepolia mainnet slot; do
echo "=== $file configuration ==="
jq 'type' "../environments/data/$file.json"
doneAlso applies to: 32-32, 34-34
Production Strategies Implementation
This PR introduces two distinct strategies for resource production :
LaborBurnPrStrategyandMultipleResourceBurnPrStrategy.Labor Burn Production Strategy
The
LaborBurnPrStrategyimplements a labor-based intermediary system for resource conversion, allowing players to convert from a resource -> labor -> resource while trying to maintain economic balance through resource rarity and depreciation mechanics.Key Components:
resource_rarity: Defines the resource's relative value and labor conversion ratedepreciation_percent_num/denom: Controls value loss during conversionwheat/fish_burn_per_labor: Food costs per labor unitExample Conversion Flow:
Given:
Wood: Rarity = 100, Depreciation = 20% (2/10)
Gold: Rarity = 1000, Depreciation = 10% (1/10)
Converting 3 Wood → Gold:
Converting 7 Gold → Wood:
The depreciation is always applied based on the target resource's rate, creating an intentional loss in the conversion process to maintain economic balance.
The labor burn strategy includes two main conversion functions:
burn_other_resource_for_labor_production: Converts resources to laborburn_labor_resource_for_other_production: Converts labor to resourcesMultiple Resource Burn Production Strategy
The
MultipleResourceBurnPrStrategykeeps the simpler and more efficient crafting system where multiple input resources are required to produce an output resource.Key Components:
required_resources_id: References the list of required input resourcesrequired_resources_count: Number of different resources neededExample:
Crafting Stone Requirements:
Configuration:
required_resources_id: Points to ResourceCost entry containing the above requirements
required_resources_count: 3 (number of different resources needed)
Technical Implementation
The
burn_other_predefined_resources_for_resourcefunction handles the multiple resource conversion:Summary by CodeRabbit
Release Notes
New Features
Changes
Improvements