BibliothecaDAO / eternum

onchain eternal game
https://alpha-eternum.realms.world
MIT License
37 stars 31 forks source link

Enh quests popup #915

Closed cwastche closed 1 month ago

cwastche commented 1 month ago

User description


PR Type

Enhancement, Bug fix


Description


Changes walkthrough ๐Ÿ“

Relevant files
Enhancement
useQuests.tsx
Enhance quest tracking and completion logic                           

client/src/hooks/helpers/useQuests.tsx
  • Added HasClaimedStartingResources component to track claimed
    resources.
  • Updated quest completion logic to check for claimed resources.
  • Simplified quest steps and prizes structure.
  • +31/-93 
    HintBox.tsx
    Update HintBox component to handle claimed quests               

    client/src/ui/components/hints/HintBox.tsx
  • Added claimed property to quest interface.
  • Removed redundant hasClaimed logic.
  • Updated UI to reflect claimed status and toggle popup.
  • +16/-22 
    Bug fix
    BottomNavigation.tsx
    Improve quest button state handling in navigation               

    client/src/ui/modules/navigation/BottomNavigation.tsx
  • Disabled quest button if no claimable quests are available.
  • Added null check for claimableQuests.
  • +2/-2     

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

    vercel[bot] commented 1 month ago

    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 Jun 12, 2024 9:52pm
    github-actions[bot] commented 1 month ago

    PR Reviewer Guide ๐Ÿ”

    โฑ๏ธ Estimated effort to review [1-5] 3
    ๐Ÿงช Relevant tests No
    ๐Ÿ”’ Security concerns No
    โšก Key issues to review Possible Bug:
    The refactoring in `useQuests.tsx` introduces a new way to handle quests by checking if all prizes have been claimed. However, the logic assumes that all quests have prizes which might not always be the case. This could potentially lead to runtime errors if any quest does not have a `prizes` array.
    Redundancy:
    The `useQuests.tsx` and `HintBox.tsx` both contain logic related to checking if a quest's prizes have been claimed. This could be simplified and centralized to avoid redundancy and potential discrepancies in quest status handling.
    github-actions[bot] commented 1 month ago

    PR Code Suggestions โœจ

    CategorySuggestion                                                                                                                                    Score
    Maintainability
    Reduce redundancy by consolidating repeated state update calls into a single function ___ **Consolidate the repeated setIsLoading(true) calls into a single function to reduce
    redundancy and improve maintainability.** [client/src/ui/components/hints/HintBox.tsx [39-54]](https://github.com/BibliothecaDAO/eternum/pull/915/files#diff-5cf22aac8f0607d764110992d7cb5eac12ced82af11478c86e79fd3659cefc6aR39-R54) ```diff -setIsLoading(true); +startLoading(); ```
    Suggestion importance[1-10]: 9 Why: Consolidating repeated `setIsLoading(true)` calls into a single function significantly improves maintainability and reduces redundancy, making the code cleaner and easier to manage.
    9
    Reduce redundancy by using a constant for repeated steps in quest definitions ___ **Refactor the repeated steps in each quest object to a constant to avoid redundancy and
    potential errors in future modifications.** [client/src/hooks/helpers/useQuests.tsx [35]](https://github.com/BibliothecaDAO/eternum/pull/915/files#diff-02207afc6e5bd13a56e092c44f234cbddae30dd5cf9704eb786a3b8f7da20d33R35-R35) ```diff -steps: [{ description: "Claim Food" }, { description: "Build a farm" }], +steps: commonSteps, ```
    Suggestion importance[1-10]: 8 Why: This suggestion addresses maintainability by reducing redundancy, which can help prevent potential errors in future modifications. It is a good practice to use constants for repeated values.
    8
    Performance
    Optimize code by ensuring entityId is consistently a bigint ___ **Ensure that the entityId is always a bigint by converting it at the point of use rather
    than repeatedly converting it in multiple function calls.** [client/src/hooks/helpers/useQuests.tsx [79]](https://github.com/BibliothecaDAO/eternum/pull/915/files#diff-02207afc6e5bd13a56e092c44f234cbddae30dd5cf9704eb786a3b8f7da20d33R79-R79) ```diff -getEntityIdFromKeys([BigInt(entityId || "0"), BigInt(prize.id)]), +getEntityIdFromKeys([entityId, BigInt(prize.id)]), ```
    Suggestion importance[1-10]: 7 Why: Ensuring `entityId` is consistently a `bigint` improves performance and code clarity. However, the improvement is minor as it only affects a small part of the code.
    7
    Readability
    Improve code readability by using more descriptive variable names ___ **Use a more descriptive variable name than updatedQuests to reflect the nature of the data,
    such as questsWithClaimStatus.** [client/src/hooks/helpers/useQuests.tsx [30]](https://github.com/BibliothecaDAO/eternum/pull/915/files#diff-02207afc6e5bd13a56e092c44f234cbddae30dd5cf9704eb786a3b8f7da20d33R30-R30) ```diff -const updatedQuests = [ +const questsWithClaimStatus = [ ```
    Suggestion importance[1-10]: 6 Why: Using more descriptive variable names enhances code readability and maintainability. However, this is a minor improvement and does not affect the functionality.
    6