update labor #2667
update labor #2667
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
|
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/core/src/provider/index.tsOops! Something went wrong! :( ESLint: 9.18.0 ESLint couldn't find an eslint.config.(js|mjs|cjs) file. From ESLint v9.0.0, the default configuration file is now eslint.config.js. https://eslint.org/docs/latest/use/configure/migration-guide If you still have problems after following the migration guide, please stop by WalkthroughThis pull request introduces modifications to several files in the game contracts and core provider. The changes primarily focus on updating contract class hashes in the Sepolia manifest, removing labor cost-related configurations, and simplifying production configuration methods. The modifications streamline the production and resource management logic by eliminating redundant labor calculations and adjusting method signatures across multiple components of the system. Changes
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: 0
🧹 Nitpick comments (1)
packages/core/src/provider/index.ts (1)
1921-1921: LGTM! Consider adding type safety for calldata arrays.The simplified production config calldata looks good. The separation of production and labor configs improves code organization.
Consider creating TypeScript types for the calldata arrays to improve type safety:
type ProductionCalldata = [resource_type: number, amount: number]; type LaborCalldata = [resource_type: number, cost_length: number, ...cost: Array<[resource: number, amount: number]>];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
contracts/game/manifest_sepolia.json(3 hunks)contracts/game/src/models/config.cairo(1 hunks)contracts/game/src/models/resource/production/production.cairo(2 hunks)contracts/game/src/systems/config/contracts.cairo(2 hunks)contracts/game/src/systems/map/map_generation.cairo(1 hunks)contracts/game/src/utils/testing/config.cairo(1 hunks)packages/core/src/provider/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- contracts/game/src/models/config.cairo
🔇 Additional comments (10)
contracts/game/src/models/resource/production/production.cairo (3)
10-10: LGTM! Added import for RESOURCE_PRECISION constant.The import aligns with the changes to use a fixed precision value instead of configurable labor costs.
Line range hint
166-166: Simplified labor calculation logic.The labor calculation has been simplified to directly use the actual production amount. This change aligns with the removal of configurable labor costs.
55-57: Verify the impact of using fixed RESOURCE_PRECISION.The change from using
labor_costtoRESOURCE_PRECISIONfor divisibility checks could affect existing labor calculations. Please ensure this doesn't break existing functionality.contracts/game/src/utils/testing/config.cairo (1)
144-144: LGTM! Simplified production config setup.Removed unnecessary empty array argument, aligning with the updated production config interface.
contracts/game/src/systems/map/map_generation.cairo (1)
166-166: Simplified shard mining labor calculation.The labor calculation has been simplified to directly use the actual production amount, removing the dependency on production configuration for labor calculations.
✅ Verification successful
Verified: Shard mining labor calculation simplification is properly isolated
The simplified labor calculation that directly uses the actual production amount is correctly implemented and doesn't affect other parts of the codebase. The change is contained within the shard mining system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for any other shard mining related calculations that might need updates rg -A 5 "get_shards_reward"Length of output: 1438
Script:
#!/bin/bash # Search for shard mining and labor-related code rg -l "EARTHEN_SHARD|labor_amount" contracts/ # Check for any configuration files related to production or mining fd -e cairo -e toml "config|production|mining" contracts/Length of output: 5737
Script:
#!/bin/bash # Check implementation in production and labor models ast-grep --pattern 'fn $_($$$ labor_amount $$$)' contracts/game/src/models/resource/production/labor.cairo ast-grep --pattern 'fn $_($$$ labor_amount $$$)' contracts/game/src/models/resource/production/production.cairo # Check for any other labor calculations that might depend on production config rg -A 3 "labor_amount.*production" contracts/game/src/Length of output: 608
contracts/game/src/systems/config/contracts.cairo (2)
134-134: Updated production config interface.Removed the
labor_amountparameter from the trait definition, simplifying the production configuration.
604-607: LGTM! Implementation matches updated interface.The implementation has been updated to match the simplified interface, removing labor-related configuration.
contracts/game/manifest_sepolia.json (3)
5404-5404: Class hash update for map_generation_systems contractThe class hash for the map generation systems contract has been updated from
0x48a8ae3f6841aed95fb3186cf390db19854c8174fea69fdca19c88fd8c58c63to0x1829437a65fddf03e8f1322558035b38be8c06a5dd81f6a863a68569364b4cc.Let's verify this contract upgrade:
✅ Verification successful
Class hash update for map_generation_systems contract is valid
The updated class hash corresponds to the map generation systems contract implementation found in the codebase. This contract handles critical game mechanics including shard mine discovery and mercenary deployment, and the hash update represents a legitimate contract upgrade.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the map generation systems contract upgrade # 1. Check if the old class hash exists in git history # 2. Check for any changes in map generation logic # Check git history for the old class hash git log -p | rg "0x48a8ae3f6841aed95fb3186cf390db19854c8174fea69fdca19c88fd8c58c63" # Check for changes in map generation logic ast-grep --pattern 'interface IMapGenerationSystems { $$$ discover_shards_mine($$$) { $$$ } $$$ }'Length of output: 112
Script:
#!/bin/bash # Search for old hash in current files rg "0x48a8ae3f6841aed95fb3186cf390db19854c8174fea69fdca19c88fd8c58c63" # Find manifest files fd manifest.json # Find map generation related files fd -t f -e cairo map_generationLength of output: 175
Script:
#!/bin/bash # View map generation contract content cat contracts/game/src/systems/map/map_generation.cairo # Check recent changes to the file git log --oneline contracts/game/src/systems/map/map_generation.cairo | head -n 5Length of output: 11224
6344-6344: Class hash update for production_systems contractThe class hash for the production systems contract has been updated from
0x72be272c9af1769ad38beaf68495533628fdb4ead0a7879760d17b1f6bab196to0x580ae58de580e41af4107e2b651ba52fa4ebeb5266568c6658cfa95fdecf48b.Let's verify this contract upgrade:
2385-2385: Class hash update for config_systems contractThe class hash for the config systems contract has been updated from
0x7fb2e9574aa693d81a15f3734dd0a8612ba8d64057b2aef692bd3117118a446to0x37798122f6085a277eedb203c93fe5ee2fbc99afe7427e4508b79000301dc2c.Let's verify this contract upgrade:
✅ Verification successful
Class hash update verified - No impact on labor costs
The config_systems contract upgrade maintains all existing functionality including labor cost handling. The class hash update appears to be a routine implementation change rather than a feature removal.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the config systems contract upgrade # 1. Check if the old class hash exists in git history # 2. Check the contract's interface changes # Check git history for the old class hash git log -p | rg "0x7fb2e9574aa693d81a15f3734dd0a8612ba8d64057b2aef692bd3117118a446" # Check if the contract interface has changed ast-grep --pattern 'interface IProductionConfig { $$$ set_production_config($$$) { $$$ } $$$ }'Length of output: 109
Script:
#!/bin/bash # Search for the old class hash in current files rg "0x7fb2e9574aa693d81a15f3734dd0a8612ba8d64057b2aef692bd3117118a446" # Find relevant contract files fd -e cairo -e json "config|production" # Search for labor cost related patterns rg -i "labor.*cost|cost.*config" -A 2 -B 2Length of output: 67676
Script:
#!/bin/bash # Search for production-related configurations and systems rg -A 2 "production.*config|config.*production" -g "*.cairo" # Look for any recent changes in production systems fd -e cairo "production" --exec head -n 1Length of output: 4898
Summary by CodeRabbit
Chores
Refactor