Add follow player feature with in-app notifications - #105
Conversation
Allow players to edit their adventurer names directly from the Leaderboard view. Only game owners can see and use the edit functionality. Changes: - Add updatePlayerName system call to useSystemCalls hook - Add inline edit UI with input field, save/cancel buttons - Add ownership check to show edit icon only for owned games - Support keyboard shortcuts (Enter to save, Escape to cancel) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Store locally updated names to prevent UI reverting to old name while waiting for backend to sync after transaction. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When re-editing a name, use the locally stored name instead of the stale value from the backend data. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Validate incoming games data matches the active tab before updating. For "My Games" tab, only accept data where all games are owned by the current user - this prevents stale "All" data from overwriting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Port all desktop leaderboard improvements to mobile: - Inline name editing with save/cancel buttons - Local name overrides for optimistic UI updates - Race condition fix to prevent stale data when switching tabs - Ownership check to show edit icon only for owned games 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Check that address is defined before using addAddressPadding in the race condition validation effect. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace hardcoded contract address with VITE_PUBLIC_DENSHOKAN_ADDRESS env var - Add validation for account connection and name length in updatePlayerName - Add error snackbar notification when name update fails - Fix loading vs empty state display logic - Add aria-labels for accessibility (InputBase, Save/Cancel/Edit buttons) - Extract MAX_PLAYER_NAME_LENGTH constant - Pre-compute ownership using useMemo Set for better performance - Use theme color 'primary.main' for CircularProgress Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Implement ability for players to follow other players and receive notifications when followed players start new games. New files: - followStore.ts: Zustand store with localStorage persistence - FollowButton.tsx: Heart icon button for follow/unfollow - FollowingList.tsx: Component to display followed players - useFollowedPlayerNotifications.tsx: Polling hook for game notifications Changes: - Add "Following" tab to desktop and mobile leaderboards - Add follow button next to player names in leaderboard - Initialize notification subscription in App.tsx Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughAdds a persistent player-following system, UI components for follow/search/list, polling notifications for newly minted games from followed players via Torii, inline player-name editing backed by a Denshokan contract call, environment config for Denshokan, and consolidated AGENTS.md documentation across repo, client, and contracts. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant UI as "UI\n(FollowButton / Leaderboard)"
participant Store as "Follow Store\n(useFollowStore)"
participant Hook as "Notif Hook\n(useFollowedPlayerNotifications)"
participant Torii as "Torii\n(SQL)"
participant SC as "System Calls\n(useSystemCalls)"
participant Contract as "Denshokan\nContract"
User->>UI: Click follow / unfollow
UI->>Store: followPlayer(address) / unfollowPlayer(address)
Store-->>UI: state updated -> re-render
Note over Hook,Store: Polling for new games
Hook->>Store: getFollowedAddresses()
Hook->>Torii: query TokenMetadataUpdate (since lastSeen)
Torii-->>Hook: results
Hook->>UI: enqueue snackbar (Watch button) for new games
User->>UI: Edit name & save
UI->>SC: updatePlayerName(tokenId, name)
SC->>Contract: call update_player_name (tx)
Contract-->>SC: tx receipt
SC-->>UI: return receipt
UI->>Store: apply local name override / re-render
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 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 |
|
Claude finished @loothero's task —— View job Code Review: Follow Player FeatureReviewing the follow player feature implementation with in-app notifications. Todo List
Code Review Summary✅ StrengthsArchitecture & Patterns:
StarkNet Integration:
User Experience:
🔴 Critical Issues1. SQL Injection Vulnerability ( const ownersList = followedAddresses
.map((addr) => `"${addAddressPadding(addr)}"`)
.join(',');
const url = `${currentNetworkConfig.toriiUrl}/sql?query=
SELECT token_id, owner, player_name, minted_at
FROM "relayer_0_0_1-TokenMetadataUpdate"
WHERE owner IN (${ownersList})`; // Direct string interpolation!Impact: Malicious addresses could inject SQL commands 2. Performance Concerns
🟡 Security & Robustness Issues3. Error Handling Gaps
4. Memory Management
🟠 Minor Issues5. Code Quality
RecommendationsHigh Priority
Medium Priority
Low Priority
Overall AssessmentThis is a well-architected feature that follows the codebase patterns effectively. The core functionality is solid, but the SQL injection vulnerability is critical and must be addressed before merging. With security fixes and performance optimizations, this will be a valuable addition to the game's social features. Recommendation: Request changes to address security issues before approval. |
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 social and interactive aspects of the game by introducing a 'Follow Player' feature. Players can now track their favorite or friends' activities, receiving timely in-app notifications when followed players initiate new games. This is complemented by UI updates to the leaderboard, allowing easy following/unfollowing and filtering of games, alongside the ability for players to personalize their in-game names. The changes also include a major overhaul of the project's documentation structure. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a 'follow player' feature, complete with in-app notifications for when followed players start new games. The implementation includes a new Zustand store for managing follow state, new UI components for following/unfollowing and listing followed players, and a polling mechanism to check for new games. The leaderboard is also updated to integrate this new functionality.
The code is generally well-structured and the new features are thoughtfully implemented. I've identified a couple of high-severity issues in the notification polling logic that could lead to incorrect behavior, and some medium-severity issues in the leaderboard components related to performance and type safety. My detailed comments provide suggestions for addressing these points.
| const url = `${currentNetworkConfig.toriiUrl}/sql?query= | ||
| SELECT token_id, owner, player_name, minted_at | ||
| FROM "relayer_0_0_1-TokenMetadataUpdate" | ||
| WHERE owner IN (${ownersList}) | ||
| ORDER BY minted_at DESC | ||
| LIMIT 10`; |
There was a problem hiding this comment.
The SQL query hardcodes the table name "relayer_0_0_1-TokenMetadataUpdate". The relayer_0_0_1 part appears to be a namespace. Hardcoding this is brittle and may break if the namespace changes in different environments or future deployments. This value should be retrieved from a configuration, similar to how namespace is used in other parts of the codebase (e.g., from currentNetworkConfig).
| // Find games that were minted after our last check | ||
| const newGames = data.filter((game) => { | ||
| const mintedAt = new Date(game.minted_at).getTime(); | ||
| return mintedAt > lastSeenTimestampRef.current; | ||
| }); | ||
|
|
||
| // Show notifications for new games | ||
| for (const game of newGames) { | ||
| const normalizedOwner = addAddressPadding(game.owner).toLowerCase(); | ||
| const followedPlayer = followedPlayers[normalizedOwner]; | ||
| const playerName = followedPlayer?.name || game.player_name || 'A followed player'; | ||
|
|
||
| enqueueSnackbar(`${playerName} just started a new game!`, { | ||
| variant: 'info', | ||
| autoHideDuration: 8000, | ||
| action: ( | ||
| <Button | ||
| size="small" | ||
| color="inherit" | ||
| onClick={() => watchGame(game.token_id)} | ||
| > | ||
| Watch | ||
| </Button> | ||
| ), | ||
| }); | ||
| } |
There was a problem hiding this comment.
The current logic to find new games uses mintedAt > lastSeenTimestampRef.current. If multiple games are created with the exact same timestamp as the last seen one, they will be missed. This can be fixed by using >= and tracking notified game IDs to prevent duplicates.
You'll need to add a ref to store notified game IDs at the top of your component: const notifiedGameIds = useRef(new Set<number>());
// Find games that were minted after our last check
const newGames = data.filter((game) => {
const mintedAt = new Date(game.minted_at).getTime();
return mintedAt >= lastSeenTimestampRef.current && !notifiedGameIds.current.has(game.token_id);
});
// Show notifications for new games
for (const game of newGames) {
const normalizedOwner = addAddressPadding(game.owner).toLowerCase();
const followedPlayer = followedPlayers[normalizedOwner];
const playerName = followedPlayer?.name || game.player_name || 'A followed player';
enqueueSnackbar(`${playerName} just started a new game!`, {
variant: 'info',
autoHideDuration: 8000,
action: (
<Button
size="small"
color="inherit"
onClick={() => watchGame(game.token_id)}
>
Watch
</Button>
),
});
notifiedGameIds.current.add(game.token_id);
}
| } else if (activeTab === 2) { | ||
| // Following tab: filter to show only games from followed players | ||
| const filteredGames = games.filter((g: any) => { | ||
| const normalizedOwner = addAddressPadding(g.owner).toLowerCase(); | ||
| return followedAddresses.includes(normalizedOwner); | ||
| }); | ||
| setDisplayedGames(filteredGames); | ||
| } |
There was a problem hiding this comment.
For the "Following" tab, all games are fetched and then filtered on the client-side. This can be inefficient if the total number of games is large. The useFollowedPlayerNotifications hook demonstrates building a direct SQL query with WHERE owner IN (...) to fetch data only for specific players. A similar approach could be used here for better performance, for example by creating a new specialized hook that fetches only the games for followed players, or by extending useGameTokens to support filtering by multiple owners if possible.
| <Typography sx={{ textAlign: "center", py: 2 }}> | ||
| {activeTab === 1 ? "You have no games yet." : activeTab === 2 ? "No games from followed players." : "No games found."} | ||
| </Typography> | ||
| ) : displayedGames.map((game: any, index: number) => ( |
There was a problem hiding this comment.
The game object is typed as any. This weakens type safety and makes the code harder to maintain. Please define and use a specific type for the game objects returned by useGameTokens. The metagame-sdk/sql package might export a type for this, or you could define one locally.
| } else if (activeTab === 2) { | ||
| // Following tab: filter to show only games from followed players | ||
| const filteredGames = games.filter((g: any) => { | ||
| const normalizedOwner = addAddressPadding(g.owner).toLowerCase(); | ||
| return followedAddresses.includes(normalizedOwner); | ||
| }); | ||
| setDisplayedGames(filteredGames); | ||
| } |
There was a problem hiding this comment.
For the "Following" tab, all games are fetched and then filtered on the client-side. This can be inefficient if the total number of games is large. The useFollowedPlayerNotifications hook demonstrates building a direct SQL query with WHERE owner IN (...) to fetch data only for specific players. A similar approach could be used here for better performance, for example by creating a new specialized hook that fetches only the games for followed players, or by extending useGameTokens to support filtering by multiple owners if possible.
| <Typography sx={{ textAlign: "center", py: 2 }}> | ||
| {activeTab === 1 ? "You have no games yet." : activeTab === 2 ? "No games from followed players." : "No games found."} | ||
| </Typography> | ||
| ) : displayedGames.map((game: any, index: number) => ( |
There was a problem hiding this comment.
The game object is typed as any. This weakens type safety and makes the code harder to maintain. Please define and use a specific type for the game objects returned by useGameTokens. The metagame-sdk/sql package might export a type for this, or you could define one locally.
|
Claude finished @loothero's task —— View job Code Review: Follow Player FeatureComprehensive review of the follow player feature implementation with in-app notifications. Todo List
Review SummaryThis PR implements a well-architected follow player feature with in-app notifications. The code follows established patterns and integrates cleanly with the existing codebase. However, there are several critical security and performance issues that need attention. Critical Issues
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dad90fcc88
ℹ️ 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".
| const newGames = data.filter((game) => { | ||
| const mintedAt = new Date(game.minted_at).getTime(); | ||
| return mintedAt > lastSeenTimestampRef.current; |
There was a problem hiding this comment.
The Torii SQL tables return numeric fields as raw felt strings (often hex) and token metadata timestamps are seconds-based (see getTokenMetadata in client/src/api/starknet.ts, which does parseInt(..., 16) * 1000). Here new Date(game.minted_at) will produce NaN for hex strings or a 1970-era timestamp for seconds strings, so mintedAt > lastSeenTimestampRef.current will never be true and lastSeenTimestampRef can become NaN, effectively disabling notifications for all followed players. Parse the timestamp (e.g. parseInt(game.minted_at, 16) * 1000 or detect numeric seconds) before comparing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR implements Phase 1 of a follow player feature that allows users to follow other players and receive in-app toast notifications when followed players start new games. The implementation uses localStorage for persistence and Torii SQL polling for notifications.
Changes:
- Added Zustand store with localStorage persistence for managing follow relationships
- Created reusable FollowButton and FollowingList components for follow interactions
- Implemented polling-based notification hook that queries Torii SQL every 30 seconds
- Added "Following" tab to desktop and mobile leaderboards showing games from followed players
- Added player name editing functionality to leaderboards with inline editing UI
- Updated documentation files (AGENTS.md, CLAUDE.md, GEMINI.md) with comprehensive project guidance
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
client/src/stores/followStore.ts |
Zustand store managing follow relationships with localStorage persistence |
client/src/components/FollowButton.tsx |
Heart icon button component for follow/unfollow actions with self-follow prevention |
client/src/components/FollowingList.tsx |
Component displaying list of followed players (created but not yet integrated) |
client/src/hooks/useFollowedPlayerNotifications.tsx |
Polling hook that checks Torii SQL for new games from followed players |
client/src/desktop/components/Leaderboard.tsx |
Added Following tab, follow buttons, and inline name editing |
client/src/mobile/components/Leaderboard.tsx |
Added Following tab, follow buttons, and inline name editing |
client/src/dojo/useSystemCalls.ts |
Added updatePlayerName function for updating game token names |
client/src/App.tsx |
Initialized notification subscription hook at app level |
client/.env.production |
Added DENSHOKAN_ADDRESS environment variable |
client/.env.local |
Added DENSHOKAN_ADDRESS environment variable |
contracts/AGENTS.md |
Comprehensive Cairo smart contract documentation |
client/AGENTS.md |
Comprehensive React frontend documentation |
AGENTS.md |
Root-level project overview documentation |
GEMINI.md, CLAUDE.md, contracts/GEMINI.md, contracts/CLAUDE.md, client/GEMINI.md, client/CLAUDE.md |
Documentation reference files |
Comments suppressed due to low confidence (1)
client/src/dojo/useSystemCalls.ts:489
- The validation only checks if name length is greater than 31, but doesn't check if the name is empty after trimming. An empty string will pass this validation and be sent to the contract, which may fail or behave unexpectedly.
Add a check for empty names after trimming: if (!name || !name.trim() || name.length > 31)
await executeAction([{
contractAddress: DUNGEON_ADDRESS,
entrypoint: "claim_jackpot",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!response.ok) { | ||
| console.error('Failed to fetch new games:', response.statusText); | ||
| return; | ||
| } |
There was a problem hiding this comment.
When the fetch request fails (response.ok is false), the function only logs an error and returns early, but isPollingRef.current remains true. This prevents any subsequent polling attempts because the next call to checkForNewGames will immediately return due to the isPollingRef check.
The isPollingRef.current should be set to false when returning early on error, or the error handling should be moved inside the try-catch block so the finally clause handles cleanup.
| // Build SQL query to check for new games from followed players | ||
| const ownersList = followedAddresses | ||
| .map((addr) => `"${addAddressPadding(addr)}"`) | ||
| .join(','); | ||
|
|
||
| // Query for recently minted games by followed players | ||
| const url = `${currentNetworkConfig.toriiUrl}/sql?query= | ||
| SELECT token_id, owner, player_name, minted_at | ||
| FROM "relayer_0_0_1-TokenMetadataUpdate" | ||
| WHERE owner IN (${ownersList}) | ||
| ORDER BY minted_at DESC | ||
| LIMIT 10`; |
There was a problem hiding this comment.
The SQL query is vulnerable to injection if address values contain double quotes or other special characters. Although addAddressPadding comes from the starknet library, the addresses stored in followedPlayers come from user input through the followPlayer function. An attacker could potentially manipulate addresses stored in localStorage to inject SQL.
The addresses should be properly escaped or validated before being included in the SQL query, or use a parameterized query approach if the API supports it.
| @@ -0,0 +1 @@ | |||
| AGENTS.md | |||
There was a problem hiding this comment.
The reference to AGENTS.md is missing the @ symbol prefix that is used consistently in other similar files. For example, contracts/GEMINI.md and CLAUDE.md use "@AGENTS.md" while this file uses "AGENTS.md".
| AGENTS.md | |
| @AGENTS.md |
| const handleTabChange = useCallback((event: React.SyntheticEvent, newValue: number) => { | ||
| setActiveTab(newValue); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
When switching between tabs, the pagination state (currentPage) is not reset to 0. This can lead to a confusing user experience where switching to the "Following" or "My Games" tab shows an empty page because the user was on page 5 of "All" tab, but "Following" only has 1 page of results.
Consider resetting the page to 0 when activeTab changes, or manage separate pagination state for each tab.
| }, []); | |
| goToPage(0); | |
| }, [goToPage]); |
| playerName={localNameOverrides[game.token_id] ?? game.player_name ?? `Player #${game.token_id}`} | ||
| /> |
There was a problem hiding this comment.
When a player follows someone, the current displayed name (which could be from localNameOverrides) is stored in the followStore. However, if the player later edits their own name, the followed player's stored name in followStore is not updated. This creates an inconsistency where notifications and the FollowingList might show an outdated name.
Consider storing only the address in followStore and always fetching the current name from the games data, or implement a mechanism to update followed player names when they change.
| playerName={localNameOverrides[game.token_id] ?? game.player_name ?? `Player #${game.token_id}`} | |
| /> | |
| /> | |
| followPlayer: (address: string, name: string) => { | ||
| const normalizedAddress = address.toLowerCase(); | ||
| set((state) => ({ | ||
| followedPlayers: { | ||
| ...state.followedPlayers, | ||
| [normalizedAddress]: { | ||
| address: normalizedAddress, | ||
| name, | ||
| followedAt: Date.now(), | ||
| }, | ||
| }, | ||
| })); | ||
| }, |
There was a problem hiding this comment.
The followPlayer function does not validate the address or name parameters. Malicious input in the name parameter could lead to XSS vulnerabilities when the name is displayed in the UI, or storage issues. The address parameter should also be validated to ensure it's a valid Starknet address format before normalization.
Add input validation for both address format and name sanitization to prevent security issues.
| } else if (activeTab === 2) { | ||
| // Following tab: filter to show only games from followed players | ||
| const filteredGames = games.filter((g: any) => { | ||
| const normalizedOwner = addAddressPadding(g.owner).toLowerCase(); | ||
| return followedAddresses.includes(normalizedOwner); | ||
| }); | ||
| setDisplayedGames(filteredGames); |
There was a problem hiding this comment.
The "Following" tab loads all games and then filters them client-side. If there are many games in the database, this approach is inefficient and could cause performance issues. Additionally, it doesn't respect pagination properly - the filtered results might have fewer than 10 items even though more followed players' games exist on other pages.
Consider modifying the useGameTokens call to support filtering by multiple owners at the API level, or implement proper pagination for filtered results.
| // Following tab: filter to show only games from followed players | ||
| const filteredGames = games.filter((g: any) => { | ||
| const normalizedOwner = addAddressPadding(g.owner).toLowerCase(); | ||
| return followedAddresses.includes(normalizedOwner); | ||
| }); | ||
| setDisplayedGames(filteredGames); | ||
| } |
There was a problem hiding this comment.
The "Following" tab loads all games and then filters them client-side. If there are many games in the database, this approach is inefficient and could cause performance issues. Additionally, it doesn't respect pagination properly - the filtered results might have fewer than 10 items even though more followed players' games exist on other pages.
Consider modifying the useGameTokens call to support filtering by multiple owners at the API level, or implement proper pagination for filtered results.
| const handleTabChange = useCallback((event: React.SyntheticEvent, newValue: number) => { | ||
| setActiveTab(newValue); | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
When switching between tabs, the pagination state (currentPage) is not reset to 0. This can lead to a confusing user experience where switching to the "Following" or "My Games" tab shows an empty page because the user was on page 5 of "All" tab, but "Following" only has 1 page of results.
Consider resetting the page to 0 when activeTab changes, or manage separate pagination state for each tab.
| }, []); | |
| // Reset pagination to the first page when switching tabs to avoid empty pages on shorter lists | |
| goToPage(0); | |
| }, [goToPage]); |
| playerName={localNameOverrides[game.token_id] ?? game.player_name ?? `Player #${game.token_id}`} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
When a player follows someone, the current displayed name (which could be from localNameOverrides) is stored in the followStore. However, if the player later edits their own name, the followed player's stored name in followStore is not updated. This creates an inconsistency where notifications and the FollowingList might show an outdated name.
Consider storing only the address in followStore and always fetching the current name from the games data, or implement a mechanism to update followed player names when they change.
| playerName={localNameOverrides[game.token_id] ?? game.player_name ?? `Player #${game.token_id}`} | |
| /> | |
| )} | |
| /> | |
| )} | |
| )} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Around line 7-12: The fenced code blocks in AGENTS.md lack language
identifiers causing linter MD040 failures; update the repository tree block to
use a plain text language (e.g., add "text" after the opening ```) and ensure
all shell/command blocks (e.g., the commands in the client and contracts
sections shown in the diff) use "bash" after the opening ```; apply the same
change to the other affected fenced blocks referenced (lines 25-33) so every
triple-backtick fence has an appropriate language identifier.
In `@client/.env.local`:
- Around line 1-2: Reorder the environment variable entries so they follow
dotenv-linter's expected alphabetical order: place VITE_PUBLIC_DENSHOKAN_ADDRESS
before VITE_PUBLIC_VRF_PROVIDER_ADDRESS; update the .env.local file by swapping
the two lines referencing VITE_PUBLIC_DENSHOKAN_ADDRESS and
VITE_PUBLIC_VRF_PROVIDER_ADDRESS to eliminate the ordering warning.
In `@client/.env.production`:
- Around line 1-2: Reorder the two environment variables so
VITE_PUBLIC_DENSHOKAN_ADDRESS appears before VITE_PUBLIC_VRF_PROVIDER_ADDRESS to
satisfy dotenv-linter; locate the entries for VITE_PUBLIC_DENSHOKAN_ADDRESS and
VITE_PUBLIC_VRF_PROVIDER_ADDRESS in the .env production content and swap their
order so the DENSHOKAN key is listed first.
In `@client/AGENTS.md`:
- Around line 15-57: Update the markdown code fence that contains the project
directory tree (the block starting with "src/" and the tree lines ending with
"└── abi/") to include a language identifier (e.g., add "text" after the opening
```), so the fence becomes ```text and satisfies markdownlint MD040.
In `@client/src/desktop/components/Leaderboard.tsx`:
- Around line 112-138: Remove the stale-reference guard that prevents
re-filtering when tab or follows change: in the useEffect that currently checks
"if (!loading && games && games !== prevGamesRef.current) { ... }", stop
comparing games to prevGamesRef.current so the effect runs whenever its
dependencies change (loading, games, activeTab, address, followedAddresses);
keep the existing ownership check for the "My Games" branch and the filtering
logic for the "Following" branch (which uses addAddressPadding,
followedAddresses and setDisplayedGames) intact, and remove or stop updating
prevGamesRef.current if it’s no longer needed.
In `@client/src/hooks/useFollowedPlayerNotifications.tsx`:
- Around line 121-145: The effect that starts polling (useEffect) early-returns
when getFollowedAddresses() is empty and thus never restarts if the user follows
someone later; update the dependency array to include followedPlayers (the
reactive list) so the effect will rerun and set up polling when follows change,
and move the lastSeenTimestampRef.current = Date.now() initialization into a
separate useEffect that runs only on mount (so following someone doesn’t reset
the timestamp). Ensure checkForNewGames, getFollowedAddresses, POLL_INTERVAL and
lastSeenTimestampRef usage remain the same inside the polling effect.
In `@client/src/mobile/components/Leaderboard.tsx`:
- Around line 109-135: Remove the stale-reference guard (the check comparing
games !== prevGamesRef.current) from the useEffect so the effect runs when
dependencies like activeTab or followedAddresses change; keep the existing
ownership validation for the "My Games" branch (using prevGamesRef.current and
addAddressPadding/address) if you still want to prevent race conditions there,
but ensure branches for activeTab 0 (All), 1 (My Games using ownership check and
setDisplayedGames), and 2 (Following: filter by followedAddresses and call
setDisplayedGames) execute whenever loading, games, activeTab, address, or
followedAddresses change.
In `@contracts/AGENTS.md`:
- Around line 22-51: The markdown code fence containing the project tree (the
block starting with ``` and the line "src/") needs a language identifier to
satisfy MD040; update the opening fence from ``` to ```text (or another
appropriate language) so it becomes ```text and leave the rest of the block
unchanged.
| ``` | ||
| death-mountain/ | ||
| ├── client/ # React + TypeScript frontend (Vite) | ||
| ├── contracts/ # Cairo smart contracts (Dojo 1.6.0) | ||
| └── .github/ # CI workflows | ||
| ``` |
There was a problem hiding this comment.
Add language identifiers to fenced code blocks.
Markdownlint (MD040) requires specifying a language for fenced blocks; otherwise lint will fail. Consider adding text for the tree and bash for commands.
🧹 Suggested fix
-```
+```text
death-mountain/
├── client/ # React + TypeScript frontend (Vite)
├── contracts/ # Cairo smart contracts (Dojo 1.6.0)
└── .github/ # CI workflows@@
-bash +bash
cd client && pnpm install && pnpm dev # Port 5173
@@
-```bash
+```bash
cd contracts && sozo build && sozo test
</details>
Also applies to: 25-33
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @AGENTS.md around lines 7 - 12, The fenced code blocks in AGENTS.md lack
language identifiers causing linter MD040 failures; update the repository tree
block to use a plain text language (e.g., add "text" after the opening ) and ensure all shell/command blocks (e.g., the commands in the client and contracts sections shown in the diff) use "bash" after the opening ; apply the same
change to the other affected fenced blocks referenced (lines 25-33) so every
triple-backtick fence has an appropriate language identifier.
</details>
<!-- fingerprinting:phantom:triton:eagle -->
<!-- This is an auto-generated comment by CodeRabbit -->
| VITE_PUBLIC_VRF_PROVIDER_ADDRESS=0x051fea4450da9d6aee758bdeba88b2f665bcbf549d2c61421aa724e9ac0ced8f | ||
| VITE_PUBLIC_DENSHOKAN_ADDRESS=0x036017e69d21d6d8c13e266eabb73ef1f1d02722d86bdcabe5f168f8e549d3cd |
There was a problem hiding this comment.
Reorder env keys to satisfy dotenv-linter.
This file will also trigger the ordering warning; reordering avoids lint noise.
♻️ Suggested reorder
-VITE_PUBLIC_VRF_PROVIDER_ADDRESS=0x051fea4450da9d6aee758bdeba88b2f665bcbf549d2c61421aa724e9ac0ced8f
VITE_PUBLIC_DENSHOKAN_ADDRESS=0x036017e69d21d6d8c13e266eabb73ef1f1d02722d86bdcabe5f168f8e549d3cd
+VITE_PUBLIC_VRF_PROVIDER_ADDRESS=0x051fea4450da9d6aee758bdeba88b2f665bcbf549d2c61421aa724e9ac0ced8f📝 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_VRF_PROVIDER_ADDRESS=0x051fea4450da9d6aee758bdeba88b2f665bcbf549d2c61421aa724e9ac0ced8f | |
| VITE_PUBLIC_DENSHOKAN_ADDRESS=0x036017e69d21d6d8c13e266eabb73ef1f1d02722d86bdcabe5f168f8e549d3cd | |
| VITE_PUBLIC_DENSHOKAN_ADDRESS=0x036017e69d21d6d8c13e266eabb73ef1f1d02722d86bdcabe5f168f8e549d3cd | |
| VITE_PUBLIC_VRF_PROVIDER_ADDRESS=0x051fea4450da9d6aee758bdeba88b2f665bcbf549d2c61421aa724e9ac0ced8f |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 2-2: [UnorderedKey] The VITE_PUBLIC_DENSHOKAN_ADDRESS key should go before the VITE_PUBLIC_VRF_PROVIDER_ADDRESS key
(UnorderedKey)
🤖 Prompt for AI Agents
In `@client/.env.local` around lines 1 - 2, Reorder the environment variable
entries so they follow dotenv-linter's expected alphabetical order: place
VITE_PUBLIC_DENSHOKAN_ADDRESS before VITE_PUBLIC_VRF_PROVIDER_ADDRESS; update
the .env.local file by swapping the two lines referencing
VITE_PUBLIC_DENSHOKAN_ADDRESS and VITE_PUBLIC_VRF_PROVIDER_ADDRESS to eliminate
the ordering warning.
| VITE_PUBLIC_VRF_PROVIDER_ADDRESS=0x051fea4450da9d6aee758bdeba88b2f665bcbf549d2c61421aa724e9ac0ced8f | ||
| VITE_PUBLIC_DENSHOKAN_ADDRESS=0x036017e69d21d6d8c13e266eabb73ef1f1d02722d86bdcabe5f168f8e549d3cd |
There was a problem hiding this comment.
Reorder env keys to satisfy dotenv-linter.
The linter warning indicates the new key should be placed before the VRF provider key. Consider reordering to keep lint output clean.
♻️ Suggested reorder
-VITE_PUBLIC_VRF_PROVIDER_ADDRESS=0x051fea4450da9d6aee758bdeba88b2f665bcbf549d2c61421aa724e9ac0ced8f
VITE_PUBLIC_DENSHOKAN_ADDRESS=0x036017e69d21d6d8c13e266eabb73ef1f1d02722d86bdcabe5f168f8e549d3cd
+VITE_PUBLIC_VRF_PROVIDER_ADDRESS=0x051fea4450da9d6aee758bdeba88b2f665bcbf549d2c61421aa724e9ac0ced8f📝 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_VRF_PROVIDER_ADDRESS=0x051fea4450da9d6aee758bdeba88b2f665bcbf549d2c61421aa724e9ac0ced8f | |
| VITE_PUBLIC_DENSHOKAN_ADDRESS=0x036017e69d21d6d8c13e266eabb73ef1f1d02722d86bdcabe5f168f8e549d3cd | |
| VITE_PUBLIC_DENSHOKAN_ADDRESS=0x036017e69d21d6d8c13e266eabb73ef1f1d02722d86bdcabe5f168f8e549d3cd | |
| VITE_PUBLIC_VRF_PROVIDER_ADDRESS=0x051fea4450da9d6aee758bdeba88b2f665bcbf549d2c61421aa724e9ac0ced8f |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 2-2: [UnorderedKey] The VITE_PUBLIC_DENSHOKAN_ADDRESS key should go before the VITE_PUBLIC_VRF_PROVIDER_ADDRESS key
(UnorderedKey)
🤖 Prompt for AI Agents
In `@client/.env.production` around lines 1 - 2, Reorder the two environment
variables so VITE_PUBLIC_DENSHOKAN_ADDRESS appears before
VITE_PUBLIC_VRF_PROVIDER_ADDRESS to satisfy dotenv-linter; locate the entries
for VITE_PUBLIC_DENSHOKAN_ADDRESS and VITE_PUBLIC_VRF_PROVIDER_ADDRESS in the
.env production content and swap their order so the DENSHOKAN key is listed
first.
| ``` | ||
| src/ | ||
| ├── App.tsx # Root component, routing setup | ||
| ├── Main.tsx # Entry point with providers | ||
| ├── stores/ # Zustand state management | ||
| │ ├── gameStore.ts # Adventurer, beast, bag, events, market | ||
| │ ├── marketStore.ts # Item purchases, pricing state | ||
| │ └── uiStore.ts # UI toggles, overlays | ||
| ├── dojo/ # Blockchain integration hooks | ||
| │ ├── useSystemCalls.ts # Contract call wrappers (explore, attack, etc.) | ||
| │ ├── useGameTokens.ts # Token ownership, minting | ||
| │ ├── useGameSettings.ts# Dungeon configuration | ||
| │ ├── useGameEvents.ts # Event subscription | ||
| │ └── useDungeon.ts # Dungeon state | ||
| ├── desktop/ # Desktop-optimized UI | ||
| │ ├── pages/ # Full page components | ||
| │ ├── overlays/ # Modal overlays (Combat, Explore, Market, Inventory) | ||
| │ ├── components/ # Desktop-specific components | ||
| │ └── contexts/ # Desktop context providers | ||
| ├── mobile/ # Mobile-optimized UI | ||
| │ ├── pages/ # Mobile page layouts | ||
| │ ├── containers/ # Screen containers (BeastScreen, etc.) | ||
| │ ├── components/ # Mobile-specific components | ||
| │ └── contexts/ # Mobile context providers | ||
| ├── components/ # Shared UI components | ||
| ├── contexts/ # Global context providers | ||
| │ ├── controller.tsx # Cartridge Controller wallet | ||
| │ └── starknet.tsx # Starknet connection | ||
| ├── types/ # TypeScript interfaces | ||
| │ └── game.ts # Adventurer, Beast, Item, Stats types | ||
| ├── utils/ # Utility functions | ||
| │ ├── events.ts # GameEvent processing | ||
| │ ├── translation.ts # Contract event → UI event mapping | ||
| │ ├── loot.ts # Item utilities (slots, types, boosts) | ||
| │ └── game.ts # Game logic helpers | ||
| ├── constants/ # Static game data | ||
| │ ├── beast.ts # Beast names, prefixes, suffixes | ||
| │ ├── loot.ts # Item definitions | ||
| │ └── obstacles.ts # Obstacle definitions | ||
| ├── api/ # External API integrations | ||
| ├── generated/ # Auto-generated Dojo bindings | ||
| └── abi/ # Contract ABIs | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to the project structure code fence.
This block will fail markdownlint MD040 without a language.
🧹 Suggested fix
-```
+```text
src/
├── App.tsx # Root component, routing setup
├── Main.tsx # Entry point with providers
...
└── abi/ # Contract ABIs</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@client/AGENTS.md` around lines 15 - 57, Update the markdown code fence that
contains the project directory tree (the block starting with "src/" and the tree
lines ending with "└── abi/") to include a language identifier (e.g., add "text"
after the opening ```), so the fence becomes ```text and satisfies markdownlint
MD040.
| // Validate incoming games data to prevent race condition when switching tabs | ||
| useEffect(() => { | ||
| if (!loading && games && games !== prevGamesRef.current) { | ||
| prevGamesRef.current = games; | ||
|
|
||
| if (activeTab === 1 && address) { | ||
| // My Games tab: only accept if all games are owned by user | ||
| const allOwned = games.length === 0 || games.every((g: any) => | ||
| addAddressPadding(g.owner).toLowerCase() === addAddressPadding(address).toLowerCase() | ||
| ); | ||
| if (allOwned) { | ||
| setDisplayedGames(games); | ||
| } | ||
| // If not all owned, this is stale "All" data - ignore it | ||
| } else if (activeTab === 0) { | ||
| // All tab: accept any data | ||
| setDisplayedGames(games); | ||
| } else if (activeTab === 2) { | ||
| // Following tab: filter to show only games from followed players | ||
| const filteredGames = games.filter((g: any) => { | ||
| const normalizedOwner = addAddressPadding(g.owner).toLowerCase(); | ||
| return followedAddresses.includes(normalizedOwner); | ||
| }); | ||
| setDisplayedGames(filteredGames); | ||
| } | ||
| } | ||
| }, [loading, games, activeTab, address, followedAddresses]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "Leaderboard.tsx"Repository: Provable-Games/death-mountain
Length of output: 163
🏁 Script executed:
wc -l client/src/desktop/components/Leaderboard.tsxRepository: Provable-Games/death-mountain
Length of output: 122
🏁 Script executed:
cat -n client/src/desktop/components/Leaderboard.tsx | sed -n '100,160p'Repository: Provable-Games/death-mountain
Length of output: 2691
🏁 Script executed:
rg -n "prevGamesRef" client/src/desktop/components/Leaderboard.tsxRepository: Provable-Games/death-mountain
Length of output: 224
🏁 Script executed:
cat -n client/src/desktop/components/Leaderboard.tsx | sed -n '1,120p' | grep -A 5 -B 5 "prevGamesRef"Repository: Provable-Games/death-mountain
Length of output: 1314
🏁 Script executed:
cat -n client/src/desktop/components/Leaderboard.tsx | sed -n '25,60p'Repository: Provable-Games/death-mountain
Length of output: 1741
🏁 Script executed:
rg -n "setDisplayedGames" client/src/desktop/components/Leaderboard.tsxRepository: Provable-Games/death-mountain
Length of output: 266
Following tab fails to refresh when switching tabs or when the follow list changes without a new games fetch.
The guard games !== prevGamesRef.current blocks the effect body from executing when activeTab or followedAddresses changes but the games array reference remains the same. This prevents the Following tab from filtering games when switching to it or when following/unfollowing players while the tab is open.
The suggested fix of removing the ref guard is sound. The "My Games" tab's internal ownership check already protects against race conditions, and the "All" and "Following" tabs have no validation that could be harmed by running on every dependency change.
🔧 Suggested fix (remove the ref guard)
- if (!loading && games && games !== prevGamesRef.current) {
- prevGamesRef.current = games;
+ if (!loading && games) {
+ prevGamesRef.current = games;📝 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.
| // Validate incoming games data to prevent race condition when switching tabs | |
| useEffect(() => { | |
| if (!loading && games && games !== prevGamesRef.current) { | |
| prevGamesRef.current = games; | |
| if (activeTab === 1 && address) { | |
| // My Games tab: only accept if all games are owned by user | |
| const allOwned = games.length === 0 || games.every((g: any) => | |
| addAddressPadding(g.owner).toLowerCase() === addAddressPadding(address).toLowerCase() | |
| ); | |
| if (allOwned) { | |
| setDisplayedGames(games); | |
| } | |
| // If not all owned, this is stale "All" data - ignore it | |
| } else if (activeTab === 0) { | |
| // All tab: accept any data | |
| setDisplayedGames(games); | |
| } else if (activeTab === 2) { | |
| // Following tab: filter to show only games from followed players | |
| const filteredGames = games.filter((g: any) => { | |
| const normalizedOwner = addAddressPadding(g.owner).toLowerCase(); | |
| return followedAddresses.includes(normalizedOwner); | |
| }); | |
| setDisplayedGames(filteredGames); | |
| } | |
| } | |
| }, [loading, games, activeTab, address, followedAddresses]); | |
| // Validate incoming games data to prevent race condition when switching tabs | |
| useEffect(() => { | |
| if (!loading && games) { | |
| prevGamesRef.current = games; | |
| if (activeTab === 1 && address) { | |
| // My Games tab: only accept if all games are owned by user | |
| const allOwned = games.length === 0 || games.every((g: any) => | |
| addAddressPadding(g.owner).toLowerCase() === addAddressPadding(address).toLowerCase() | |
| ); | |
| if (allOwned) { | |
| setDisplayedGames(games); | |
| } | |
| // If not all owned, this is stale "All" data - ignore it | |
| } else if (activeTab === 0) { | |
| // All tab: accept any data | |
| setDisplayedGames(games); | |
| } else if (activeTab === 2) { | |
| // Following tab: filter to show only games from followed players | |
| const filteredGames = games.filter((g: any) => { | |
| const normalizedOwner = addAddressPadding(g.owner).toLowerCase(); | |
| return followedAddresses.includes(normalizedOwner); | |
| }); | |
| setDisplayedGames(filteredGames); | |
| } | |
| } | |
| }, [loading, games, activeTab, address, followedAddresses]); |
🤖 Prompt for AI Agents
In `@client/src/desktop/components/Leaderboard.tsx` around lines 112 - 138, Remove
the stale-reference guard that prevents re-filtering when tab or follows change:
in the useEffect that currently checks "if (!loading && games && games !==
prevGamesRef.current) { ... }", stop comparing games to prevGamesRef.current so
the effect runs whenever its dependencies change (loading, games, activeTab,
address, followedAddresses); keep the existing ownership check for the "My
Games" branch and the filtering logic for the "Following" branch (which uses
addAddressPadding, followedAddresses and setDisplayedGames) intact, and remove
or stop updating prevGamesRef.current if it’s no longer needed.
| useEffect(() => { | ||
| // Reset timestamp when component mounts to avoid stale notifications | ||
| lastSeenTimestampRef.current = Date.now(); | ||
|
|
||
| // Only start polling if user is logged in and following someone | ||
| const followedAddresses = getFollowedAddresses(); | ||
| if (!address || followedAddresses.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| // Initial check after a short delay | ||
| const initialTimeout = setTimeout(() => { | ||
| checkForNewGames(); | ||
| }, 5000); | ||
|
|
||
| // Set up polling interval | ||
| const pollInterval = setInterval(() => { | ||
| checkForNewGames(); | ||
| }, POLL_INTERVAL); | ||
|
|
||
| return () => { | ||
| clearTimeout(initialTimeout); | ||
| clearInterval(pollInterval); | ||
| }; | ||
| }, [address, checkForNewGames, getFollowedAddresses]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n client/src/hooks/useFollowedPlayerNotifications.tsx | head -150Repository: Provable-Games/death-mountain
Length of output: 5774
🏁 Script executed:
fd -t f "followStore" client/src --type fileRepository: Provable-Games/death-mountain
Length of output: 105
🏁 Script executed:
cat -n client/src/stores/followStore.tsRepository: Provable-Games/death-mountain
Length of output: 2497
Polling won't start if the user follows someone after mount.
The effect doesn't depend on followedPlayers, only on the stable getFollowedAddresses function reference. It runs once and returns early when there are no followed players. After a user follows someone, the polling interval is never recreated unless they reload. Add followedPlayers to the dependency array, and consider separating timestamp initialization into its own effect to avoid resetting on every follow change.
✅ Suggested fix
+ useEffect(() => {
+ // Initialize once
+ lastSeenTimestampRef.current = Date.now();
+ }, []);
+
useEffect(() => {
- // Reset timestamp when component mounts to avoid stale notifications
- lastSeenTimestampRef.current = Date.now();
-
// Only start polling if user is logged in and following someone
const followedAddresses = getFollowedAddresses();
if (!address || followedAddresses.length === 0) {
return;
}
@@
- }, [address, checkForNewGames, getFollowedAddresses]);
+ }, [address, checkForNewGames, getFollowedAddresses, followedPlayers]);📝 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.
| useEffect(() => { | |
| // Reset timestamp when component mounts to avoid stale notifications | |
| lastSeenTimestampRef.current = Date.now(); | |
| // Only start polling if user is logged in and following someone | |
| const followedAddresses = getFollowedAddresses(); | |
| if (!address || followedAddresses.length === 0) { | |
| return; | |
| } | |
| // Initial check after a short delay | |
| const initialTimeout = setTimeout(() => { | |
| checkForNewGames(); | |
| }, 5000); | |
| // Set up polling interval | |
| const pollInterval = setInterval(() => { | |
| checkForNewGames(); | |
| }, POLL_INTERVAL); | |
| return () => { | |
| clearTimeout(initialTimeout); | |
| clearInterval(pollInterval); | |
| }; | |
| }, [address, checkForNewGames, getFollowedAddresses]); | |
| useEffect(() => { | |
| // Initialize once | |
| lastSeenTimestampRef.current = Date.now(); | |
| }, []); | |
| useEffect(() => { | |
| // Only start polling if user is logged in and following someone | |
| const followedAddresses = getFollowedAddresses(); | |
| if (!address || followedAddresses.length === 0) { | |
| return; | |
| } | |
| // Initial check after a short delay | |
| const initialTimeout = setTimeout(() => { | |
| checkForNewGames(); | |
| }, 5000); | |
| // Set up polling interval | |
| const pollInterval = setInterval(() => { | |
| checkForNewGames(); | |
| }, POLL_INTERVAL); | |
| return () => { | |
| clearTimeout(initialTimeout); | |
| clearInterval(pollInterval); | |
| }; | |
| }, [address, checkForNewGames, getFollowedAddresses, followedPlayers]); |
🤖 Prompt for AI Agents
In `@client/src/hooks/useFollowedPlayerNotifications.tsx` around lines 121 - 145,
The effect that starts polling (useEffect) early-returns when
getFollowedAddresses() is empty and thus never restarts if the user follows
someone later; update the dependency array to include followedPlayers (the
reactive list) so the effect will rerun and set up polling when follows change,
and move the lastSeenTimestampRef.current = Date.now() initialization into a
separate useEffect that runs only on mount (so following someone doesn’t reset
the timestamp). Ensure checkForNewGames, getFollowedAddresses, POLL_INTERVAL and
lastSeenTimestampRef usage remain the same inside the polling effect.
| // Validate incoming games data to prevent race condition when switching tabs | ||
| useEffect(() => { | ||
| if (!loading && games && games !== prevGamesRef.current) { | ||
| prevGamesRef.current = games; | ||
|
|
||
| if (activeTab === 1 && address) { | ||
| // My Games tab: only accept if all games are owned by user | ||
| const allOwned = games.length === 0 || games.every((g: any) => | ||
| addAddressPadding(g.owner).toLowerCase() === addAddressPadding(address).toLowerCase() | ||
| ); | ||
| if (allOwned) { | ||
| setDisplayedGames(games); | ||
| } | ||
| // If not all owned, this is stale "All" data - ignore it | ||
| } else if (activeTab === 0) { | ||
| // All tab: accept any data | ||
| setDisplayedGames(games); | ||
| } else if (activeTab === 2) { | ||
| // Following tab: filter to show only games from followed players | ||
| const filteredGames = games.filter((g: any) => { | ||
| const normalizedOwner = addAddressPadding(g.owner).toLowerCase(); | ||
| return followedAddresses.includes(normalizedOwner); | ||
| }); | ||
| setDisplayedGames(filteredGames); | ||
| } | ||
| } | ||
| }, [loading, games, activeTab, address, followedAddresses]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's locate and read the file
fd -t f "Leaderboard.tsx" client/src/Repository: Provable-Games/death-mountain
Length of output: 163
🏁 Script executed:
# Check the file size first
wc -l client/src/mobile/components/Leaderboard.tsxRepository: Provable-Games/death-mountain
Length of output: 121
🏁 Script executed:
# Read the specific lines mentioned in the review (109-135)
sed -n '100,150p' client/src/mobile/components/Leaderboard.tsxRepository: Provable-Games/death-mountain
Length of output: 1894
🏁 Script executed:
# Get context around the useEffect to understand the overall logic
sed -n '80,160p' client/src/mobile/components/Leaderboard.tsxRepository: Provable-Games/death-mountain
Length of output: 2762
🏁 Script executed:
# Check the desktop Leaderboard component for comparison
sed -n '100,150p' client/src/desktop/components/Leaderboard.tsxRepository: Provable-Games/death-mountain
Length of output: 1938
🏁 Script executed:
# Get a broader context from desktop version to see if it has the same issue
sed -n '80,160p' client/src/desktop/components/Leaderboard.tsxRepository: Provable-Games/death-mountain
Length of output: 2763
Following tab may not refresh when the follow list or tab selection changes.
The games !== prevGamesRef.current guard prevents the effect from executing when activeTab or followedAddresses changes without a new games reference, leaving the Following tab displaying stale data until the next games fetch. The ownership validation in the "My Games" tab already protects against race conditions, making the guard unnecessary.
Remove the guard:
Suggested fix
- if (!loading && games && games !== prevGamesRef.current) {
- prevGamesRef.current = games;
+ if (!loading && games) {
+ prevGamesRef.current = games;📝 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.
| // Validate incoming games data to prevent race condition when switching tabs | |
| useEffect(() => { | |
| if (!loading && games && games !== prevGamesRef.current) { | |
| prevGamesRef.current = games; | |
| if (activeTab === 1 && address) { | |
| // My Games tab: only accept if all games are owned by user | |
| const allOwned = games.length === 0 || games.every((g: any) => | |
| addAddressPadding(g.owner).toLowerCase() === addAddressPadding(address).toLowerCase() | |
| ); | |
| if (allOwned) { | |
| setDisplayedGames(games); | |
| } | |
| // If not all owned, this is stale "All" data - ignore it | |
| } else if (activeTab === 0) { | |
| // All tab: accept any data | |
| setDisplayedGames(games); | |
| } else if (activeTab === 2) { | |
| // Following tab: filter to show only games from followed players | |
| const filteredGames = games.filter((g: any) => { | |
| const normalizedOwner = addAddressPadding(g.owner).toLowerCase(); | |
| return followedAddresses.includes(normalizedOwner); | |
| }); | |
| setDisplayedGames(filteredGames); | |
| } | |
| } | |
| }, [loading, games, activeTab, address, followedAddresses]); | |
| // Validate incoming games data to prevent race condition when switching tabs | |
| useEffect(() => { | |
| if (!loading && games) { | |
| prevGamesRef.current = games; | |
| if (activeTab === 1 && address) { | |
| // My Games tab: only accept if all games are owned by user | |
| const allOwned = games.length === 0 || games.every((g: any) => | |
| addAddressPadding(g.owner).toLowerCase() === addAddressPadding(address).toLowerCase() | |
| ); | |
| if (allOwned) { | |
| setDisplayedGames(games); | |
| } | |
| // If not all owned, this is stale "All" data - ignore it | |
| } else if (activeTab === 0) { | |
| // All tab: accept any data | |
| setDisplayedGames(games); | |
| } else if (activeTab === 2) { | |
| // Following tab: filter to show only games from followed players | |
| const filteredGames = games.filter((g: any) => { | |
| const normalizedOwner = addAddressPadding(g.owner).toLowerCase(); | |
| return followedAddresses.includes(normalizedOwner); | |
| }); | |
| setDisplayedGames(filteredGames); | |
| } | |
| } | |
| }, [loading, games, activeTab, address, followedAddresses]); |
🤖 Prompt for AI Agents
In `@client/src/mobile/components/Leaderboard.tsx` around lines 109 - 135, Remove
the stale-reference guard (the check comparing games !== prevGamesRef.current)
from the useEffect so the effect runs when dependencies like activeTab or
followedAddresses change; keep the existing ownership validation for the "My
Games" branch (using prevGamesRef.current and addAddressPadding/address) if you
still want to prevent race conditions there, but ensure branches for activeTab 0
(All), 1 (My Games using ownership check and setDisplayedGames), and 2
(Following: filter by followedAddresses and call setDisplayedGames) execute
whenever loading, games, activeTab, address, or followedAddresses change.
| ``` | ||
| src/ | ||
| ├── lib.cairo # Module declarations | ||
| ├── systems/ # Dojo contract implementations | ||
| │ ├── game/ # Core game loop (explore, attack, flee, buy, equip) | ||
| │ ├── adventurer/ # Character management, stat calculations | ||
| │ ├── beast/ # Enemy generation, combat logic | ||
| │ ├── loot/ # Item type/tier resolution | ||
| │ ├── settings/ # Game configuration | ||
| │ ├── renderer/ # On-chain SVG NFT metadata | ||
| │ ├── game_token/ # Token contract integration | ||
| │ └── objectives/ # Dungeon objectives | ||
| ├── models/ # Dojo models (on-chain state) | ||
| │ ├── adventurer/ # Adventurer, Bag, Equipment, Item, Stats | ||
| │ ├── game.cairo # GameSettings, GameState, GameEvent | ||
| │ ├── beast.cairo # Beast struct and combat specs | ||
| │ ├── combat.cairo # CombatSpec, damage calculations | ||
| │ ├── loot.cairo # Item definitions (101 base items) | ||
| │ ├── market.cairo # Market state, ItemPurchase | ||
| │ └── obstacle.cairo # Obstacle types (75 unique) | ||
| ├── constants/ # Game constants and enums | ||
| ├── libs/ # Shared utilities | ||
| │ ├── game.cairo # GameLibs dispatcher factory | ||
| │ └── settings.cairo # Settings helpers | ||
| └── utils/ # Utility functions | ||
| ├── loot.cairo # Item type/tier lookup | ||
| ├── vrf.cairo # VRF randomness integration | ||
| ├── renderer/ # SVG generation utilities | ||
| └── string/ # String manipulation | ||
| ``` |
There was a problem hiding this comment.
Add language identifier to the project structure code fence.
This block will trigger markdownlint MD040 unless a language is specified.
🧹 Suggested fix
-```
+```text
src/
├── lib.cairo # Module declarations
├── systems/ # Dojo contract implementations
...
└── utils/ # Utility functions
├── loot.cairo # Item type/tier lookup
├── vrf.cairo # VRF randomness integration
├── renderer/ # SVG generation utilities
└── string/ # String manipulation</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.18.1)</summary>
22-22: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
In @contracts/AGENTS.md around lines 22 - 51, The markdown code fence containing
the project tree (the block starting with and the line "src/") needs a language identifier to satisfy MD040; update the opening fence from to
text (or another appropriate language) so it becomes text and leave the
rest of the block unchanged.
</details>
<!-- fingerprinting:phantom:triton:eagle -->
<!-- This is an auto-generated comment by CodeRabbit -->
- Add PlayerSearch component with name search functionality - Search queries Torii SQL for players matching the input - Shows search results with follow button for each player - Displays "Already following" indicator for followed players - Component appears in "Following" tab on both desktop and mobile Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @loothero's task —— View job Code Review: Follow Player FeatureComprehensive review of the follow player feature implementation with in-app notifications. Todo List
Review SummaryThis PR implements a well-architected follow player feature with in-app notifications. The code follows established patterns and integrates cleanly with the existing codebase. However, there are several critical security and performance issues that need attention. Critical Issues
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@client/src/components/PlayerSearch.tsx`:
- Around line 45-58: The Torii SQL query in the URL construction for
PlayerSearch (uses currentNetworkConfig.toriiUrl, searchQuery, and the fetch
call) interpolates raw user input and can be broken by quotes or injection; fix
by sanitizing searchQuery before building the query: escape single quotes in
searchQuery (e.g., replace ' with ''), then construct the SQL string and
URL-encode the whole query parameter with encodeURIComponent before appending to
`${currentNetworkConfig.toriiUrl}/sql?query=`; apply the same
escaping+encodeURIComponent pattern to all other Torii SQL query usages (e.g.,
in useFollowedPlayerNotifications.tsx, useGameSettings.ts, useGameEvents.ts).
♻️ Duplicate comments (2)
client/src/desktop/components/Leaderboard.tsx (1)
113-139: Following tab can stay stale because of theprevGamesRefguard.
The guard blocks re-filtering when the tab or follow list changes without a newgamesreference.🔧 Suggested fix (remove stale reference guard)
- if (!loading && games && games !== prevGamesRef.current) { - prevGamesRef.current = games; + if (!loading && games) { + prevGamesRef.current = games;client/src/mobile/components/Leaderboard.tsx (1)
110-136: Following tab can stay stale because of theprevGamesRefguard.
Same issue as desktop: tab/follow changes won’t re-filter without a newgamesreference.🔧 Suggested fix (remove stale reference guard)
- if (!loading && games && games !== prevGamesRef.current) { - prevGamesRef.current = games; + if (!loading && games) { + prevGamesRef.current = games;
- Use TokenPlayerNameUpdate table joined with OwnersUpdate for player data - Decode hex-encoded player names using hexToAscii - Filter results client-side after decoding for accurate name matching Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @loothero's task —— View job Code Review: Follow Player FeatureComprehensive review of the follow player feature implementation with in-app notifications. ✅ StrengthsArchitecture & Patterns:
StarkNet Integration:
User Experience:
🔴 Critical Issues1. SQL Injection Vulnerability ( const ownersList = followedAddresses
.map((addr) => `"${addAddressPadding(addr)}"`)
.join(',');
const url = `${currentNetworkConfig.toriiUrl}/sql?query=
SELECT token_id, owner, player_name, minted_at
FROM "relayer_0_0_1-TokenMetadataUpdate"
WHERE owner IN (${ownersList})`; // Direct string interpolation!Impact: Malicious addresses could inject SQL commands 2. Performance Concerns
🟡 Security & Robustness Issues3. Error Handling Gaps
4. Memory Management
🟠 Minor Issues5. Code Quality
RecommendationsHigh Priority
Medium Priority
Low Priority
Overall AssessmentThis is a well-architected feature that follows the codebase patterns effectively. The core functionality is solid, but the SQL injection vulnerability is critical and must be addressed before merging. With security fixes and performance optimizations, this will be a valuable addition to the game's social features. Recommendation: Request changes to address security issues before approval. |
- Use the same useGameTokens hook that leaderboard uses - Search within fetched games data for matching player names - Removes dependency on specific SQL table structure - Guarantees same data source as leaderboard display Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @loothero's task —— View job Code Review: Follow Player FeatureComprehensive review of the follow player feature implementation with in-app notifications. Todo List
Review SummaryThis PR implements a well-architected follow player feature with in-app notifications. The code follows established patterns and integrates cleanly with the existing codebase. However, there are several critical security and performance issues that need attention. Critical Issues
|
Fetches unique player names directly from database with proper joins between TokenPlayerNameUpdate, OwnersUpdate, TokenMetadataUpdate, and MinterRegistryUpdate tables. Decodes hex-encoded player names and filters client-side for search matches. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @loothero's task —— View job Code Review: Follow Player FeatureComprehensive review of the follow player feature implementation with in-app notifications. Todo List
Review SummaryThis PR implements a well-architected follow player feature with in-app notifications. The code follows established patterns and integrates cleanly with the existing codebase. However, there are several critical security and performance issues that need attention. Critical Issues
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@client/src/hooks/usePlayerSearch.ts`:
- Around line 23-32: The code uses GAME_TOKEN_ADDRESS (from getContractByName)
to set mintedByAddress which can be undefined when chainId ===
ChainId.WP_PG_SLOT; update the searchPlayers function to guard against an
undefined mintedByAddress before building/executing the SQL query: check the
computed mintedByAddress (the branch using ChainId.WP_PG_SLOT and
GAME_TOKEN_ADDRESS or the addAddressPadding(dungeon.address) branch) and if it's
falsy either return an empty result or throw a clear error, so you never
interpolate "undefined" into the SQL; reference GAME_TOKEN_ADDRESS,
mintedByAddress, searchPlayers, ChainId.WP_PG_SLOT, and
addAddressPadding(dungeon.address) when locating where to add the guard.
| const GAME_TOKEN_ADDRESS = getContractByName( | ||
| currentNetworkConfig.manifest, | ||
| currentNetworkConfig.namespace, | ||
| "game_token_systems" | ||
| )?.address; | ||
|
|
||
| const mintedByAddress = currentNetworkConfig.chainId === ChainId.WP_PG_SLOT | ||
| ? GAME_TOKEN_ADDRESS | ||
| : addAddressPadding(dungeon.address); | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "usePlayerSearch.ts" --exec cat -n {} \;Repository: Provable-Games/death-mountain
Length of output: 4992
🏁 Script executed:
# Search for useDungeon hook definition
fd -t f "useDungeon*" --exec head -50 {} \;Repository: Provable-Games/death-mountain
Length of output: 1506
🏁 Script executed:
# Search for useDungeon hook implementation
rg "export.*useDungeon|function useDungeon" -A 20 --type ts --type tsxRepository: Provable-Games/death-mountain
Length of output: 100
🏁 Script executed:
# Search for useDungeon hook implementation
rg "export.*useDungeon|function useDungeon" -A 20 --type tsRepository: Provable-Games/death-mountain
Length of output: 428
🏁 Script executed:
# Check getContractByName return type
rg "getContractByName" -B 2 -A 5 client/src --type tsRepository: Provable-Games/death-mountain
Length of output: 13529
🏁 Script executed:
# Check if addAddressPadding can handle undefined/null
web_search="addAddressPadding starknet undefined null behavior"
python3 << 'EOF'
import urllib.parse
query = "addAddressPadding starknet undefined null behavior"
print(f"Query: {query}")
# This would need web_search but let's also check starknet documentation locally
EOF
# Also check if there are type definitions for addAddressPadding
rg "addAddressPadding" -B 3 -A 3 client/src --type ts | head -40Repository: Provable-Games/death-mountain
Length of output: 159
🏁 Script executed:
# Check addAddressPadding usage patterns in the codebase
rg "addAddressPadding" client/src --type ts -B 2 -A 2Repository: Provable-Games/death-mountain
Length of output: 16992
🏁 Script executed:
# Check if there are any guard checks in the searchPlayers function or elsewhere
rg "mintedByAddress" client/src/hooks/usePlayerSearch.ts -B 5 -A 5Repository: Provable-Games/death-mountain
Length of output: 1228
Guard against missing GAME_TOKEN_ADDRESS before querying.
Line 23–31: When chainId === ChainId.WP_PG_SLOT, GAME_TOKEN_ADDRESS can be undefined due to optional chaining. This makes mintedByAddress undefined, which gets interpolated directly into the SQL query at line 54, producing invalid queries like WHERE mr.minter_address = "undefined". Add a guard check in searchPlayers to validate mintedByAddress before querying.
Suggested fix
const searchPlayers = useCallback(async (searchQuery: string): Promise<PlayerSearchResult[]> => {
if (!searchQuery.trim()) {
setResults([]);
return [];
}
setLoading(true);
setError(null);
try {
+ if (!mintedByAddress) {
+ setError('Game token address not available');
+ setResults([]);
+ return [];
+ }
// Query unique player names with their owners🤖 Prompt for AI Agents
In `@client/src/hooks/usePlayerSearch.ts` around lines 23 - 32, The code uses
GAME_TOKEN_ADDRESS (from getContractByName) to set mintedByAddress which can be
undefined when chainId === ChainId.WP_PG_SLOT; update the searchPlayers function
to guard against an undefined mintedByAddress before building/executing the SQL
query: check the computed mintedByAddress (the branch using ChainId.WP_PG_SLOT
and GAME_TOKEN_ADDRESS or the addAddressPadding(dungeon.address) branch) and if
it's falsy either return an empty result or throw a clear error, so you never
interpolate "undefined" into the SQL; reference GAME_TOKEN_ADDRESS,
mintedByAddress, searchPlayers, ChainId.WP_PG_SLOT, and
addAddressPadding(dungeon.address) when locating where to add the guard.
Logs query construction, API response, hex decoding, and filtering steps to help diagnose search issues. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Claude finished @loothero's task —— View job Code Review: Follow Player FeatureComprehensive review of the follow player feature implementation with in-app notifications. Todo List
Review SummaryThis PR implements a well-architected follow player feature with in-app notifications. The code follows established patterns and integrates cleanly with the existing codebase. However, there are several critical security and performance issues that need attention. Critical Issues
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@client/src/hooks/usePlayerSearch.ts`:
- Around line 36-40: In usePlayerSearch, when the branch that handles an empty
query returns early (the block that logs and calls setResults([])), also clear
the hook's error state so a previous failure isn't still shown; add a call to
the error-state setter (e.g., setError(null) or setSearchError(null) depending
on the hook's state variable) immediately after setResults([]) to reset errors
before returning.
♻️ Duplicate comments (1)
client/src/hooks/usePlayerSearch.ts (1)
42-58: Guard against missingmintedByAddressbefore constructing the SQL.
IfGAME_TOKEN_ADDRESSis unavailable forChainId.WP_PG_SLOT, the query becomes... = "undefined".🛠️ Proposed fix
- setLoading(true); - setError(null); + if (!mintedByAddress) { + setError('Game token address not available'); + setResults([]); + return []; + } + + setLoading(true); + setError(null);
| if (!searchQuery.trim()) { | ||
| console.log('[PlayerSearch] Empty search query, clearing results'); | ||
| setResults([]); | ||
| return []; | ||
| } |
There was a problem hiding this comment.
Clear error state when the query is empty.
Without this, a previous failure can keep an error visible after the user clears the input.
🛠️ Proposed fix
if (!searchQuery.trim()) {
console.log('[PlayerSearch] Empty search query, clearing results');
+ setError(null);
setResults([]);
return [];
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!searchQuery.trim()) { | |
| console.log('[PlayerSearch] Empty search query, clearing results'); | |
| setResults([]); | |
| return []; | |
| } | |
| if (!searchQuery.trim()) { | |
| console.log('[PlayerSearch] Empty search query, clearing results'); | |
| setError(null); | |
| setResults([]); | |
| return []; | |
| } |
🤖 Prompt for AI Agents
In `@client/src/hooks/usePlayerSearch.ts` around lines 36 - 40, In
usePlayerSearch, when the branch that handles an empty query returns early (the
block that logs and calls setResults([])), also clear the hook's error state so
a previous failure isn't still shown; add a call to the error-state setter
(e.g., setError(null) or setSearchError(null) depending on the hook's state
variable) immediately after setResults([]) to reset errors before returning.
Summary
Implement Phase 1 of the Follow Player Feature - in-app notifications using polling via Torii SQL queries. This allows players to follow other players and receive toast notifications when followed players start new games.
Changes
New Files
client/src/stores/followStore.ts: Zustand store with localStorage persistence for follow relationshipsclient/src/components/FollowButton.tsx: Reusable heart icon button for follow/unfollow actionsclient/src/components/FollowingList.tsx: Component to display list of followed playersclient/src/hooks/useFollowedPlayerNotifications.tsx: Polling-based hook (30s interval) that checks for new games from followed players via Torii SQLModified Files
client/src/desktop/components/Leaderboard.tsx: Added follow button next to player names, added "Following" tabclient/src/mobile/components/Leaderboard.tsx: Added follow button next to player names, added "Following" tabclient/src/App.tsx: Initialize notification subscription hook at app levelFeatures
Testing
pnpm buildpasses without errors@codex review
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.