BibliothecaDAO / eternum

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

fix: wrong usage of useMemo #913

Closed r0man1337 closed 3 months ago

r0man1337 commented 3 months ago

User description

Fixed wrong usage of useMemo. All setState calls should be placed inside useEffect hooks instead of useMemo.

Wrong usage causing such errors: image


PR Type

Bug fix


Description


Changes walkthrough πŸ“

Relevant files
Bug fix
_mapStore.tsx
Replace `useMemo` with `useEffect` for built structures setup

client/src/hooks/store/_mapStore.tsx
  • Replaced useMemo with useEffect for setting up built structures.
  • Ensured state updates are correctly placed inside useEffect.
  • +1/-1     
    WorldHexagon.tsx
    Replace `useMemo` with `useEffect` for stamina calculations

    client/src/ui/components/worldmap/hexagon/WorldHexagon.tsx
  • Replaced useMemo with useEffect for stamina-related calculations.
  • Ensured state updates are correctly placed inside useEffect.
  • +1/-1     

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

    vercel[bot] commented 3 months 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 3:25pm
    github-actions[bot] commented 3 months ago

    PR Reviewer Guide πŸ”

    ⏱️ Estimated effort to review [1-5] 2
    πŸ§ͺ Relevant tests No
    πŸ”’ Security concerns No
    ⚑ Key issues to review None
    github-actions[bot] commented 3 months ago

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Add missing dependencies to useEffect to ensure it updates correctly ___ **Ensure that setup.components.Position and entity are included in the dependency array of
    useEffect to handle updates correctly.** [client/src/hooks/store/_mapStore.tsx [139-142]](https://github.com/BibliothecaDAO/eternum/pull/913/files#diff-6d3df53ed6fbfb737106e9d7b48fa7c3fa33d11521804ea10a7f5d2fad2517beR139-R142) ```diff useEffect(() => { const _tmp = builtStructures .map((entity) => { const position = getComponentValue(setup.components.Position, entity); +}, [setup.components.Position, builtStructures]); ```
    Suggestion importance[1-10]: 9 Why: Including `setup.components.Position` and `entity` in the dependency array ensures that the `useEffect` hook updates correctly when these values change, preventing potential bugs.
    9
    Update useEffect dependencies to react to changes in selectedEntity and stamina ___ **Add selectedEntity and stamina to the dependency array of useEffect to ensure that it
    reacts to changes in these values.** [client/src/ui/components/worldmap/hexagon/WorldHexagon.tsx [111-114]](https://github.com/BibliothecaDAO/eternum/pull/913/files#diff-eab0cbdc1c33d4db9d2fda4be1bb8f506aff500e566c896d59d061882758345cR111-R114) ```diff useEffect(() => { if (!selectedEntity || !stamina) return; const maxTravelPossible = Math.floor((stamina.amount || 0) / EternumGlobalConfig.stamina.travelCost); +}, [selectedEntity, stamina]); ```
    Suggestion importance[1-10]: 9 Why: Adding `selectedEntity` and `stamina` to the dependency array ensures that the `useEffect` hook reacts to changes in these values, preventing potential bugs and ensuring correct behavior.
    9
    Performance
    Memoize the result of getComponentValue to improve performance ___ **Replace the direct usage of getComponentValue with a memoized value to avoid unnecessary
    recalculations each time the component re-renders. This is particularly useful if
    getComponentValue is computationally expensive.** [client/src/hooks/store/_mapStore.tsx [142]](https://github.com/BibliothecaDAO/eternum/pull/913/files#diff-6d3df53ed6fbfb737106e9d7b48fa7c3fa33d11521804ea10a7f5d2fad2517beR142-R142) ```diff -const position = getComponentValue(setup.components.Position, entity); +const position = useMemo(() => getComponentValue(setup.components.Position, entity), [entity]); ```
    Suggestion importance[1-10]: 8 Why: Memoizing `getComponentValue` can improve performance by avoiding unnecessary recalculations during re-renders, especially if the function is computationally expensive. This is a valuable optimization.
    8
    Use useMemo for calculating maxTravelPossible to enhance performance ___ **Consider using useMemo to compute maxTravelPossible since it is derived from stamina,
    which might not change on every render. This will help in avoiding unnecessary
    calculations.** [client/src/ui/components/worldmap/hexagon/WorldHexagon.tsx [114]](https://github.com/BibliothecaDAO/eternum/pull/913/files#diff-eab0cbdc1c33d4db9d2fda4be1bb8f506aff500e566c896d59d061882758345cR114-R114) ```diff -const maxTravelPossible = Math.floor((stamina.amount || 0) / EternumGlobalConfig.stamina.travelCost); +const maxTravelPossible = useMemo(() => Math.floor((stamina.amount || 0) / EternumGlobalConfig.stamina.travelCost), [stamina]); ```
    Suggestion importance[1-10]: 8 Why: Using `useMemo` for `maxTravelPossible` can prevent unnecessary calculations on every render, enhancing performance. This is a useful optimization.
    8