search

BibliothecaDAO / eternum

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

remove food quest #1047

Closed aymericdelab closed 4 days ago

aymericdelab commented 4 days ago

PR Type

Enhancement, Bug fix

Description

Changes walkthrough 📝

Relevant files
Enhancement
useQuestStore.tsx
Remove `ClaimFood` quest from quest store                               
client/src/hooks/store/useQuestStore.tsx
  • Removed ClaimFood quest from QuestName enum.
  • Deleted the ClaimFood quest object from the quests array.
    		•  +0/-10   
    LeftNavigationModule.tsx
    Remove `ClaimFood` quest condition from left navigation   
    client/src/ui/modules/navigation/LeftNavigationModule.tsx
  • Removed condition to hide the construction button based on ClaimFood
    quest.
    		•  +0/-1     
    RightNavigationModule.tsx
    Remove `ClaimFood` quest condition from right navigation 
    client/src/ui/modules/navigation/RightNavigationModule.tsx
  • Removed ClaimFood quest related class condition from the resources
    button.
    		•  +0/-3     
    global.ts
    Add starting resources configuration                                         
    sdk/packages/eternum/src/constants/global.ts - Added `startingResources` configuration with initial resources.
    		+6/-0     
    resources.ts
    Remove `QuestType.Food` and associated resources                 
    sdk/packages/eternum/src/constants/resources.ts - Removed `QuestType.Food` and its associated resources.
    		+9/-13   
    index.ts
    Mint starting resources in realm creation                               
    sdk/packages/eternum/src/provider/index.ts
  • Added minting of starting resources in create_multiple_realms method.
    		•  +18/-0   
    Bug fix
    HintBox.tsx
    Update quest display depth initialization                               
    client/src/ui/components/hints/HintBox.tsx
  • Updated the initial depth value in QuestsDisplay to start from 1.
  • Adjusted the QuestDepthGroup component to format the QuestCard
    mapping.
    		•  +5/-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 4 days 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 29, 2024 0:17am
    github-actions[bot] commented 4 days ago

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 3
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The removal of the ClaimFood quest from useQuestStore.tsx and related UI components might affect other parts of the application that could still be referencing it. Ensure that all dependencies and references to this quest have been properly removed or updated.
    Code Clarity:
    In HintBox.tsx, the change in initial depth value calculation from 0 to 1 should be clearly documented in the code comments to explain why this change was necessary.
    github-actions[bot] commented 4 days ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Security
    Implement validation checks for new resource minting logic to ensure security and correctness ___ **With the introduction of new resource minting logic in the create_multiple_realms method,
    it's crucial to ensure that the minting process is secure and efficient. Consider
    implementing checks to validate the uuid, resource IDs, and amounts before proceeding with
    the minting operation to prevent potential misuse or errors in resource allocation.** [sdk/packages/eternum/src/provider/index.ts [213-225]](https://github.com/BibliothecaDAO/eternum/pull/1047/files#diff-467989f145beaf5796d645b8891d6c2468a39d539d62b48a1459185c6fe1de55R213-R225) ```diff +contractAddress: getContractByName(this.manifest, "dev_resource_systems"), +entrypoint: "mint", +calldata: [ + uuid, + EternumGlobalConfig.resources.startingResources.length, + ...EternumGlobalConfig.resources.startingResources.flatMap(({ resourceId, amount }) => [ + resourceId, + amount * + EternumGlobalConfig.resources.resourcePrecision * + EternumGlobalConfig.resources.resourceMultiplier, + ]), +], - ```
    Suggestion importance[1-10]: 10 Why: The suggestion addresses a critical security concern by recommending validation checks for the new resource minting logic. This is essential to prevent misuse and ensure the correctness of resource allocation.
    		10
    Possible bug
    Ensure removal of dependent logic on the ClaimFood quest to avoid errors ___ **Since the ClaimFood quest has been removed from the QuestName enum and the quests array,
    it's important to ensure that any logic dependent on this quest is also updated or
    removed. This includes removing any conditional rendering or logic that checks for the
    ClaimFood quest in other parts of the application to avoid runtime errors or incorrect
    application behavior.** [client/src/hooks/store/useQuestStore.tsx [138-140]](https://github.com/BibliothecaDAO/eternum/pull/1047/files#diff-a7c36ca377844946d158e339e652e7588b89111d70276c01cf716ce0dea0c9daR138-R140) ```diff +name: QuestName.BuildFarm, +description: "Wheat is the lifeblood of your people. Go to the construction menu and build a farm.", - ```
    Suggestion importance[1-10]: 9 Why: The suggestion correctly identifies the need to remove any logic dependent on the `ClaimFood` quest to prevent runtime errors and incorrect application behavior. This is crucial for maintaining the integrity of the application.
    		9
    Update systems impacted by the removal of resources associated with the ClaimFood quest ___ **The removal of the ClaimFood quest and its associated resources should be carefully
    managed to ensure that any system relying on these resources is updated accordingly. This
    includes updating resource management systems, reward systems, and any other
    functionalities that might be impacted by the removal of these resources.** [sdk/packages/eternum/src/constants/resources.ts [849-851]](https://github.com/BibliothecaDAO/eternum/pull/1047/files#diff-03877f9a7de964bc35d454531a120cf998aca1a8b8ebd1d0471d35393c38ac4aR849-R851) ```diff +[QuestType.CommonResources]: [ + { resource: ResourcesIds.Wood, amount: 5 }, + { resource: ResourcesIds.Stone, amount: 5 }, - ```
    Suggestion importance[1-10]: 8 Why: The suggestion correctly points out the need to update any systems that relied on the resources associated with the `ClaimFood` quest. This is important to prevent potential issues in resource management and reward systems.
    		8
    Possible issue
    Review and adjust UI conditional rendering logic due to the removal of the ClaimFood quest ___ **The removal of the ClaimFood quest should be accompanied by a review of any UI elements
    that are conditionally rendered based on this quest's status. Specifically, the hidden
    class that was conditionally applied based on the ClaimFood quest should be reassessed to
    ensure that the UI behaves as expected without this quest.** [client/src/ui/modules/navigation/LeftNavigationModule.tsx [147-148]](https://github.com/BibliothecaDAO/eternum/pull/1047/files#diff-be9e8648ba688d13b3b68afca51d87c31cca8d9412f8c7d77edee0d71737a287R147-R148) ```diff +"animate-pulse": view != View.ConstructionView && isBuildQuest && isPopupOpen(questsPopup), - ```
    Suggestion importance[1-10]: 8 Why: The suggestion highlights the importance of reassessing UI elements that were conditionally rendered based on the `ClaimFood` quest. This is important to ensure the UI behaves as expected without the quest.
    		8