Add CDN support for battle scenes and beast images - #102
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis change introduces a centralized CDN configuration system for game assets. Environment variables are added to support both local and production CDN URLs. New asset helper functions are created to generate URLs for battle scenes and beast images, replacing scattered hard-coded paths throughout the codebase. Changes
Poem
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @loothero, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the game's performance and user experience by integrating Cloudflare CDN for image assets, leading to faster loading times and optimized delivery. It also introduces a highly requested feature allowing players to personalize their game tokens by editing their names directly from the leaderboard, backed by a new on-chain system call. Additionally, the project's AI agent documentation has been streamlined for better maintainability. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request successfully implements CDN support for game assets, which should improve loading times. The new assets.ts config file is a good way to centralize asset URL generation. The changes in beast.ts, Combat.tsx, and events.ts correctly adopt these new helpers.
I've left a few comments:
- A suggestion to make the
getAssetUrlhelper more robust against different CDN URL formats. - A minor correction for consistency in one of the new documentation files.
Additionally, this PR introduces a new feature allowing users to edit their player names on the leaderboard. This is a significant change that wasn't mentioned in the PR summary. The implementation looks solid, but I've suggested a small improvement to the Leaderboard components to provide more specific validation feedback to the user.
Overall, these are great changes that improve both performance and functionality.
There was a problem hiding this comment.
Pull request overview
This pull request claims to add CDN support for battle scene and beast images to improve global load times, but actually contains multiple unrelated features mixed together, which is a critical issue for code review and maintenance.
Changes:
- CDN configuration module for image asset delivery
- Complete player name editing functionality for leaderboards (unrelated)
- Comprehensive documentation restructuring (unrelated)
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
client/src/config/assets.ts |
New CDN configuration module with helper functions for constructing asset URLs |
client/src/utils/beast.ts |
Updated beast image functions to use CDN helpers |
client/src/utils/events.ts |
Battle scene preloading uses CDN-aware URLs |
client/src/desktop/overlays/Combat.tsx |
Combat backgrounds use CDN URLs |
client/.env.production |
Added CDN URL and Denshokan address (partially unrelated) |
client/.env.local |
Added empty CDN URL and Denshokan address (partially unrelated) |
client/src/mobile/components/Leaderboard.tsx |
Mixed: minimal CDN changes + extensive player name editing feature (unrelated) |
client/src/desktop/components/Leaderboard.tsx |
Mixed: minimal CDN changes + extensive player name editing feature (unrelated) |
client/src/dojo/useSystemCalls.ts |
Added updatePlayerName system call (unrelated) |
AGENTS.md, CLAUDE.md, GEMINI.md, client/AGENTS.md, client/CLAUDE.md, client/GEMINI.md, contracts/AGENTS.md, contracts/CLAUDE.md, contracts/GEMINI.md |
Complete documentation restructuring (unrelated) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/mobile/components/Leaderboard.tsx (1)
95-99: Missing dependency in useEffect.The
addressdependency is missing from the dependency array. Ifaddresschanges whiletokenResult.rankingremains the same,playerBestGamewon't be cleared.Proposed fix
useEffect(() => { if (address && tokenResult.ranking) { setPlayerBestGame(tokenResult.ranking); + } else if (!address) { + setPlayerBestGame(null); } - }, [tokenResult.ranking]); + }, [tokenResult.ranking, address]);
🤖 Fix all issues with AI agents
In @client/AGENTS.md:
- Around line 15-57: The fenced code block showing the project directory tree is
missing a language identifier; update the opening ``` fence in AGENTS.md for the
directory tree block to include a language such as "text" or "bash" (e.g.,
change ``` to ```text) so the directory listing is properly
highlighted/rendered; locate the block that begins with the "src/" tree and
modify its opening fence accordingly.
- Around line 145-151: The Environment Variables section in AGENTS.md is missing
the two new variables; update the env block to include VITE_PUBLIC_CDN_URL (CDN
base URL for game images, empty in dev) and VITE_PUBLIC_DENSHOKAN_ADDRESS
(contract address for player name updates) so the documented variables match the
additions in .env.local and .env.production; add brief comments for each
variable similar to the existing entries and ensure they appear alongside
VITE_PUBLIC_NODE_URL, VITE_PUBLIC_TORII, and VITE_PUBLIC_VRF_PROVIDER_ADDRESS in
the same code block.
In @client/GEMINI.md:
- Line 1: Update the inconsistent reference in client/GEMINI.md by replacing the
plain "AGENTS.md" reference with the prefixed form "@AGENTS.md" so it matches
the other documentation files; locate the string "AGENTS.md" in client/GEMINI.md
and change it to "@AGENTS.md".
In @client/src/config/assets.ts:
- Around line 39-43: getBeastImageUrl uses name.replace(' ', '_') which only
replaces the first space; update it to replace all spaces (e.g., use
name.replace(/ /g, '_') or name.replaceAll(' ', '_')) and normalize casing as
before, then pass the result into getAssetUrl with ASSETS_CONFIG.paths.beasts so
multi-word beast names produce correct URLs.
In @client/src/mobile/components/Leaderboard.tsx:
- Around line 293-320: The inner loading check is redundant because the parent
map already guards this branch with !loading; remove the unused conditional
branch that renders <Skeleton> and simplify the non-editing name container to
always render the <Typography> with the player name fallback
(localNameOverrides[game.token_id] ?? game.player_name) and the edit button when
ownedGameIds.has(game.token_id); update the JSX around the Box in the
non-editing branch (symbols: localNameOverrides, game.token_id,
game.player_name, ownedGameIds, startEditing, EditIcon, Skeleton) to eliminate
the unreachable Skeleton rendering.
In @client/src/utils/beast.ts:
- Around line 125-128: getBeastImageById currently passes BEAST_NAMES[id]
directly to getBeastImageUrl which will throw if the name is undefined; update
getBeastImageById to guard the lookup (check that id is in-range and name is
truthy) and either return a safe fallback URL (e.g., call getBeastImageUrl with
a placeholder name like "unknown" or a dedicated fallback constant) or throw a
clear Error referencing the invalid id; modify the function to use the
BEAST_NAMES lookup result (e.g., const name = BEAST_NAMES[id]) and then if
(!name) handle the fallback/error before calling getBeastImageUrl(name).
In @client/src/utils/events.ts:
- Around line 235-239: The getVideoId function currently uses non-null
assertions on event.beast!.baseName which will throw if a GameEvent has no
beast; change these to optional chaining and a soft-fail: read baseName via
event.beast?.baseName, if undefined return null (or an appropriate fallback)
before calling preloadAssets/getBattleSceneUrl, and then index streamIds with
that safe baseName; update the branch using preloadAssets, getBattleSceneUrl and
streamIds to use the guarded baseName rather than event.beast!.
🧹 Nitpick comments (3)
contracts/AGENTS.md (1)
22-51: Add language identifier to fenced code block.The directory structure code block is missing a language identifier. Add
textorplaintextafter the opening backticks for consistency with Markdown linting rules.📝 Suggested fix
-``` +```text src/ ├── lib.cairo # Module declarations ├── systems/ # Dojo contract implementationsBased on static analysis hints.
client/src/mobile/components/Leaderboard.tsx (2)
149-167: Consider handling the case where the name hasn't changed.The
saveNewNamefunction will still make an API call even if the trimmed name equals the original name. This could be optimized to avoid unnecessary network requests.Proposed optimization
const saveNewName = useCallback(async (tokenId: number) => { if (!editingName.trim()) { cancelEditing(); return; } const newName = editingName.trim(); + // Find the current game to compare names + const currentGame = displayedGames.find((g: any) => g.token_id === tokenId); + const currentName = localNameOverrides[tokenId] ?? currentGame?.player_name ?? ""; + if (newName === currentName) { + cancelEditing(); + return; + } + setIsSaving(true); try { await updatePlayerName(tokenId, newName);
39-44: Consider clearing editing state when tab changes or component unmounts.If a user is mid-edit and switches tabs, the editing state persists but the game being edited may no longer be visible. Add cleanup when
activeTabchanges.Proposed fix
+ useEffect(() => { + // Clear editing state when switching tabs + cancelEditing(); + }, [activeTab]);
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
AGENTS.mdCLAUDE.mdGEMINI.mdclient/.env.localclient/.env.productionclient/AGENTS.mdclient/CLAUDE.mdclient/GEMINI.mdclient/src/config/assets.tsclient/src/desktop/components/Leaderboard.tsxclient/src/desktop/overlays/Combat.tsxclient/src/dojo/useSystemCalls.tsclient/src/mobile/components/Leaderboard.tsxclient/src/utils/beast.tsclient/src/utils/events.tscontracts/AGENTS.mdcontracts/CLAUDE.mdcontracts/GEMINI.md
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
.tsextension for utility files (non-component TypeScript files)
Files:
client/src/utils/events.tsclient/src/config/assets.tsclient/src/dojo/useSystemCalls.tsclient/src/utils/beast.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript strict mode
Use React functional components with hooks
Files:
client/src/utils/events.tsclient/src/desktop/overlays/Combat.tsxclient/src/config/assets.tsclient/src/dojo/useSystemCalls.tsclient/src/utils/beast.tsclient/src/mobile/components/Leaderboard.tsxclient/src/desktop/components/Leaderboard.tsx
**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use
.tsxextension for React component files
Files:
client/src/desktop/overlays/Combat.tsxclient/src/mobile/components/Leaderboard.tsxclient/src/desktop/components/Leaderboard.tsx
client/src/**/*.tsx
📄 CodeRabbit inference engine (CLAUDE.md)
Use Material-UI components and maintain consistent styling
Files:
client/src/desktop/overlays/Combat.tsxclient/src/mobile/components/Leaderboard.tsxclient/src/desktop/components/Leaderboard.tsx
🧠 Learnings (18)
📚 Learning: 2025-12-17T13:20:17.126Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:20:17.126Z
Learning: Applies to client/.env : Configure environment variables for VITE_PUBLIC_NODE_URL, VITE_PUBLIC_TORII, and contract addresses (ETH, LORDS tokens, game contracts)
Applied to files:
CLAUDE.mdclient/.env.productionclient/AGENTS.mdclient/src/config/assets.tsclient/.env.localclient/src/dojo/useSystemCalls.ts
📚 Learning: 2025-09-27T04:41:07.357Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: contracts/AGENTS.md:0-0
Timestamp: 2025-09-27T04:41:07.357Z
Learning: For PRs, describe gameplay/contract impact, migrations/manifest updates, manual test commands, and link the issue; include screenshots/logs when behavior changes
Applied to files:
client/AGENTS.mdAGENTS.mdcontracts/AGENTS.md
📚 Learning: 2025-12-17T13:20:17.126Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:20:17.126Z
Learning: React + TypeScript application using Vite as build tool for frontend development
Applied to files:
client/AGENTS.md
📚 Learning: 2025-12-17T13:20:17.126Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:20:17.126Z
Learning: Applies to client/src/stores/**/*.ts : Use Zustand for global state management
Applied to files:
client/AGENTS.md
📚 Learning: 2025-12-17T13:20:17.126Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:20:17.126Z
Learning: Organize frontend code with platform-specific UI in separate directories (desktop/ and mobile/ folders)
Applied to files:
client/AGENTS.md
📚 Learning: 2025-12-17T13:20:17.126Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:20:17.126Z
Learning: Death Mountain is a blockchain-based adventure RPG game built on StarkNet using the Dojo engine featuring adventurers battling beasts, collecting loot, and progressing through challenges
Applied to files:
AGENTS.md
📚 Learning: 2025-09-27T04:41:07.357Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: contracts/AGENTS.md:0-0
Timestamp: 2025-09-27T04:41:07.357Z
Learning: Applies to contracts/src/systems/**/contracts.cairo : Place #[starknet::interface] above the corresponding #[dojo::contract] module and group public interfaces there
Applied to files:
AGENTS.mdcontracts/AGENTS.md
📚 Learning: 2025-12-17T13:20:17.126Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:20:17.126Z
Learning: Use Dojo engine (version 1.5.1) for on-chain game state management
Applied to files:
AGENTS.md
📚 Learning: 2025-12-17T13:20:17.126Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:20:17.126Z
Learning: Applies to client/src/**/*.tsx : Use Material-UI components and maintain consistent styling
Applied to files:
client/src/mobile/components/Leaderboard.tsxclient/src/desktop/components/Leaderboard.tsx
📚 Learning: 2025-09-27T04:41:07.357Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: contracts/AGENTS.md:0-0
Timestamp: 2025-09-27T04:41:07.357Z
Learning: Applies to contracts/src/models/**/*.cairo : Define packed structs and events under src/models/ (e.g., models/adventurer/*.cairo)
Applied to files:
contracts/AGENTS.md
📚 Learning: 2025-09-27T04:41:07.357Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: contracts/AGENTS.md:0-0
Timestamp: 2025-09-27T04:41:07.357Z
Learning: Applies to contracts/src/systems/**/contracts.cairo : Expose StarkNet entry points in systems/<module>/contracts.cairo
Applied to files:
contracts/AGENTS.md
📚 Learning: 2025-09-27T04:41:07.357Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: contracts/AGENTS.md:0-0
Timestamp: 2025-09-27T04:41:07.357Z
Learning: Applies to contracts/src/systems/**/contracts.cairo : Use dojo_cairo_test::spawn_test_world and starknet::testing utilities for deterministic contexts
Applied to files:
contracts/AGENTS.md
📚 Learning: 2025-09-27T04:41:07.357Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: contracts/AGENTS.md:0-0
Timestamp: 2025-09-27T04:41:07.357Z
Learning: Applies to contracts/src/systems/**/contracts.cairo : Co-locate tests inside contracts.cairo with #[test] functions near the bottom
Applied to files:
contracts/AGENTS.md
📚 Learning: 2025-09-27T04:41:07.357Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: contracts/AGENTS.md:0-0
Timestamp: 2025-09-27T04:41:07.357Z
Learning: Applies to contracts/{src,utils}/**/*.cairo : Follow Cairo 2 defaults: 4-space indentation; snake_case for functions/modules; UpperCamelCase for types; CONSTANT_CASE for constants
Applied to files:
contracts/AGENTS.md
📚 Learning: 2025-09-27T04:41:07.357Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: contracts/AGENTS.md:0-0
Timestamp: 2025-09-27T04:41:07.357Z
Learning: Applies to contracts/{src,utils}/**/*.cairo : Run scarb fmt before pushing (max-line-length = 120; sort-module-level-items = true)
Applied to files:
contracts/AGENTS.md
📚 Learning: 2025-09-27T04:41:07.357Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: contracts/AGENTS.md:0-0
Timestamp: 2025-09-27T04:41:07.357Z
Learning: Applies to contracts/**/dojo_*.toml : Maintain profile-specific settings in dojo_*.toml files
Applied to files:
contracts/AGENTS.md
📚 Learning: 2025-09-27T04:41:07.357Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: contracts/AGENTS.md:0-0
Timestamp: 2025-09-27T04:41:07.357Z
Learning: Applies to contracts/utils/setup_denshokan.cairo : Keep and update Dojo test scaffolding in utils/setup_denshokan.cairo
Applied to files:
contracts/AGENTS.md
📚 Learning: 2025-12-17T13:20:17.126Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:20:17.126Z
Learning: Applies to contracts/src/**/*.cairo : Use Cairo 2.10.1 syntax for smart contracts
Applied to files:
contracts/AGENTS.md
🧬 Code graph analysis (6)
client/src/utils/events.ts (1)
client/src/config/assets.ts (1)
getBattleSceneUrl(29-34)
client/src/desktop/overlays/Combat.tsx (1)
client/src/config/assets.ts (1)
getBattleSceneUrl(29-34)
client/src/dojo/useSystemCalls.ts (1)
client/src/utils/utils.ts (1)
stringToFelt(4-5)
client/src/utils/beast.ts (2)
client/src/config/assets.ts (1)
getBeastImageUrl(39-43)client/src/constants/beast.ts (1)
BEAST_NAMES(124-220)
client/src/mobile/components/Leaderboard.tsx (1)
client/src/dojo/useSystemCalls.ts (1)
useSystemCalls(23-603)
client/src/desktop/components/Leaderboard.tsx (1)
client/src/dojo/useSystemCalls.ts (1)
useSystemCalls(23-603)
🪛 dotenv-linter (4.0.0)
client/.env.production
[warning] 2-2: [UnorderedKey] The VITE_PUBLIC_DENSHOKAN_ADDRESS key should go before the VITE_PUBLIC_VRF_PROVIDER_ADDRESS key
(UnorderedKey)
[warning] 3-3: [UnorderedKey] The VITE_PUBLIC_CLOUDFLARE_ID key should go before the VITE_PUBLIC_DENSHOKAN_ADDRESS key
(UnorderedKey)
[warning] 4-4: [UnorderedKey] The VITE_PUBLIC_ALCHEMY_URL key should go before the VITE_PUBLIC_CLOUDFLARE_ID key
(UnorderedKey)
[warning] 5-5: [UnorderedKey] The VITE_PUBLIC_POSTHOG_KEY key should go before the VITE_PUBLIC_VRF_PROVIDER_ADDRESS key
(UnorderedKey)
[warning] 6-6: [UnorderedKey] The VITE_PUBLIC_POSTHOG_HOST key should go before the VITE_PUBLIC_POSTHOG_KEY key
(UnorderedKey)
[warning] 8-8: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 8-8: [UnorderedKey] The VITE_PUBLIC_CDN_URL key should go before the VITE_PUBLIC_CLOUDFLARE_ID key
(UnorderedKey)
client/.env.local
[warning] 2-2: [UnorderedKey] The VITE_PUBLIC_DENSHOKAN_ADDRESS key should go before the VITE_PUBLIC_VRF_PROVIDER_ADDRESS key
(UnorderedKey)
[warning] 3-3: [UnorderedKey] The VITE_PUBLIC_CLOUDFLARE_ID key should go before the VITE_PUBLIC_DENSHOKAN_ADDRESS key
(UnorderedKey)
[warning] 4-4: [UnorderedKey] The VITE_PUBLIC_ALCHEMY_URL key should go before the VITE_PUBLIC_CLOUDFLARE_ID key
(UnorderedKey)
[warning] 5-5: [UnorderedKey] The VITE_PUBLIC_POSTHOG_KEY key should go before the VITE_PUBLIC_VRF_PROVIDER_ADDRESS key
(UnorderedKey)
[warning] 6-6: [UnorderedKey] The VITE_PUBLIC_POSTHOG_HOST key should go before the VITE_PUBLIC_POSTHOG_KEY key
(UnorderedKey)
[warning] 8-8: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 8-8: [UnorderedKey] The VITE_PUBLIC_CDN_URL key should go before the VITE_PUBLIC_CLOUDFLARE_ID key
(UnorderedKey)
🪛 Gitleaks (8.30.0)
client/.env.production
[high] 5-5: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 markdownlint-cli2 (0.18.1)
client/AGENTS.md
15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
contracts/AGENTS.md
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (28)
client/.env.local (1)
2-2: Environment variables for CDN and contract support are correctly configured.The empty
VITE_PUBLIC_CDN_URLin development allows fallback to local assets in/public/images/, while production is set separately. The newVITE_PUBLIC_DENSHOKAN_ADDRESSfollows the existing contract address pattern.Also applies to: 7-8
client/.env.production (1)
2-2: Production CDN URL correctly targets Cloudflare custom domain.The
VITE_PUBLIC_CDN_URL=https://images.lootsurvivor.ioproperly points to the Cloudflare R2 bucket's custom domain, enabling edge caching and Polish optimization as described in the PR objectives.Also applies to: 7-8
GEMINI.md (1)
1-1: LGTM! Reference pattern is consistent.The file correctly references @AGENTS.md, following the established pattern used across other documentation files in this repository.
client/src/dojo/useSystemCalls.ts (3)
35-35: LGTM! Environment variable follows established pattern.The
DENSHOKAN_ADDRESSenvironment variable is correctly configured following the same pattern as other contract addresses in the file.
600-601: LGTM! Function correctly exported.The
updatePlayerNamefunction is properly exposed in the hook's return object, making it available to consumer components like the Leaderboard.
474-507: LGTM! Function implementation is well-structured with proper validation.The function correctly:
- Validates configuration, account, and name length (1-31 characters aligns with felt252 limits)
- Uses
stringToFeltfor Cairo compatibility- Handles errors appropriately with logging and rethrowing
All dependencies are properly imported and the validation logic is sound.
contracts/AGENTS.md (1)
1-169: LGTM! Comprehensive and well-structured documentation.The updated AGENTS.md provides clear guidance on:
- Cairo/Dojo development workflow
- Project structure and conventions
- Code patterns and examples
- Testing and configuration
The code examples follow Cairo 2 syntax and Dojo best practices as outlined in the retrieved learnings.
client/src/config/assets.ts (3)
3-12: LGTM! Clean CDN configuration structure.The configuration properly:
- Falls back to empty string for local development (serves from
/public/images/)- Centralizes asset paths
- Follows the Vite environment variable convention
17-24: LGTM! URL construction logic is correct.The function properly handles:
- Double slash prevention when base URL is present
- Fallback to relative paths for local development
- Both leading-slash and non-leading-slash paths
29-34: LGTM! Battle scene URL generation is well-implemented.The function correctly:
- Supports jackpot variant scenes
- Normalizes beast names to lowercase
- Uses centralized path configuration
client/src/desktop/components/Leaderboard.tsx (6)
1-43: LGTM! State management and imports are well-organized.The implementation includes:
- Proper constant for name length matching backend validation (31 chars)
- Appropriate state for editing flow and optimistic updates
- Necessary Material-UI icons and hooks
104-123: LGTM! Effective race condition prevention.The validation logic correctly handles the tab-switching race condition by:
- Verifying all games belong to the user before displaying in "My Games" tab
- Allowing any data in "All" tab
- Using a ref to track when the games array actually changes
This prevents stale "All Games" data from appearing when switching to "My Games".
125-132: LGTM! Efficient ownership tracking.Using a
Setfor owned game IDs provides O(1) lookups when determining edit permissions, and the memoization dependencies are correct.
138-166: LGTM! Editing functions are well-implemented.The implementation correctly:
- Uses optimistic updates via
localNameOverrides- Validates input before saving
- Provides user feedback on errors
- Uses
useCallbackto prevent unnecessary re-renders
247-319: LGTM! Inline editing UI is well-implemented.The interface provides:
- Intuitive keyboard shortcuts (Enter to save, Escape to cancel)
- Clear visual feedback with loading indicators
- Accessibility attributes
- Proper gating of edit functionality to owned games only
423-467: LGTM! Styles follow Material-UI patterns.The styling is consistent with the existing codebase:
- Uses theme values for colors
- Includes proper hover states
- Follows the sx prop pattern used throughout the file
Based on coding guidelines for Material-UI usage in client/src/**/*.tsx.
client/src/mobile/components/Leaderboard.tsx (2)
101-120: Data validation logic looks good for preventing stale data display.The approach of validating ownership before updating
displayedGameswhen on the "My Games" tab prevents race conditions from showing incorrect data during tab switches. The reference comparisongames !== prevGamesRef.currentcorrectly avoids redundant processing.
248-267: Good accessibility implementation for the inline editing input.The editing input includes proper
aria-label, keyboard handling for Enter/Escape,autoFocus, and a disabled state during save operations. This provides a good user experience.contracts/CLAUDE.md (1)
1-2: Documentation reference looks correct.The file creates a pointer to
AGENTS.mdfor centralized agent documentation. This aligns with the pattern used in other similar files in the PR.CLAUDE.md (1)
1-1: Documentation reference approved.Consistent with the documentation restructuring pattern used elsewhere in this PR.
client/src/desktop/overlays/Combat.tsx (2)
14-14: Good adoption of centralized asset URL helper.The import of
getBattleSceneUrlaligns with the PR's goal of centralizing CDN-aware asset URL construction.
128-128: CDN integration looks correct.The usage of
getBattleSceneUrl(beast!.baseName, !!isJackpot)properly leverages the centralized helper. The conditional renderingbeast?.baseName &&ensures the function is only called whenbaseNameis available, making the non-null assertion safe in this context.contracts/GEMINI.md (1)
1-2: Documentation reference approved.The format is consistent with other documentation pointer files in the contracts directory.
client/CLAUDE.md (1)
1-1: Documentation reference approved.Consistent with the documentation restructuring pattern across the repository.
client/src/utils/events.ts (1)
10-10: Good move centralizing battle-scene URL building.
ImportinggetBattleSceneUrlhere alignsevents.tswith the new CDN-aware asset strategy.client/src/utils/beast.ts (2)
7-8: Nice cleanup: beast image URLs are now centralized.
Keeps path/host logic out ofutils/beast.tsand makes CDN rollout consistent.
121-123:getBeastImage()delegation looks good.AGENTS.md (1)
9-22: No action needed. The version claims in AGENTS.md (Dojo 1.6.0, Cairo 2.10.1) are correct and match the actual toolchain configuration incontracts/Scarb.toml. Both referencedCLAUDE.mdfiles exist. Formatting requirements also align with repo configuration (max-line-length = 120, sort-module-level-items = true).Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f8b04aacf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- Create centralized asset configuration module (src/config/assets.ts) - Add getBattleSceneUrl and getBeastImageUrl helpers for CDN URL construction - Update Combat.tsx to use CDN helper for battle scene backgrounds - Update beast.ts to use CDN helper for beast images - Update events.ts to use CDN helper for asset preloading - Add VITE_PUBLIC_CDN_URL environment variable (empty for dev, CDN URL for prod) Production images served from https://images.lootsurvivor.io with Cloudflare edge caching, Polish image optimization, and 1-year cache TTL. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
3f8b04a to
ee64eeb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
client/src/config/assets.ts (1)
39-43: Consider using a more robust space replacement.The current
.replace(' ', '_')only replaces the first space occurrence. While existing beast names appear to be single words, using.replaceAll(' ', '_')or.replace(/\s+/g, '_')would be more robust for future-proofing.♻️ Proposed refactor for robustness
export const getBeastImageUrl = (name: string): string => { return getAssetUrl( - `${ASSETS_CONFIG.paths.beasts}/${name.replace(' ', '_').toLowerCase()}.png` + `${ASSETS_CONFIG.paths.beasts}/${name.replaceAll(' ', '_').toLowerCase()}.png` ); };
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
client/.env.localclient/.env.productionclient/src/config/assets.tsclient/src/desktop/overlays/Combat.tsxclient/src/utils/beast.tsclient/src/utils/events.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- client/src/desktop/overlays/Combat.tsx
- client/src/utils/events.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use
.tsextension for utility files (non-component TypeScript files)
Files:
client/src/config/assets.tsclient/src/utils/beast.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Use TypeScript strict mode
Use React functional components with hooks
Files:
client/src/config/assets.tsclient/src/utils/beast.ts
🧠 Learnings (1)
📚 Learning: 2025-12-17T13:20:17.126Z
Learnt from: CR
Repo: Provable-Games/death-mountain PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-17T13:20:17.126Z
Learning: Applies to client/.env : Configure environment variables for VITE_PUBLIC_NODE_URL, VITE_PUBLIC_TORII, and contract addresses (ETH, LORDS tokens, game contracts)
Applied to files:
client/src/config/assets.tsclient/.env.localclient/.env.production
🧬 Code graph analysis (1)
client/src/utils/beast.ts (2)
client/src/config/assets.ts (1)
getBeastImageUrl(39-43)client/src/constants/beast.ts (1)
BEAST_NAMES(124-220)
🪛 dotenv-linter (4.0.0)
client/.env.local
[warning] 5-5: [UnorderedKey] The VITE_PUBLIC_POSTHOG_HOST key should go before the VITE_PUBLIC_POSTHOG_KEY key
(UnorderedKey)
[warning] 7-7: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 7-7: [UnorderedKey] The VITE_PUBLIC_CDN_URL key should go before the VITE_PUBLIC_CLOUDFLARE_ID key
(UnorderedKey)
client/.env.production
[warning] 5-5: [UnorderedKey] The VITE_PUBLIC_POSTHOG_HOST key should go before the VITE_PUBLIC_POSTHOG_KEY key
(UnorderedKey)
[warning] 7-7: [EndingBlankLine] No blank line at the end of the file
(EndingBlankLine)
[warning] 7-7: [UnorderedKey] The VITE_PUBLIC_CDN_URL key should go before the VITE_PUBLIC_CLOUDFLARE_ID key
(UnorderedKey)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: sozo-test
- GitHub Check: claude-review
🔇 Additional comments (8)
client/.env.production (1)
6-7: LGTM! CDN configuration properly added.The CDN URL is correctly configured for production. The static analysis warnings about key ordering and missing blank line are pedantic style preferences that can be safely ignored—environment variables are typically grouped by functionality rather than alphabetically.
client/.env.local (1)
6-7: LGTM! Local development configuration is correct.The empty CDN URL value with the explanatory comment is the right approach for local development. Static analysis warnings can be ignored for the same reasons as in the production file.
client/src/config/assets.ts (3)
3-12: LGTM! Clean configuration structure.The CDN configuration is well-structured with appropriate fallback behavior for local development.
17-24: LGTM! URL construction logic is sound.The function correctly handles path concatenation with proper double-slash prevention for both CDN and local scenarios.
29-34: LGTM! Battle scene URL logic is correct.The function appropriately handles both standard and jackpot scenarios with clear filename construction.
client/src/utils/beast.ts (3)
7-7: LGTM! Import correctly added.The import statement properly references the new centralized asset configuration module.
121-123: LGTM! Clean refactoring to use centralized helper.The function now correctly delegates to the centralized URL builder while maintaining the same public API.
125-128: LGTM! Correct refactoring with proper name resolution.The function properly resolves the beast name from the ID and delegates to the centralized URL builder.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Implements Cloudflare CDN delivery for game images (battle scenes and beast images), reducing load times globally through edge caching and automatic image optimization.
Changes
src/config/assets.ts: Centralized CDN configuration module with helpers:getAssetUrl()- Construct full asset URL with CDN basegetBattleSceneUrl()- Get battle scene background URLgetBeastImageUrl()- Get beast image URLsrc/utils/beast.ts:getBeastImage()andgetBeastImageById()now use CDN helpersrc/desktop/overlays/Combat.tsx: Battle scene backgrounds usegetBattleSceneUrl()src/utils/events.ts: Asset preloading uses CDN-aware URLsVITE_PUBLIC_CDN_URL(empty for dev, CDN URL for prod)Configuration
VITE_PUBLIC_CDN_URL→ images served from/public/images/VITE_PUBLIC_CDN_URL=https://images.lootsurvivor.io→ images served from CDNCDN Setup (Already Configured)
lootsurvivor-imagesimages.lootsurvivor.ioTesting
pnpm build)cf-polished: okheader)@codex review
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.