fix small ui bugs

[bob0005]

PR Type

Bug fix, Enhancement

---

Description

• Refactored the createQuest function in useQuests.tsx to use useMemo directly, simplifying the code and improving performance.
• Simplified the status determination logic for various quests in useQuests.tsx.
• Fixed the filter condition for unclaimedQuests in QuestList.tsx to correctly exclude claimed quests.
• Added questClaimStatus to the dependency array of useMemo in LeftNavigationModule.tsx to ensure proper memoization.

---

Changes walkthrough 📝

	Relevant files
Enhancement	useQuests.tsx Refactor quest creation and status logic                                  client/src/hooks/helpers/useQuests.tsx Refactored createQuest function to use useMemo directly. Simplified status determination logic for various quests. Removed unnecessary useMemo wrapping for quests array. +77/-90
Bug fix	QuestList.tsx Fix unclaimed quests filter condition                                        client/src/ui/components/quest/QuestList.tsx Fixed filter condition for unclaimedQuests to exclude claimed quests. +1/-1      LeftNavigationModule.tsx Update dependencies for navigation memoization                      client/src/ui/modules/navigation/LeftNavigationModule.tsx Added questClaimStatus to dependency array of useMemo. +1/-1

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
eternum	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 26, 2024 4:31pm

[mentatbot]

Overall, this pull request contains several important optimizations and bug fixes. The changes in useQuests.tsx improve performance and readability. The fix in QuestList.tsx ensures all unclaimed quests are included, which is crucial for the skip tutorial functionality. The update to LeftNavigationModule.tsx ensures proper reactivity to quest status changes. These changes should improve the overall user experience and application performance.

Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!

[mentatbot]

Good optimization. Using useMemo for each quest creation will prevent unnecessary re-renders.

[mentatbot]

This refactoring improves readability and reduces nesting. It's a good change.

[github-actions]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Dependency Array The dependency array for useMemo in the createQuest function may be incorrect. It currently includes questDependencies[questId], which might not trigger re-computation when other properties of questDependencies change that could affect the quest's status. Simplification The refactoring of the createQuest function and the quests array simplifies the code but might introduce performance issues if not all dependencies are correctly tracked, leading to unnecessary re-renders. Logic Change The change in the filter condition for unclaimedQuests from checking for QuestStatus.InProgress to not QuestStatus.Claimed might unintentionally include quests with other statuses that should not be considered unclaimed.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add error handling for potentially undefined quest details Consider adding error handling for cases where questDetails.get(questId) might return undefined, to prevent runtime errors. client/src/hooks/helpers/useQuests.tsx [42] -...questDetails.get(questId)!, +...(questDetails.get(questId) || { error: 'Quest details not found' }), Suggestion importance[1-10]: 10 Why: Adding error handling for potentially undefined quest details prevents runtime errors, improving the robustness and reliability of the code.	10
Best practice	Replace useMemo with useCallback for function memoization Consider using useCallback instead of useMemo for defining the createQuest function to ensure that the function is memoized correctly. useMemo should ideally be used for memoizing values, not functions. client/src/hooks/helpers/useQuests.tsx [37-46] -const createQuest = (questId: QuestId) => { +const createQuest = useCallback((questId: QuestId) => { const dependency = questDependencies[questId]; - return useMemo( - () => ({ - id: questId, - ...questDetails.get(questId)!, - status: dependency.status, - }), - [questDependencies[questId]], - ); -}; + return { + id: questId, + ...questDetails.get(questId)!, + status: dependency.status, + }; +}, [questDependencies]); Suggestion importance[1-10]: 9 Why: Using useCallback for memoizing functions is more appropriate than useMemo, which is intended for memoizing values. This change improves the correctness and performance of the code.	9
Enhancement	Use a loop to dynamically generate the quests array Use a loop to generate the quests array to avoid repetition and improve code scalability. client/src/hooks/helpers/useQuests.tsx [49-62] -const quests = [ - createQuest(QuestId.Settle), - createQuest(QuestId.BuildFarm), - createQuest(QuestId.BuildResource), - createQuest(QuestId.CreateTrade), - createQuest(QuestId.CreateArmy), - createQuest(QuestId.Travel), - createQuest(QuestId.BuildWorkersHut), - createQuest(QuestId.Market), - createQuest(QuestId.Pillage), - createQuest(QuestId.Mine), - createQuest(QuestId.Contribution), - createQuest(QuestId.Hyperstructure), -]; +const questIds = [QuestId.Settle, QuestId.BuildFarm, QuestId.BuildResource, QuestId.CreateTrade, QuestId.CreateArmy, QuestId.Travel, QuestId.BuildWorkersHut, QuestId.Market, QuestId.Pillage, QuestId.Mine, QuestId.Contribution, QuestId.Hyperstructure]; +const quests = questIds.map(id => createQuest(id)); Suggestion importance[1-10]: 8 Why: Using a loop to generate the quests array reduces repetition and enhances scalability, making the code more maintainable and easier to extend.	8
Maintainability	Refactor complex ternary operations into a function for clarity Refactor the ternary operations for setting quest status to improve readability and maintainability. client/src/hooks/helpers/useQuests.tsx [135-139] -status: questClaimStatus[QuestId.BuildFarm] - ? QuestStatus.Claimed - : buildingQuantities.farms > 0 - ? QuestStatus.Completed - : QuestStatus.InProgress, +status: getQuestStatus(questClaimStatus[QuestId.BuildFarm], buildingQuantities.farms) Suggestion importance[1-10]: 7 Why: Refactoring the ternary operations into a function improves readability and maintainability, making the code easier to understand and modify.	7

[edisontim]

lgtm