-
Notifications
You must be signed in to change notification settings - Fork 34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: player id implemented #1723
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Failed to generate code suggestions for PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request implements a player ID feature and makes several improvements to the chat system. The changes look good overall, with a few minor suggestions for optimization and code organization. The new PlayerId component provides a nice overview of player information, and the chat system updates to support direct messaging are well integrated. The use of zustand for state management is consistent throughout the changes. Consider some minor optimizations in the Chat component and potentially breaking down the PlayerId component into smaller subcomponents for better maintainability.
Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!
setTabs(newTabs); | ||
localStorage.setItem(CHAT_STORAGE_KEY + account.address, JSON.stringify(newTabs)); | ||
}; | ||
|
||
const messages = useMemo(() => { | ||
const messageMap = new Map<ContractAddress, ChatMessage[]>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider memoizing this value to avoid recalculating on every render.
@@ -116,7 +117,7 @@ | |||
messages.sort((a, b) => a.timestamp.getTime() - b.timestamp.getTime()); | |||
}); | |||
return messageMap; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This dependency array includes account.address
, which may cause unnecessary re-renders. Consider using a more specific dependency if possible.
const realmSettleData = getComponentValue( | ||
SettleRealmData, | ||
Array.from(runQuery([HasValue(SettleRealmData, { owner_address: selectedPlayer })]))[0], | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is accessing the store state directly. Consider using a selector or moving this logic to a custom hook for better performance and maintainability.
)} | ||
</div> | ||
); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider moving this avatar generation logic to a separate utility function or component for better reusability and separation of concerns.
No description provided.