search

BibliothecaDAO / eternum

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

Fix/hover building #1054

Closed cwastche closed 2 days ago

cwastche commented 2 days ago

PR Type

Bug fix, Enhancement Closes #1051

Description

Changes walkthrough 📝

Relevant files
Enhancement
ExistingBuildings.tsx
Fix and enhance hover information for resource buildings 
client/src/ui/components/construction/ExistingBuildings.tsx
  • Added resource prop to BuiltBuilding and HoverBuilding components.
  • Updated HoverBuilding to use resource prop for ResourceInfo.
    		•  +4/-1     
    Formatting
    HintBox.tsx
    Format quest mapping in QuestDepthGroup component               
    client/src/ui/components/hints/HintBox.tsx - Minor formatting change in `QuestDepthGroup` component.
    		+1/-3     

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

    vercel[bot] commented 2 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 30, 2024 7:19pm
    github-actions[bot] commented 2 days ago

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 2
    🧪 Relevant tests No
    🔒 Security concerns No
    ⚡ Key issues to review Possible Bug:
    The resource prop is used in HoverBuilding component without null checking. Since resource is optional (resource?), ensure that it is not undefined before using it in ResourceInfo.
    github-actions[bot] commented 2 days ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    Ensure type safety by defining the type of the 'resource' prop ___ **Add type definition for the new resource prop in the HoverBuilding component to ensure
    type safety and better maintainability.** [client/src/ui/components/construction/ExistingBuildings.tsx [300]](https://github.com/BibliothecaDAO/eternum/pull/1054/files#diff-b8b13d61f7fc906dae1ff3473650c5893b1d3971d1b65e062a1fb4c6ed5e749bR300-R300) ```diff -resource, +resource?: ResourcesIds, ```
    Suggestion importance[1-10]: 10 Why: Adding a type definition for the `resource` prop is essential for maintaining type safety and improving code maintainability, making this a highly valuable suggestion.
    		10
    Possible bug
    Replace the non-null assertion with a default value or conditional rendering ___ **Avoid using the non-null assertion operator (!) on resource in the ResourceInfo component
    to prevent runtime errors. Instead, handle the potential undefined or null values
    gracefully.** [client/src/ui/components/construction/ExistingBuildings.tsx [316]](https://github.com/BibliothecaDAO/eternum/pull/1054/files#diff-b8b13d61f7fc906dae1ff3473650c5893b1d3971d1b65e062a1fb4c6ed5e749bR316-R316) ```diff - + ```
    Suggestion importance[1-10]: 9 Why: Avoiding the non-null assertion operator and handling potential undefined values gracefully is important for preventing runtime errors, making this a significant improvement.
    		9
    Possible issue
    Add a default value or handle the potential undefined state for the 'resource' prop ___ **Ensure that the resource prop in BuiltBuilding component is always provided or handled
    properly when undefined. If resource is optional, consider adding a default value or
    conditional rendering to avoid runtime errors.** [client/src/ui/components/construction/ExistingBuildings.tsx [288]](https://github.com/BibliothecaDAO/eternum/pull/1054/files#diff-b8b13d61f7fc906dae1ff3473650c5893b1d3971d1b65e062a1fb4c6ed5e749bR288-R288) ```diff -resource={resource} +resource={resource ?? 'defaultResource'} ```
    Suggestion importance[1-10]: 8 Why: This suggestion addresses a potential runtime error by ensuring that the `resource` prop is always defined, which is crucial for preventing crashes and improving code robustness.
    		8
    Enhancement
    Simplify the lambda expression in the map function for better readability ___ **Simplify the rendering of QuestCard components by removing unnecessary parentheses and
    maintaining the concise syntax as shown in the new code.** [client/src/ui/components/hints/HintBox.tsx [169]](https://github.com/BibliothecaDAO/eternum/pull/1054/files#diff-5cf22aac8f0607d764110992d7cb5eac12ced82af11478c86e79fd3659cefc6aR169-R169) ```diff -{depthQuests?.map((quest: Quest) => )} +{depthQuests?.map(quest => )} ```
    Suggestion importance[1-10]: 6 Why: This suggestion improves code readability by simplifying the lambda expression, which is a minor but useful enhancement.
    		6