fix game build #2623
fix game build #2623
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Warning Rate limit exceeded@aymericdelab has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 31 minutes and 34 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughThe pull request introduces multiple changes across various files in the project. Key updates include modifying import paths for manifest JSON files in Changes
Sequence DiagramsequenceDiagram
participant User
participant SettingsUI
participant LocalStorage
participant GuildsHook
User->>SettingsUI: Open Settings
SettingsUI->>GuildsHook: Fetch Guilds
GuildsHook-->>SettingsUI: Return Guild List
User->>SettingsUI: Select/Deselect Guilds
SettingsUI->>LocalStorage: Update Whitelisted Guilds
SettingsUI->>User: Show Toast Notification
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![mentatbot[bot]](https://avatars.githubusercontent.com/in/837875?s=60&v=4){kind=small}
There was a problem hiding this comment.
The changes look good overall. The manifest files have been moved to a more logical location under common/manifests and the imports have been updated accordingly. The useGuilds import that initially appeared unused is actually being used in the component. No issues found with the changes.
Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!
| @@ -105,7 +106,7 @@ export const SettingsWindow = () => { | |||
| return newFlatMode; | |||
There was a problem hiding this comment.
The useGuilds import is used in the component (on line 77), so it's correctly imported.
|
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
🔭 Outside diff range comments (3)
client/apps/game/src/ui/modules/settings/Settings.tsx (3)
Line range hint
82-87: Add error handling for localStorage operationsThe localStorage operations should be wrapped in try-catch blocks to handle potential errors (e.g., when localStorage is full or disabled).
const [selectedGuilds, setSelectedGuilds] = useState<string[]>(() => { - const savedGuilds = localStorage.getItem("WHITELIST"); - return savedGuilds ? savedGuilds.split(",") : []; + try { + const savedGuilds = localStorage.getItem("WHITELIST"); + return savedGuilds ? savedGuilds.split(",") : []; + } catch (error) { + console.error("Failed to load guild whitelist:", error); + return []; + } });
Line range hint
89-103: Add error handling for localStorage operations in handlersBoth handlers should handle potential localStorage failures gracefully.
const handleGuildSelect = (guildId: string) => { setSelectedGuilds((prev) => { const newGuilds = prev.includes(guildId) ? prev.filter((id) => id !== guildId) : [...prev, guildId]; - localStorage.setItem("WHITELIST", newGuilds.join(",")); - toast(prev.includes(guildId) ? "Guild removed from whitelist!" : "Guild added to whitelist!"); + try { + localStorage.setItem("WHITELIST", newGuilds.join(",")); + toast(prev.includes(guildId) ? "Guild removed from whitelist!" : "Guild added to whitelist!"); + } catch (error) { + toast.error("Failed to save guild selection"); + console.error("Failed to save guild whitelist:", error); + } return newGuilds; }); }; const handleClearGuilds = () => { setSelectedGuilds([]); - localStorage.removeItem("WHITELIST"); - toast("Guild whitelist cleared!"); + try { + localStorage.removeItem("WHITELIST"); + toast("Guild whitelist cleared!"); + } catch (error) { + toast.error("Failed to clear guild whitelist"); + console.error("Failed to clear guild whitelist:", error); + } };
Line range hint
171-186: Enhance accessibility and loading state handlingThe guild management UI needs improvements in accessibility and loading state handling:
- Add ARIA labels for better screen reader support
- Handle loading and error states for guild data
<div className="flex flex-col gap-2"> - <h5>Whitelist guilds</h5> + <h5 id="whitelist-heading">Whitelist guilds</h5> - <div className="flex flex-wrap gap-2"> + <div + className="flex flex-wrap gap-2" + role="group" + aria-labelledby="whitelist-heading" + > + {!guilds ? ( + <div>Loading guilds...</div> + ) : guilds.length === 0 ? ( + <div>No guilds available</div> + ) : ( {guilds.map((guild) => ( <Button size="xs" key={guild.entityId} variant={selectedGuilds.includes(guild.entityId.toString()) ? "success" : "outline"} onClick={() => handleGuildSelect(guild.entityId.toString())} + aria-pressed={selectedGuilds.includes(guild.entityId.toString())} > {guild.name} </Button> ))} + )} </div> {selectedGuilds.length > 0 && ( <Button size="xs" variant="danger" onClick={handleClearGuilds} className="self-start"> Clear All </Button> )} </div>
🧹 Nitpick comments (1)
client/apps/game/src/ui/modules/settings/Settings.tsx (1)
Line range hint
82-87: Consider using a more specific localStorage key nameThe key "WHITELIST" is too generic and could potentially conflict with other features. Consider using a more specific key like "GUILD_WHITELIST" or "SETTINGS_GUILD_WHITELIST".
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
client/apps/game/dojoConfig.ts(1 hunks)client/apps/game/src/ui/modules/settings/Settings.tsx(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- client/apps/game/dojoConfig.ts
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run pr agent on every pull request, respond to user comments
🔇 Additional comments (1)
client/apps/game/src/ui/modules/settings/Settings.tsx (1)
9-9: LGTM!The import statement follows the project's import organization pattern.
…into fix-build-raschel
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
client/apps/game/package.json (1)
49-51: Consider using a version range for starknet peer dependencyThe starknet peer dependency is pinned to an exact version (6.11.0). Consider using a version range (e.g., "^6.11.0") to allow for compatible updates while avoiding breaking changes.
- "starknet": "6.11.0" + "starknet": "^6.11.0"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
client/apps/balancing/public/vite.svgis excluded by!**/*.svgclient/apps/balancing/src/assets/react.svgis excluded by!**/*.svgpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
client/apps/balancing/package.json(1 hunks)client/apps/game/package.json(2 hunks)client/apps/landing/package.json(2 hunks)client/config/package.json(1 hunks)client/package.json(1 hunks)client/sdk/packages/eternum/package.json(2 hunks)contracts/package.json(1 hunks)contracts/season_pass/scripts/deployment/package.json(1 hunks)contracts/season_resources/scripts/deployment/package.json(0 hunks)package.json(1 hunks)pnpm-workspace.yaml(1 hunks)
💤 Files with no reviewable changes (1)
- contracts/season_resources/scripts/deployment/package.json
✅ Files skipped from review due to trivial changes (2)
- contracts/package.json
- package.json
⏰ Context from checks skipped due to timeout of 90000ms (14)
- GitHub Check: test-various (test_ownership_systems)
- GitHub Check: test-various (transport_test)
- GitHub Check: test-various (guild_test)
- GitHub Check: test-various (realm_test)
- GitHub Check: test-various (trade_test)
- GitHub Check: test-various (resource_bridge_test)
- GitHub Check: test-various (map_test)
- GitHub Check: test-various (bank_test)
- GitHub Check: test-various (hyperstructure_test)
- GitHub Check: test-various (config_test)
- GitHub Check: test-various (resources_test)
- GitHub Check: test-various (owner_test)
- GitHub Check: test-various (combat_test)
- GitHub Check: test-various (season_test)
🔇 Additional comments (6)
contracts/season_pass/scripts/deployment/package.json (1)
Line range hint
7-11: Verify deployment script dependenciesThe scripts reference bun and environment files, but all dependencies (
colors,dotenv,starknet) have been removed. This could break the deployment process.Run this script to check if dependencies are defined at workspace root:
client/config/package.json (1)
2-2: Verify package rename impactThe package has been renamed from "config" to "eternum-config". Ensure all imports and dependencies across the workspace are updated accordingly.
Run this script to check for potential broken imports:
✅ Verification successful
Package rename is safe
The package rename from "config" to "eternum-config" has no impact on the codebase. All existing config imports are relative paths to local configuration files, and no workspace packages depend on either the old or new package name.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to the old package name echo "Searching for 'config' package references:" rg -l '"config"' -g 'package.json' echo -e "\nSearching for config imports:" rg -l 'from ["'\'']config[/"'\'']'Length of output: 247
Script:
#!/bin/bash # Search for both old and new package names in package.json files echo "Searching package.json files for package references:" rg -l '"dependencies"|"devDependencies"|"peerDependencies"' -g 'package.json' | xargs cat | grep -E '"(config|eternum-config)":' echo -e "\nSearching for imports (TypeScript/JavaScript):" rg -l -t ts -t js -t tsx -t jsx "from ['\"].*config.*['\"]" echo -e "\nSearching for require statements:" rg -l -t ts -t js -t tsx -t jsx "require\(['\"].*config.*['\"]\)" echo -e "\nChecking workspace dependencies:" find . -name "package.json" -exec grep -l "workspace:" {} \;Length of output: 845
Script:
#!/bin/bash # Check workspace package.json contents echo "Contents of workspace package.json files:" echo -e "\nclient/apps/game/package.json:" cat client/apps/game/package.json echo -e "\ncontracts/formula/battles/package.json:" cat contracts/formula/battles/package.json echo -e "\nSearching for config imports (fixed pattern):" rg -l "from.*config" --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' -t ts -t js echo -e "\nSearching for relative path imports:" rg -l "\.\./config|\./config" --type-add 'ts:*.{ts,tsx}' --type-add 'js:*.{js,jsx}' -t ts -t jsLength of output: 5228
Script:
#!/bin/bash echo "Checking content of files with relative config imports:" echo -e "\nclient/sdk/packages/eternum/src/index.ts:" cat client/sdk/packages/eternum/src/index.ts echo -e "\nclient/apps/game/src/ui/layouts/World.tsx:" cat client/apps/game/src/ui/layouts/World.tsx echo -e "\nclient/apps/game/src/ui/elements/HintModalButton.tsx:" cat client/apps/game/src/ui/elements/HintModalButton.tsxLength of output: 14423
client/sdk/packages/eternum/package.json (1)
9-10: LGTM! Improved dependency organizationGood improvements:
- Added lint scripts for consistency
- Correctly moved type definitions to devDependencies
- Properly specified starknet as a peer dependency
Also applies to: 28-36
client/apps/game/package.json (1)
19-35: Verify Three.js version and check for breaking changesThe Three.js version (0.166.0) is not the latest available. Consider upgrading to the latest stable version for potential bug fixes and performance improvements.
Run this script to check for breaking changes:
✅ Verification successful
Three.js version 0.166.0 is appropriate for this project
The current version is stable and shows full compatibility with other dependencies, particularly three-stdlib. The extensive usage across the codebase shows no signs of version-specific issues that would require an immediate upgrade.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check Three.js changelog for breaking changes between versions curl -s https://raw.githubusercontent.com/mrdoob/three.js/dev/CHANGELOG.md | grep -A 10 "r167"Length of output: 94
Script:
#!/bin/bash # Check Three.js usage patterns and imports echo "=== Three.js imports ===" rg "from 'three'" -A 2 rg "from \"three\"" -A 2 echo -e "\n=== three-stdlib imports ===" rg "from 'three-stdlib'" -A 2 rg "from \"three-stdlib\"" -A 2 echo -e "\n=== Package versions ===" fd package.json --exec grep -l "three\|three-stdlib" {} \; --exec cat {}Length of output: 12834
client/apps/landing/package.json (1)
44-44: Verify dependency versions for security and compatibilityThe newly added dependencies should be verified:
- [email protected]
- @tanstack/[email protected]
Run this script to check for known vulnerabilities and latest versions:
Also applies to: 63-63
✅ Verification successful
Dependencies are secure but updates are available
- Current version of vite-plugin-vercel (9.0.3) is secure but can be updated to 9.0.4 for latest patches
- @tanstack/router-plugin (1.73.1) is secure but a significant update (1.95.1) is available - recommend planning the upgrade separately due to potential breaking changes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for vulnerabilities and latest versions echo "Checking vite-plugin-vercel..." curl -s https://registry.npmjs.org/vite-plugin-vercel | jq '.versions | keys[-1]' echo "Checking @tanstack/router-plugin..." curl -s https://registry.npmjs.org/@tanstack/router-plugin | jq '.versions | keys[-1]' # Check for known vulnerabilities gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NPM, package: "vite-plugin-vercel") { nodes { advisory { summary severity } vulnerableVersionRange } } }'Length of output: 618
Script:
#!/bin/bash # Check for known vulnerabilities in @tanstack/router-plugin gh api graphql -f query=' { securityVulnerabilities(first: 5, ecosystem: NPM, package: "@tanstack/router-plugin") { nodes { advisory { summary severity } vulnerableVersionRange } } }' # Check changelog or release notes for breaking changes curl -s https://raw.githubusercontent.com/TanStack/router/main/CHANGELOG.md || curl -s https://raw.githubusercontent.com/TanStack/router/master/CHANGELOG.mdLength of output: 373
client/package.json (1)
6-18: Well-organized workspace scripts structureThe script organization using --filter for different apps is a good practice for monorepo management. This makes it clear which commands apply to which packages.
| - "client" | ||
| - "client/apps/*" | ||
| - "client/config" | ||
| - "client/balancing" | ||
| - "contracts" |
There was a problem hiding this comment.
💡 Codebase verification
Workspace entries are too broad and may expose unintended packages
The broad entries "client" and "contracts" will expose several packages that might need isolation:
- In
client: SDK packages underclient/sdk/packages/ - In
contracts: Deployment script packages undercontracts/*/scripts/deployment/
Consider using specific entries like "contracts/formula/battles" instead of the broad "contracts" directory.
🔗 Analysis chain
Verify the scope of newly added workspace packages
The addition of broad package entries "client" and "contracts" includes entire directories in the workspace. This could potentially expose unintended modules to other packages.
Run this script to analyze the package structure:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# List all package.json files in client and contracts directories
echo "Package.json files in client directory:"
fd package.json client/
echo -e "\nPackage.json files in contracts directory:"
fd package.json contracts/
Length of output: 605
| @@ -1,12 +1,17 @@ | |||
| { | |||
| "name": "config", | |||
| "name": "eternum-config", | |||
There was a problem hiding this comment.
Fix incorrect ESLint version
The specified ESLint version ^9.9.0 appears incorrect as the current stable version is in the 8.x range.
Update the ESLint version to the latest stable:
- "eslint": "^9.9.0"
+ "eslint": "^8.57.0"Also applies to: 11-11
| }, | ||
| "dependencies": { | ||
| "@bibliothecadao/eternum": "workspace:^", | ||
| "@radix-ui/react-icons": "^1.3.0", | ||
| "@radix-ui/react-tabs": "^1.1.0", | ||
| "@tanstack/react-table": "^8.20.5", | ||
| "class-variance-authority": "^0.7.0", | ||
| "clsx": "^1.2.1", | ||
| "lucide-react": "^0.365.0", | ||
| "react": "^18.3.1", | ||
| "react-dom": "^18.3.1", | ||
| "tailwind-merge": "^2.5.2", | ||
| "tailwindcss-animate": "^1.0.7" | ||
| }, | ||
| "devDependencies": { | ||
| "@eslint/js": "^9.9.0", | ||
| "@types/node": "^20.11.10", | ||
| "@types/react": "^18.3.3", | ||
| "@types/react-dom": "^18.3.0", | ||
| "@vitejs/plugin-react": "^4.3.1", | ||
| "autoprefixer": "^10.4.18", | ||
| "eslint": "^9.9.0", | ||
| "eslint-plugin-react-hooks": "^5.1.0-rc.0", | ||
| "eslint-plugin-react-refresh": "^0.4.9", | ||
| "globals": "^15.9.0", | ||
| "postcss": "^8.4.35", | ||
| "tailwindcss": "^3.4.1", | ||
| "typescript": "^5.5.3", | ||
| "typescript-eslint": "^8.0.1", | ||
| "vite": "^5.4.1" | ||
| "typescript-eslint": "^8.0.1" |
There was a problem hiding this comment.
Critical dependencies missing for React application
Several essential dependencies have been removed:
- Core React packages (
react,react-dom) - Type definitions (
@types/react,@types/react-dom) - Build tooling (
@vitejs/plugin-react) - ESLint but plugins remain
This will break the application build and development workflow.
Restore the necessary dependencies:
"dependencies": {
"@radix-ui/react-icons": "^1.3.0",
"@tanstack/react-table": "^8.20.5",
"class-variance-authority": "^0.7.0",
+ "react": "^18.3.1",
+ "react-dom": "^18.3.1",
"tailwindcss-animate": "^1.0.7"
},
"devDependencies": {
"@eslint/js": "^9.9.0",
+ "@types/react": "^18.3.3",
+ "@types/react-dom": "^18.3.0",
+ "@vitejs/plugin-react": "^4.3.1",
+ "eslint": "^8.57.0",
"autoprefixer": "^10.4.18",📝 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.
| }, | |
| "dependencies": { | |
| "@bibliothecadao/eternum": "workspace:^", | |
| "@radix-ui/react-icons": "^1.3.0", | |
| "@radix-ui/react-tabs": "^1.1.0", | |
| "@tanstack/react-table": "^8.20.5", | |
| "class-variance-authority": "^0.7.0", | |
| "clsx": "^1.2.1", | |
| "lucide-react": "^0.365.0", | |
| "react": "^18.3.1", | |
| "react-dom": "^18.3.1", | |
| "tailwind-merge": "^2.5.2", | |
| "tailwindcss-animate": "^1.0.7" | |
| }, | |
| "devDependencies": { | |
| "@eslint/js": "^9.9.0", | |
| "@types/node": "^20.11.10", | |
| "@types/react": "^18.3.3", | |
| "@types/react-dom": "^18.3.0", | |
| "@vitejs/plugin-react": "^4.3.1", | |
| "autoprefixer": "^10.4.18", | |
| "eslint": "^9.9.0", | |
| "eslint-plugin-react-hooks": "^5.1.0-rc.0", | |
| "eslint-plugin-react-refresh": "^0.4.9", | |
| "globals": "^15.9.0", | |
| "postcss": "^8.4.35", | |
| "tailwindcss": "^3.4.1", | |
| "typescript": "^5.5.3", | |
| "typescript-eslint": "^8.0.1", | |
| "vite": "^5.4.1" | |
| "typescript-eslint": "^8.0.1" | |
| }, | |
| "dependencies": { | |
| "@radix-ui/react-icons": "^1.3.0", | |
| "@tanstack/react-table": "^8.20.5", | |
| "class-variance-authority": "^0.7.0", | |
| "react": "^18.3.1", | |
| "react-dom": "^18.3.1", | |
| "tailwindcss-animate": "^1.0.7" | |
| }, | |
| "devDependencies": { | |
| "@eslint/js": "^9.9.0", | |
| "@types/react": "^18.3.3", | |
| "@types/react-dom": "^18.3.0", | |
| "@vitejs/plugin-react": "^4.3.1", | |
| "eslint": "^8.57.0", | |
| "autoprefixer": "^10.4.18", | |
| "eslint-plugin-react-hooks": "^5.1.0-rc.0", | |
| "eslint-plugin-react-refresh": "^0.4.9", | |
| "globals": "^15.9.0", | |
| "postcss": "^8.4.35", | |
| "tailwindcss": "^3.4.1", | |
| "typescript-eslint": "^8.0.1" |
| "@types/react": "^18.3.3", | ||
| "@types/react-dom": "^18.3.0", | ||
| "eslint": "^9.9.0", | ||
| "typescript": "^5.5.3", |
There was a problem hiding this comment.
Fix invalid TypeScript version
The specified TypeScript version (^5.5.3) doesn't exist yet. The latest stable version is 5.3.3.
- "typescript": "^5.5.3",
+ "typescript": "^5.3.3"📝 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.
| "typescript": "^5.5.3", | |
| "typescript": "^5.3.3", |
| "@dojoengine/core": "1.0.4-alpha.3.1.0", | ||
| "@dojoengine/create-burner": "1.0.4-alpha.3.1.0", | ||
| "@dojoengine/react": "1.0.4-alpha.3.1.0", | ||
| "@dojoengine/recs": "^2.0.13", | ||
| "@dojoengine/state": "1.0.4-alpha.3.1.0", | ||
| "@dojoengine/torii-client": "1.0.4-alpha.3.1.0", | ||
| "@dojoengine/utils": "1.0.4-alpha.3.1.0", |
There was a problem hiding this comment.
💡 Codebase verification
Upgrade @dojoengine packages to latest stable version 1.0.8
The current alpha versions (1.0.4-alpha.3.1.0) are significantly behind the latest stable releases. Upgrading to version 1.0.8 will provide:
- Improved stability and reliability
- Latest bug fixes and security patches
- Better compatibility guarantees
Ensure all packages (except @dojoengine/recs) are upgraded together to maintain version consistency, and thoroughly test the application after the upgrade.
🔗 Analysis chain
Review alpha versions of @dojoengine packages
Multiple @dojoengine packages are using alpha versions (1.0.4-alpha.3.1.0). Consider the stability implications and ensure proper testing before deployment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for latest stable versions of @dojoengine packages
echo "Checking @dojoengine versions..."
for pkg in core create-burner react state torii-client utils; do
echo "@dojoengine/$pkg:"
curl -s "https://registry.npmjs.org/@dojoengine/$pkg" | jq -r '.["dist-tags"].latest'
done
Length of output: 1322
…into fix-build-raschel
Summary by CodeRabbit
Release Notes
New Features
Improvements
Chores