adding new labor system #2682
adding new labor system #2682
Conversation
… feat/new-production
…AO/eternum into feat/add-labor
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hi @aymericdelab! 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. |
![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)
client/apps/game/src/hooks/context/policies.ts (1)
704-713: Well-structured labor and resource production system.The new methods provide a clear and granular approach to resource management, with distinct operations for:
- Converting labor to other resources
- Converting predefined resources to other resources
- Converting resources to labor
This separation of concerns allows for better control and flexibility in resource management.
Consider documenting the resource conversion rates and constraints in the method descriptions to make the system more maintainable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/apps/game/src/hooks/context/policies.ts(24 hunks)client/apps/game/src/ui/modules/navigation/capacity-info.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: knip
client/apps/game/src/ui/modules/navigation/capacity-info.tsx
[warning] 9-14: Unused export 'StorehouseInfo' detected.
🔇 Additional comments (5)
client/apps/game/src/ui/modules/navigation/capacity-info.tsx (3)
9-51: Remove unused export or document its intended usage.The
StorehouseInfocomponent is exported but not used directly. Either remove the export or document its intended usage.-export const StorehouseInfo = ({ storehouseCapacity }: { storehouseCapacity: number }) => { +const StorehouseInfo = ({ storehouseCapacity }: { storehouseCapacity: number }) => {🧰 Tools
🪛 GitHub Actions: knip
[warning] 9-14: Unused export 'StorehouseInfo' detected.
86-96: Improve tooltip accessibility and user experience.The tooltip implementation could be enhanced for better accessibility and user experience.
<div + role="button" + tabIndex={0} onMouseEnter={() => { setTooltip({ position: "bottom", content: <StorehouseInfo storehouseCapacity={realmInfo.storehouses.capacityKg} />, }); }} onMouseLeave={() => { setTooltip(null); }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + setTooltip({ + position: "bottom", + content: <StorehouseInfo storehouseCapacity={realmInfo.storehouses.capacityKg} />, + }); + } + }} + onBlur={() => { + setTooltip(null); + }} + aria-label={`Storage capacity: ${realmInfo.storehouses.capacityKg.toLocaleString()} kg`} className="storehouse-selector px-3 flex gap-2 justify-start items-center text-xxs md:text-sm" >
107-117: Improve tooltip accessibility and user experience.The tooltip implementation could be enhanced for better accessibility and user experience.
<div + role="button" + tabIndex={0} onMouseEnter={() => { setTooltip({ position: "bottom", content: <WorkersHutInfo />, }); }} onMouseLeave={() => { setTooltip(null); }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + setTooltip({ + position: "bottom", + content: <WorkersHutInfo />, + }); + } + }} + onBlur={() => { + setTooltip(null); + }} + aria-label={`Population capacity: ${realmInfo?.population || 0} / ${(realmInfo?.capacity || 0) + configManager.getBasePopulationCapacity()}`} className="population-selector px-3 flex gap-2 justify-start items-center text-xs md:text-sm" >client/apps/game/src/hooks/context/policies.ts (2)
1-3: LGTM!The import and overall structure are clean and well-organized.
13-681: Verify contract address updates across the system.Multiple contract addresses have been updated. While the structure remains consistent, ensure these changes are coordinated with other parts of the system.
Run the following script to check for any remaining references to the old contract addresses:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Search for old contract addresses across the codebase # List of old addresses to check OLD_ADDRESSES=( "0x22fca458aa0869b4cacca2098ae8fe3cc462bc5cef9dd222a5a68055cb8906b" "0x158114da9a538d75512dc29b3d1995ff551dfb61bee80996232abe54b8febc2" "0x66bc2d8ef45ce10d3847cc901a41af6a05db34c0ea0ab87fc332ca8c9b2ccb0" "0x73dff02735d7031496afb34d28f6fd935d8f8e2db14b0877aaf871b3d11f524" "0x5827e314ca954f7eb04127a2a321e5c1f8fa6d51546b8fa6d6b38830f2b6d18" "0x6c0ae788bf4a46b49a57a67944bfb614c8da00b2b99c5d2917ec9bafb8ed460" ) # Search for each old address for addr in "${OLD_ADDRESSES[@]}"; do echo "Searching for address: $addr" rg "$addr" doneLength of output: 2189
Contract Address Updates Verified
The search for outdated contract addresses returned no matches in the codebase, indicating that the updates have been applied consistently. Please ensure that any dependencies or integrations referencing these contracts are updated accordingly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
client/apps/game/src/ui/components/production/production-modal.tsx (2)
17-21: Consider adding error handling for undefined structureEntityId.The
initialRealmmemo doesn't handle the case wherestructureEntityIdis undefined. This could lead to unnecessary finds on theplayerRealmsarray.const initialRealm = useMemo(() => { const structureEntityId = useUIStore.getState().structureEntityId; + if (!structureEntityId) return undefined; const selectedRealm = playerRealms.find((r) => r.entityId === structureEntityId); return selectedRealm; }, [playerRealms]);
35-46: Consider extracting CSS classes to a design system.The component uses multiple hardcoded CSS classes. Consider extracting these to a design system or CSS module for better maintainability.
client/apps/game/src/ui/components/production/production-sidebar.tsx (2)
28-36: Consider caching building query results.The
useMemofor buildings query could benefit from caching at a higher level if this data is used elsewhere in the application.
46-59: Optimize resource rendering loop.The current implementation creates a new function for each resource and performs a component value lookup in the render loop. Consider moving the value lookup to a useMemo hook.
+const resourceValues = useMemo(() => { + return Object.values(realm.resources).reduce((acc, resource) => { + const value = getComponentValue(Resource, getEntityIdFromKeys([BigInt(realm.entityId), BigInt(resource)])); + if (value && value.balance > 0) { + acc[resource] = value.balance; + } + return acc; + }, {} as Record<number, number>); +}, [realm.resources, realm.entityId]); {Object.values(realm.resources).map((resource) => { - const value = getComponentValue(Resource, getEntityIdFromKeys([BigInt(realm.entityId), BigInt(resource)])); - if (value && value.balance > 0) { + if (resourceValues[resource]) { return ( <ResourceIcon key={resource} resource={resources.find((r) => r.id === resource)?.trait || ""} size="sm" className="opacity-80" /> ); } return null; })}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/apps/game/src/ui/components/production/production-modal.tsx(1 hunks)client/apps/game/src/ui/components/production/production-sidebar.tsx(1 hunks)
🔇 Additional comments (3)
client/apps/game/src/ui/components/production/production-modal.tsx (2)
8-12: LGTM! Good use of code splitting.The implementation of lazy loading for both
ProductionSidebarandProductionBodycomponents will help optimize initial load performance.
23-23: Consider adding loading state handling.The state initialization assumes
playerRealmsis immediately available. Consider handling the loading state to prevent potential undefined errors.-const [selectedRealm, setSelectedRealm] = useState<RealmInfo | undefined>(initialRealm || playerRealms[0]); +const [selectedRealm, setSelectedRealm] = useState<RealmInfo | undefined>(initialRealm || playerRealms?.[0]);client/apps/game/src/ui/components/production/production-sidebar.tsx (1)
71-84: LGTM! Good use of memo for performance optimization.The
ProductionSidebarcomponent is correctly memoized to prevent unnecessary re-renders.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
client/apps/game/src/ui/modules/navigation/capacity-info.tsx (3)
18-45: Enhance numeric formatting for better readability.Consider using a consistent number formatting approach with appropriate precision and separators for better readability.
- {(capacity / configManager.getResourceWeight(ResourcesIds.Lords)).toLocaleString()} Lords + {Math.floor(capacity / configManager.getResourceWeight(ResourcesIds.Lords)).toLocaleString()} LordsApply similar changes to other numeric displays to ensure consistent formatting across all resource types.
99-103: Document the infinite capacity condition.Add a comment explaining when and why infinite capacity is displayed to improve code maintainability.
+ // Display infinite capacity for non-Realm structures as they don't have storage limitations {structureCategory !== StructureType.Realm ? ( <div className="self-center">∞</div> ) : ( <div className="self-center">{realmInfo.storehouses.capacityKg.toLocaleString()} kg</div> )}
89-91: Optimize performance by memoizing tooltip content.Consider memoizing the tooltip content to prevent unnecessary re-renders.
+ const storehouseTooltip = useMemo(() => ( + <StorehouseInfo storehouseCapacity={realmInfo.storehouses.capacityKg} /> + ), [realmInfo.storehouses.capacityKg]); + + const workersHutTooltip = useMemo(() => ( + <WorkersHutInfo /> + ), []); // In the JSX: setTooltip({ position: "bottom", - content: <StorehouseInfo storehouseCapacity={realmInfo.storehouses.capacityKg} />, + content: storehouseTooltip, }); // And: setTooltip({ position: "bottom", - content: <WorkersHutInfo />, + content: workersHutTooltip, });Also applies to: 110-112
packages/core/src/dojo/contract-components.ts (1)
874-885: Comprehensive refactor of ProductionConfig for the new labor system.The changes introduce a more sophisticated production model with:
- Per-building-per-tick production rates
- Labor burn strategy with depreciation and resource costs
- Multiple resource burn strategy for complex production chains
Consider documenting the following aspects of the new system:
- How depreciation affects labor efficiency
- The relationship between wheat/fish burn rates and production
- The conditions for multiple resource burn strategy activation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.prettierignore(1 hunks)client/apps/game/src/ui/components/resources/realm-transfer-manager.tsx(1 hunks)client/apps/game/src/ui/modules/navigation/capacity-info.tsx(1 hunks)config/environments/utils/building.ts(3 hunks)packages/core/src/dojo/contract-components.ts(2 hunks)packages/core/src/utils/resources.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- client/apps/game/src/ui/components/resources/realm-transfer-manager.tsx
- config/environments/utils/building.ts
🔇 Additional comments (9)
.prettierignore (2)
23-24: Generated TS Files Ignore Pattern Retained:
The ignore pattern "**/*.gen.ts" remains in the file as expected. This ensures that generated TypeScript files are not processed by Prettier.
25-26: New JSON Files Ignore Pattern Added:
The addition of the "**/*.json" ignore pattern will prevent all JSON files from being formatted by Prettier. Confirm that this behavior aligns with your project requirements, especially if some JSON files (e.g., configuration or schema files) should remain formatted.client/apps/game/src/ui/modules/navigation/capacity-info.tsx (4)
1-8: LGTM! All imports are properly used and necessary.The imports are well-organized and each dependency serves a clear purpose in the implementation.
9-9: Remove unused export or document its intended usage.The
StorehouseInfocomponent is exported but not used directly. Either remove the export or document its intended usage.-export const StorehouseInfo = ({ storehouseCapacity }: { storehouseCapacity: number }) => { +const StorehouseInfo = ({ storehouseCapacity }: { storehouseCapacity: number }) => {
53-65: LGTM! Clear and informative population capacity display.The component effectively communicates population capacity information with a well-structured UI.
86-96: Improve tooltip accessibility and user experience.The tooltip implementation could be enhanced for better accessibility and user experience.
<div + role="button" + tabIndex={0} onMouseEnter={() => { setTooltip({ position: "bottom", content: <StorehouseInfo storehouseCapacity={realmInfo.storehouses.capacityKg} />, }); }} onMouseLeave={() => { setTooltip(null); }} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + setTooltip({ + position: "bottom", + content: <StorehouseInfo storehouseCapacity={realmInfo.storehouses.capacityKg} />, + }); + } + }} + onBlur={() => { + setTooltip(null); + }} + aria-label={`Storage capacity: ${realmInfo.storehouses.capacityKg.toLocaleString()} kg`}Also applies to: 107-116
packages/core/src/utils/resources.ts (1)
171-182: LGTM! The new scaling function is well-implemented.The function correctly handles scaling of labor-related resource parameters while maintaining immutability by creating a new object.
packages/core/src/dojo/contract-components.ts (2)
891-892: Types array updated to match new component structure.The metadata types array has been correctly updated to reflect the new fields in the ProductionConfig component.
1054-1054: Resource tracking enhancement with output_amount_left.The addition of
output_amount_leftto the Resource component enables better tracking of remaining production capacity.Verify that the Resource component's production tracking is properly initialized and updated:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for Resource component initialization and updates ast-grep --pattern 'Resource { $$$ production: { $$$ output_amount_left: $_ $$$ } $$$ }'Length of output: 110
I'm not seeing any matches with the initial AST grep search. Let's run a couple of additional scripts to check for the usage and initialization of the production tracking (including output_amount_left) in the codebase.
Below are two scripts:
Search for all references to “output_amount_left” across the repository (with context):
Display the code around line 1054 of packages/core/src/dojo/contract-components.ts to verify the context of the addition:
Let's run these to gather further information about how output_amount_left is initialized and used.
🏁 Script executed:
#!/bin/bash rg -n "output_amount_left" --context=3Length of output: 7398
🏁 Script executed:
#!/bin/bash sed -n '1040,1060p' packages/core/src/dojo/contract-components.tsLength of output: 729
Production tracking for the Resource component is correctly initialized and updated.
- The Resource component's production object in packages/core/src/dojo/contract-components.ts properly declares output_amount_left with its expected type.
- References to output_amount_left for validations and production logic are confirmed in packages/core/src/managers/resource-manager.ts.
- The update operations (increase/decrease) for output_amount_left are implemented in contracts/game/src/models/resource/production/production.cairo.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
client/apps/torii-launcher/webpack.renderer.config.ts (1)
6-9: Remove duplicate CSS rules to prevent processing conflicts.There are two separate rules for processing CSS files:
- A global rule with
style-loaderandcss-loader- A src-scoped rule with
style-loader,css-loader, andpostcss-loaderThis could lead to unexpected CSS processing behavior.
Apply this diff to consolidate the rules:
-rules.push({ - test: /\.css$/, - use: [{ loader: "style-loader" }, { loader: "css-loader" }], -}); rules.push( { test: /\.tsx?$/, include: [path.resolve(__dirname, "src")], loader: "babel-loader", resolve: { extensions: [".js", ".jsx", ".json", ".ts", ".tsx"], }, }, { // loads .html files test: /\.(html)$/, include: [path.resolve(__dirname, "src")], use: { loader: "html-loader", }, }, { // loads .css files test: /\.css$/, include: [path.resolve(__dirname, "src")], use: ["style-loader", "css-loader", "postcss-loader"], }, );Also applies to: 29-33
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
.knip.json(1 hunks)client/apps/game/.env.sepolia(1 hunks)client/apps/landing/.env.sepolia(1 hunks)client/apps/landing/src/hooks/context/dojo-context.tsx(2 hunks)client/apps/torii-launcher/index.html(1 hunks)client/apps/torii-launcher/src/index.css(1 hunks)client/apps/torii-launcher/webpack.main.config.ts(1 hunks)client/apps/torii-launcher/webpack.plugins.ts(1 hunks)client/apps/torii-launcher/webpack.renderer.config.ts(1 hunks)client/apps/torii-launcher/webpack.rules.ts(1 hunks)pnpm-workspace.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- client/apps/torii-launcher/src/index.css
- client/apps/torii-launcher/index.html
- client/apps/torii-launcher/webpack.plugins.ts
- client/apps/torii-launcher/webpack.main.config.ts
- client/apps/torii-launcher/webpack.rules.ts
🔇 Additional comments (8)
pnpm-workspace.yaml (1)
3-6: Explicit Package Listing for Client AppsReplacing the wildcard entry with individual explicit paths (landing, game, balancing, and game-docs) greatly improves clarity and maintainability. This change ensures that only the intended applications are included in the workspace, reducing the risk of accidentally including extraneous directories. Moving forward, please ensure that any new apps are added explicitly to this file.
.knip.json (1)
28-30: New ignore rule added for the torii-launcher directory:
The new entry "client/apps/torii-launcher/**" efficiently excludes all files under that directory, which is in line with the broader restructuring objectives. Please double-check that this glob pattern correctly matches the intended files and doesn’t inadvertently ignore any assets required for the build or tests.client/apps/torii-launcher/webpack.renderer.config.ts (1)
32-32: Verify PostCSS configuration.The
postcss-loaderis added but its configuration is not specified. This might cause the loader to use default settings.Let's check if a PostCSS config file exists:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for PostCSS configuration files echo "Searching for PostCSS config files..." fd -e js -e json -e yaml -e yml -e config.js "postcss.config"Length of output: 329
PostCSS configuration is present in torii-launcher
A PostCSS configuration file exists at "client/apps/torii-launcher/postcss.config.js", so the loader will use this file rather than default settings.
client/apps/landing/src/hooks/context/dojo-context.tsx (2)
12-17: LGTM! Well-structured interface addition.The new
DojoResultinterface provides a clear and type-safe structure for the hook's return value, improving code organization and maintainability.
56-66: LGTM! Clean hook implementation.The
useDojohook implementation is well-structured and maintains proper error handling while providing a consistent interface through the newDojoResulttype.client/apps/game/.env.sepolia (2)
16-16: Verify version compatibility with dependent systems.The game version has been updated to v1.0.0-rc7, which is a significant version change.
Please ensure this version is compatible with all dependent systems and properly documented in the changelog.
11-13: Verify the availability and consistency of the new Sepolia endpoints.The Torii and node endpoints have been updated to use the Sepolia network paths.
Let's verify the endpoint availability and response:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check if the new endpoints are accessible curl -I https://api.cartridge.gg/x/eternum-sepolia/torii curl -I https://api.cartridge.gg/x/starknet/sepoliaLength of output: 1337
Endpoints Verified – Sepolia API endpoints are available and consistent.
- The Torii endpoint (https://api.cartridge.gg/x/eternum-sepolia/torii) returned an HTTP 200 status with a JSON payload.
- The Node endpoint (https://api.cartridge.gg/x/starknet/sepolia) also returned an HTTP 200, confirming its availability.
Both endpoints are responding as expected, confirming the updated configuration.
client/apps/landing/.env.sepolia (1)
15-18: Verify the placeholder URLs.The CDN, marketplace API, and IPFS gateway URLs are using placeholder values (
https://abc.com).Please ensure these URLs are updated with the correct production values before deployment.
| VITE_PUBLIC_TORII=https://api.cartridge.gg/x/eternum-sepolia/torii | ||
| VITE_PUBLIC_NODE_URL=https://api.cartridge.gg/x/starknet/sepolia | ||
| VITE_PUBLIC_TORII_RELAY=/dns4/api.cartridge.gg/tcp/443/x-parity-wss/%2Fx%2Fsepolia-rc-18%2Ftorii%2Fwss |
There was a problem hiding this comment.
Fix inconsistent Torii relay path between environments.
The Torii relay path in the landing environment (/x/sepolia-rc-18/torii/wss) differs from the game environment (/x/eternum-sepolia/torii/wss).
Please align the Torii relay paths across environments to ensure consistent connectivity:
-VITE_PUBLIC_TORII_RELAY=/dns4/api.cartridge.gg/tcp/443/x-parity-wss/%2Fx%2Fsepolia-rc-18%2Ftorii%2Fwss
+VITE_PUBLIC_TORII_RELAY=/dns4/api.cartridge.gg/tcp/443/x-parity-wss/%2Fx%2Feternum-sepolia%2Ftorii%2Fwss📝 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.
| VITE_PUBLIC_TORII=https://api.cartridge.gg/x/eternum-sepolia/torii | |
| VITE_PUBLIC_NODE_URL=https://api.cartridge.gg/x/starknet/sepolia | |
| VITE_PUBLIC_TORII_RELAY=/dns4/api.cartridge.gg/tcp/443/x-parity-wss/%2Fx%2Fsepolia-rc-18%2Ftorii%2Fwss | |
| VITE_PUBLIC_TORII=https://api.cartridge.gg/x/eternum-sepolia/torii | |
| VITE_PUBLIC_NODE_URL=https://api.cartridge.gg/x/starknet/sepolia | |
| VITE_PUBLIC_TORII_RELAY=/dns4/api.cartridge.gg/tcp/443/x-parity-wss/%2Fx%2Feternum-sepolia%2Ftorii%2Fwss |
contains:
Summary by CodeRabbit