Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Change to V1 + add type aliases for entity ids #1148

Merged
merged 7 commits into from
Jul 31, 2024
Merged

Conversation

edisontim
Copy link
Collaborator

@edisontim edisontim commented Jul 30, 2024

PR Type

enhancement, other


Description

  • Refactored component definitions in contractComponents.ts to use new naming conventions and updated types.
  • Added NAMESPACE constant and updated contract retrieval functions in EternumProvider to use the namespace.
  • Introduced new type aliases ContractAddress and ID across multiple files for better type safety.
  • Updated hooks and components to utilize the new type aliases.
  • Removed unused functions to clean up the codebase.

Changes walkthrough 📝

Relevant files
Enhancement
7 files
contractComponents.ts
Refactor component definitions and update types                   

client/src/dojo/contractComponents.ts

  • Refactored component definitions to use new naming conventions.
  • Updated types for various components.
  • Removed old component definitions.
  • +561/-421
    index.ts
    Add namespace handling and update contract retrieval         

    sdk/packages/eternum/src/provider/index.ts

  • Added NAMESPACE constant.
  • Updated getContractByName function to use tag instead of name.
  • Modified transaction functions to include NAMESPACE.
  • +97/-103
    useStructures.tsx
    Add type aliases and update structure hooks                           

    client/src/hooks/helpers/useStructures.tsx

  • Added new type aliases ContractAddress and ID.
  • Updated functions to use new type aliases.
  • Removed unused useStructuresPosition function.
  • +27/-125
    useArmies.tsx
    Add type aliases and update army hooks                                     

    client/src/hooks/helpers/useArmies.tsx

  • Added new type aliases ContractAddress and ID.
  • Updated functions to use new type aliases.
  • Removed unused useArmiesByBattleId function.
  • +23/-67 
    useResources.tsx
    Add type aliases and update resource hooks                             

    client/src/hooks/helpers/useResources.tsx

  • Added new type aliases ContractAddress and ID.
  • Updated functions to use new type aliases.
  • +33/-51 
    useEntities.tsx
    Add type aliases and update entity hooks                                 

    client/src/hooks/helpers/useEntities.tsx

  • Added new type aliases ContractAddress and ID.
  • Updated functions to use new type aliases.
  • +32/-26 
    PillageHistory.tsx
    Add type alias and update pillage history component           

    client/src/ui/components/military/PillageHistory.tsx

  • Added new type alias ID.
  • Updated component to use new type alias.
  • +6/-6     
    Additional files (token-limit)
    108 files
    useQuests.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useQuests.tsx

    ...

    +37/-37 
    useRealm.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useRealm.tsx

    ...

    +40/-35 
    __mock__.ts
    ...                                                                                                           

    client/src/dojo/modelManager/tests/mock.ts

    ...

    +33/-26 
    BattleManager.test.ts
    ...                                                                                                           

    client/src/dojo/modelManager/tests/BattleManager.test.ts

    ...

    +18/-18 
    useGuilds.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useGuilds.tsx

    ...

    +19/-18 
    useBattles.tsx
    ...                                                                                                           

    client/src/hooks/helpers/battles/useBattles.tsx

    ...

    +7/-85   
    useLeaderBoardStore.tsx
    ...                                                                                                           

    client/src/hooks/store/useLeaderBoardStore.tsx

    ...

    +23/-20 
    useTrade.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useTrade.tsx

    ...

    +14/-16 
    ProductionManager.ts
    ...                                                                                                           

    client/src/dojo/modelManager/ProductionManager.ts

    ...

    +20/-18 
    _mapStore.tsx
    ...                                                                                                           

    client/src/hooks/store/_mapStore.tsx

    ...

    +12/-12 
    useHyperstructures.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useHyperstructures.tsx

    ...

    +16/-24 
    useStamina.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useStamina.tsx

    ...

    +12/-13 
    common.ts
    ...                                                                                                           

    sdk/packages/eternum/src/types/common.ts

    ...

    +41/-28 
    useBuildings.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useBuildings.tsx

    ...

    +21/-21 
    utils.tsx
    ...                                                                                                           

    client/src/ui/utils/utils.tsx

    ...

    +7/-23   
    BattleView.tsx
    ...                                                                                                           

    client/src/ui/modules/military/battle-view/BattleView.tsx

    ...

    +12/-14 
    BattleActions.tsx
    ...                                                                                                           

    client/src/ui/modules/military/battle-view/BattleActions.tsx

    ...

    +12/-11 
    BattleManager.ts
    ...                                                                                                           

    client/src/dojo/modelManager/BattleManager.ts

    ...

    +10/-10 
    GuildInvites.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/guilds/GuildInvites.tsx

    ...

    +13/-14 
    TopMiddleNavigation.tsx
    ...                                                                                                           

    client/src/ui/modules/navigation/TopMiddleNavigation.tsx

    ...

    +13/-12 
    BattleSideView.tsx
    ...                                                                                                           

    client/src/ui/modules/military/battle-view/BattleSideView.tsx

    ...

    +10/-10 
    MarketOrderPanel.tsx
    ...                                                                                                           

    client/src/ui/components/trading/MarketOrderPanel.tsx

    ...

    +9/-16   
    TransferBetweenEntities.tsx
    ...                                                                                                           

    client/src/ui/components/trading/TransferBetweenEntities.tsx

    ...

    +8/-14   
    Swap.tsx
    ...                                                                                                           

    client/src/ui/components/bank/Swap.tsx

    ...

    +6/-6     
    WorldStructuresMenu.tsx
    ...                                                                                                           

    client/src/ui/modules/world-structures/WorldStructuresMenu.tsx

    ...

    +7/-23   
    SelectPreviewBuilding.tsx
    ...                                                                                                           

    client/src/ui/components/construction/SelectPreviewBuilding.tsx

    ...

    +7/-6     
    StructureCard.tsx
    ...                                                                                                           

    client/src/ui/components/hyperstructures/StructureCard.tsx

    ...

    +8/-8     
    LiquidityResourceRow.tsx
    ...                                                                                                           

    client/src/ui/components/bank/LiquidityResourceRow.tsx

    ...

    +14/-14 
    TradeHistoryEvent.tsx
    ...                                                                                                           

    client/src/ui/components/trading/TradeHistoryEvent.tsx

    ...

    +6/-25   
    AddLiquidity.tsx
    ...                                                                                                           

    client/src/ui/components/bank/AddLiquidity.tsx

    ...

    +6/-6     
    MarketManager.ts
    ...                                                                                                           

    client/src/dojo/modelManager/MarketManager.ts

    ...

    +13/-10 
    realms.tsx
    ...                                                                                                           

    client/src/ui/utils/realms.tsx

    ...

    +13/-17 
    Guilds.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/guilds/Guilds.tsx

    ...

    +6/-5     
    useTravel.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useTravel.tsx

    ...

    +7/-7     
    SelectLocationPanel.tsx
    ...                                                                                                           

    client/src/ui/components/entities/SelectLocationPanel.tsx

    ...

    +6/-6     
    useRealmStore.tsx
    ...                                                                                                           

    client/src/hooks/store/useRealmStore.tsx

    ...

    +12/-11 
    EntityDetails.tsx
    ...                                                                                                           

    client/src/ui/modules/entity-details/EntityDetails.tsx

    ...

    +6/-6     
    createUpdates.ts
    ...                                                                                                           

    client/src/dojo/createUpdates.ts

    ...

    +7/-10   
    Entity.tsx
    ...                                                                                                           

    client/src/ui/components/entities/Entity.tsx

    ...

    +5/-5     
    GuildMembers.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/guilds/GuildMembers.tsx

    ...

    +16/-9   
    ResourceBar.tsx
    ...                                                                                                           

    client/src/ui/components/bank/ResourceBar.tsx

    ...

    +6/-6     
    MarketModal.tsx
    ...                                                                                                           

    client/src/ui/components/trading/MarketModal.tsx

    ...

    +4/-4     
    useExplore.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useExplore.tsx

    ...

    +5/-4     
    ArmyManagementCard.tsx
    ...                                                                                                           

    client/src/ui/components/military/ArmyManagementCard.tsx

    ...

    +4/-5     
    MarketTradingHistory.tsx
    ...                                                                                                           

    client/src/ui/components/trading/MarketTradingHistory.tsx

    ...

    +9/-9     
    ExistingBuildings.tsx
    ...                                                                                                           

    client/src/ui/components/construction/ExistingBuildings.tsx

    ...

    +4/-4     
    Battle.tsx
    ...                                                                                                           

    client/src/ui/modules/military/battle-view/Battle.tsx

    ...

    +4/-4     
    Steps.tsx
    ...                                                                                                           

    client/src/ui/modules/onboarding/Steps.tsx

    ...

    +3/-4     
    ArmyInfoLabel.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/armies/ArmyInfoLabel.tsx

    ...

    +3/-3     
    DepositResources.tsx
    ...                                                                                                           

    client/src/ui/components/resources/DepositResources.tsx

    ...

    +4/-4     
    useMarketStore.tsx
    ...                                                                                                           

    client/src/hooks/store/useMarketStore.tsx

    ...

    +3/-3     
    StaminaResource.tsx
    ...                                                                                                           

    client/src/ui/elements/StaminaResource.tsx

    ...

    +4/-4     
    useEventHandlers.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/hexagon/useEventHandlers.tsx

    ...

    +4/-3     
    useContributions.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useContributions.tsx

    ...

    +5/-4     
    ArmyList.tsx
    ...                                                                                                           

    client/src/ui/components/military/ArmyList.tsx

    ...

    +4/-4     
    Structures.tsx
    ...                                                                                                           

    client/src/ui/components/models/buildings/worldmap/Structures.tsx

    ...

    +3/-3     
    StructureListItem.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/structures/StructureListItem.tsx

    ...

    +4/-4     
    MyGuild.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/guilds/MyGuild.tsx

    ...

    +6/-6     
    useBanks.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useBanks.tsx

    ...

    +3/-3     
    useCaravans.tsx
    ...                                                                                                           

    client/src/hooks/helpers/useCaravans.tsx

    ...

    +4/-3     
    QuestPanel.tsx
    ...                                                                                                           

    client/src/ui/components/quest/QuestPanel.tsx

    ...

    +4/-4     
    QuestList.tsx
    ...                                                                                                           

    client/src/ui/components/quest/QuestList.tsx

    ...

    +3/-2     
    ResourceChip.tsx
    ...                                                                                                           

    client/src/ui/components/resources/ResourceChip.tsx

    ...

    +6/-6     
    setupNetwork.ts
    ...                                                                                                           

    client/src/dojo/setupNetwork.ts

    ...

    +4/-4     
    StructureConstructionMenu.tsx
    ...                                                                                                           

    client/src/ui/components/structures/construction/StructureConstructionMenu.tsx

    ...

    +3/-2     
    StaminaResourceCost.tsx
    ...                                                                                                           

    client/src/ui/elements/StaminaResourceCost.tsx

    ...

    +3/-3     
    hyperstructureEventQueries.ts
    ...                                                                                                           

    client/src/dojo/events/hyperstructureEventQueries.ts

    ...

    +3/-3     
    TravelEntityPopup.tsx
    ...                                                                                                           

    client/src/ui/components/entities/TravelEntityPopup.tsx

    ...

    +3/-2     
    EntitiesArmyTable.tsx
    ...                                                                                                           

    client/src/ui/components/military/EntitiesArmyTable.tsx

    ...

    +2/-5     
    useTravelPath.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/hexagon/useTravelPath.tsx

    ...

    +2/-2     
    ResourceWeight.tsx
    ...                                                                                                           

    client/src/ui/components/resources/ResourceWeight.tsx

    ...

    +2/-2     
    Settings.tsx
    ...                                                                                                           

    client/src/ui/modules/settings/Settings.tsx

    ...

    +2/-1     
    HyperstructureResourceChip.tsx
    ...                                                                                                           

    client/src/ui/components/hyperstructures/HyperstructureResourceChip.tsx

    ...

    +2/-2     
    InventoryResources.tsx
    ...                                                                                                           

    client/src/ui/components/resources/InventoryResources.tsx

    ...

    +2/-2     
    Military.tsx
    ...                                                                                                           

    client/src/ui/modules/military/Military.tsx

    ...

    +2/-1     
    useUIStore.tsx
    ...                                                                                                           

    client/src/hooks/store/useUIStore.tsx

    ...

    +1/-1     
    BattleListItem.tsx
    ...                                                                                                           

    client/src/ui/components/battles/BattleListItem.tsx

    ...

    +3/-1     
    LockedResources.tsx
    ...                                                                                                           

    client/src/ui/modules/military/battle-view/LockedResources.tsx

    ...

    +3/-2     
    Questing.tsx
    ...                                                                                                           

    client/src/ui/modules/questing/Questing.tsx

    ...

    +2/-1     
    useBlockchainStore.tsx
    ...                                                                                                           

    client/src/hooks/store/useBlockchainStore.tsx

    ...

    +2/-2     
    EntityList.tsx
    ...                                                                                                           

    client/src/ui/components/list/EntityList.tsx

    ...

    +2/-1     
    ResourceArrivals.tsx
    ...                                                                                                           

    client/src/ui/components/trading/ResourceArrivals.tsx

    ...

    +2/-1     
    pillageEventQueries.ts
    ...                                                                                                           

    client/src/dojo/events/pillageEventQueries.ts

    ...

    +2/-2     
    QuestInfo.tsx
    ...                                                                                                           

    client/src/ui/components/quest/QuestInfo.tsx

    ...

    +2/-1     
    PlayersLeaderboard.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/leaderboard/PlayersLeaderboard.tsx

    ...

    +1/-1     
    LiquidityTable.tsx
    ...                                                                                                           

    client/src/ui/components/bank/LiquidityTable.tsx

    ...

    +3/-3     
    HyperstructurePanel.tsx
    ...                                                                                                           

    client/src/ui/components/hyperstructures/HyperstructurePanel.tsx

    ...

    +1/-1     
    BattleDetails.tsx
    ...                                                                                                           

    client/src/ui/modules/military/battle-view/BattleDetails.tsx

    ...

    +1/-1     
    useBattles.test.tsx
    ...                                                                                                           

    client/src/hooks/helpers/battles/test/useBattles.test.tsx

    ...

    +1/-1     
    index.ts
    ...                                                                                                           

    client/src/types/index.ts

    ...

    +2/-3     
    EntityResourceTable.tsx
    ...                                                                                                           

    client/src/ui/components/resources/EntityResourceTable.tsx

    ...

    +2/-2     
    ArmyFlag.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/armies/ArmyFlag.tsx

    ...

    +1/-1     
    Whitelist.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/guilds/Whitelist.tsx

    ...

    +2/-1     
    utils.tsx
    ...                                                                                                           

    client/src/ui/components/worldmap/guilds/utils.tsx

    ...

    +4/-2     
    ArmyChip.tsx
    ...                                                                                                           

    client/src/ui/components/military/ArmyChip.tsx

    ...

    +1/-1     
    dojoConfig.ts
    ...                                                                                                           

    client/dojoConfig.ts

    ...

    +2/-2     
    types.ts
    ...                                                                                                           

    client/src/hooks/store/types.ts

    ...

    +5/-3     
    index.ts
    ...                                                                                                           

    config/index.ts

    ...

    +2/-2     
    structures.ts
    ...                                                                                                           

    sdk/packages/eternum/src/constants/structures.ts

    ...

    +1/-1     
    contracts.sh
    ...                                                                                                           

    contracts/scripts/contracts.sh

    ...

    +30/-22 
    set_writer.sh
    ...                                                                                                           

    contracts/scripts/set_writer.sh

    ...

    +1/-1     
    pnpm-lock.yaml
    ...                                                                                                           

    pnpm-lock.yaml

    ...

    +1141/-2188
    manifest.toml
    ...                                                                                                           

    contracts/manifests/prod/deployment/manifest.toml

    ...

    +2152/-0
    package.json
    ...                                                                                                           

    client/package.json

    ...

    +6/-6     
    manifest.toml
    ...                                                                                                           

    contracts/manifests/dev/deployment/manifest.toml

    ...

    +1/-1     
    manifest.json
    ...                                                                                                           

    contracts/manifests/dev/deployment/manifest.json

    ...

    +1/-1     
    package.json
    ...                                                                                                           

    config/package.json

    ...

    +1/-1     
    package.json
    ...                                                                                                           

    sdk/packages/eternum/package.json

    ...

    +1/-1     

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

    Copy link

    vercel bot commented Jul 30, 2024

    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 31, 2024 3:25pm

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Type Consistency
    The types used in the component definitions have been changed, e.g., from RecsType.BigInt to RecsType.Number for entity_id. Ensure that these changes are consistent with the expected data types in the system to avoid runtime type errors.

    Metadata Updates
    The metadata for components such as 'Structure' and 'Progress' have been removed. If these components are still in use, ensure that removing metadata does not affect system functionality.

    Namespace Usage
    The introduction of the NAMESPACE constant for contract names is a significant change. Verify that all parts of the system that interact with contract names handle this new format correctly, especially if they construct contract names dynamically or use them in comparisons.

    Type Alias Usage
    The new type alias ContractAddress is used for the address parameter in removeGuildMember. Ensure that all calling contexts and the function implementation are updated to handle this type correctly.

    Copy link

    github-actions bot commented Jul 30, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Possible bug
    Update test assertions to reflect the property name changes in the BattleManager class

    Update the expect statement to check for the correct property battleEntityId instead
    of battleId to align with the changes made in the BattleManager class.

    client/src/dojo/modelManager/tests/BattleManager.test.ts [56]

    -expect(battleManager.battleId).toBe(BATTLE_ENTITY_ID);
    +expect(battleManager.battleEntityId).toBe(BATTLE_ENTITY_ID);
     
    Suggestion importance[1-10]: 10

    Why: This suggestion addresses a critical issue by updating the test assertion to match the new property name, ensuring the test correctly verifies the intended functionality.

    10
    Fix the typo in the method name to prevent runtime errors

    Correct the typo in the method name from getUdpdatedHealth to getUpdatedHealth to
    ensure the method is correctly referenced and can be invoked without errors.

    client/src/dojo/modelManager/tests/BattleManager.test.ts [162]

    -const updatedHealth = battleManager["getUdpdatedHealth"](1n, health, 0);
    +const updatedHealth = battleManager["getUpdatedHealth"](1n, health, 0);
     
    Suggestion importance[1-10]: 10

    Why: Correcting the typo in the method name is crucial to ensure the method is invoked correctly, preventing runtime errors and ensuring the test functions as intended.

    10
    Add safe type conversion for realmId to prevent potential runtime errors

    Replace the direct conversion of realmId from ID to Number with a safer
    type-checking mechanism to prevent runtime errors if realmId is not a valid number.

    client/src/dojo/events/pillageEventQueries.ts [8]

    -events(keys: ["${PILLAGE_EVENT}","*","${numberToHex(Number(realmId))}"]) {
    +events(keys: ["${PILLAGE_EVENT}","*","${numberToHex(safeConvertToNumber(realmId))}"]) {
     
    Suggestion importance[1-10]: 8

    Why: Adding a safe type conversion for realmId can prevent potential runtime errors, which is a significant improvement for robustness. However, the suggested function safeConvertToNumber is not defined in the provided code.

    8
    Add null safety checks and ensure consistent BigInt conversion

    Ensure that the entity_owner_id is properly converted to a BigInt before using it in
    getEntityIdFromKeys to maintain type consistency and avoid potential runtime errors.

    client/src/hooks/helpers/useArmies.tsx [72]

    -owner = getComponentValue(Owner, getEntityIdFromKeys([BigInt(entityOwner.entity_owner_id)]));
    +owner = getComponentValue(Owner, getEntityIdFromKeys([BigInt(entityOwner?.entity_owner_id || '0')]));
     
    Suggestion importance[1-10]: 8

    Why: Adding a null safety check and ensuring consistent BigInt conversion improves robustness and prevents potential runtime errors.

    8
    Best practice
    Replace RecsType.String with an enum type for battle_side to enforce valid values

    The battle_side field in multiple components such as Army and Battle is defined as
    RecsType.String. If this field has a limited set of possible values (e.g.,
    "attacker", "defender"), consider using an enumeration type instead of a string to
    enforce valid values and improve code clarity.

    client/src/dojo/contractComponents.ts [27]

    -battle_side: RecsType.String
    +battle_side: BattleSide // Assuming BattleSide is an enum with valid sides
     
    Suggestion importance[1-10]: 9

    Why: Using an enumeration type for battle_side instead of a string is a best practice that enforces valid values and improves code clarity. This change can prevent errors and make the code more maintainable.

    9
    Ensure robust handling of optional BigInt values by providing a default value

    Use a consistent method for handling optional BigInt values by providing a default
    value of '0' when the optional chain might result in undefined.

    client/src/hooks/helpers/useArmies.tsx [580]

    -const owner = getComponentValue(Owner, getEntityIdFromKeys([BigInt(entityOwner?.entity_owner_id || 0)]));
    +const owner = getComponentValue(Owner, getEntityIdFromKeys([BigInt(entityOwner?.entity_owner_id || '0')]));
     
    Suggestion importance[1-10]: 8

    Why: Ensuring robust handling of optional BigInt values by providing a default value is a good practice and prevents potential runtime errors.

    8
    Improve type safety by using a more specific type for entityId

    Consider using a more specific type than ID for entityId in createCombatEvents to
    ensure type safety and avoid potential runtime errors due to incorrect type
    assumptions.

    client/src/dojo/createUpdates.ts [16-17]

    -createCombatEvents: async (entityId: ID) =>
    +createCombatEvents: async (entityId: SpecificEntityType) =>
         createEventSubscription([COMBAT_EVENT, "*", numberToHex(Number(entityId)), "*"]),
     
    Suggestion importance[1-10]: 7

    Why: Using a more specific type can improve type safety and prevent potential runtime errors. However, the suggestion assumes the existence of a more specific type, which may not be defined in the current context.

    7
    Performance
    Avoid unnecessary type conversion in _inputs_available

    Refactor the method _inputs_available to directly use resourceId without converting
    it to a number, to avoid unnecessary type conversions and potential errors.

    client/src/dojo/modelManager/ProductionManager.ts [203]

    -const inputs = RESOURCE_INPUTS_SCALED[resourceId];
    +const inputs = RESOURCE_INPUTS_SCALED[resourceId as keyof typeof RESOURCE_INPUTS_SCALED];
     
    Suggestion importance[1-10]: 9

    Why: Avoiding unnecessary type conversions can improve performance and reduce the risk of errors. The suggested change is a good practice for maintaining type consistency.

    9
    Enhancement
    Use RecsType.UInt for troop counts in the Army component to enforce non-negative values

    The troops object in Army component is using RecsType.BigInt for knight_count,
    paladin_count, and crossbowman_count. If these fields represent counts that can be
    large but are always non-negative, using RecsType.UInt might be more appropriate to
    enforce non-negative values and potentially reduce storage requirements.

    client/src/dojo/contractComponents.ts [25]

    -troops: { knight_count: RecsType.BigInt, paladin_count: RecsType.BigInt, crossbowman_count: RecsType.BigInt }
    +troops: { knight_count: RecsType.UInt, paladin_count: RecsType.UInt, crossbowman_count: RecsType.UInt }
     
    Suggestion importance[1-10]: 8

    Why: Using RecsType.UInt for troop counts is a good enhancement as it enforces non-negative values and can potentially reduce storage requirements. This change improves data integrity and efficiency.

    8
    Consistency
    Ensure consistent use of battle entity ID across all test cases

    Replace the hardcoded 0 in the BattleManager constructor with BATTLE_ENTITY_ID to
    ensure consistency and correct referencing of the battle entity ID across all test
    cases.

    client/src/dojo/modelManager/tests/BattleManager.test.ts [50]

    -const battleManager = new BattleManager(0, mockDojoResult);
    +const battleManager = new BattleManager(BATTLE_ENTITY_ID, mockDojoResult);
     
    Suggestion importance[1-10]: 8

    Why: This suggestion improves consistency and correctness by using the same battle entity ID across all test cases, which is important for maintaining reliable and understandable tests.

    8
    Use consistent entity ID references in the BattleManager constructor

    Replace the use of 0 with BATTLE_ENTITY_ID in the BattleManager constructor to
    maintain consistency with the updated entity ID handling.

    client/src/dojo/modelManager/tests/BattleManager.test.ts [149]

    -const battleManager = new BattleManager(0, mockDojoResult);
    +const battleManager = new BattleManager(BATTLE_ENTITY_ID, mockDojoResult);
     
    Suggestion importance[1-10]: 8

    Why: This suggestion enhances consistency in the code by using the same entity ID reference, which helps in maintaining clarity and reducing potential errors.

    8
    Possible issue
    Change the data types of address and name in AddressName to more appropriate types

    The component AddressName uses RecsType.BigInt for both address and name fields,
    which might not be appropriate if these fields are expected to hold different types
    of data or if they are not large integers. Consider using more suitable types for
    these fields to ensure data integrity and to avoid potential runtime errors or
    misinterpretations of the data types.

    client/src/dojo/contractComponents.ts [10]

     {
    -  address: RecsType.BigInt,
    -  name: RecsType.BigInt
    +  address: RecsType.Address, // Assuming Address is a valid type for addresses
    +  name: RecsType.String     // Assuming name should be a string
     }
     
    Suggestion importance[1-10]: 7

    Why: The suggestion to change the data types of address and name to more appropriate types is valid and can help ensure data integrity. However, the exact types suggested (RecsType.Address and RecsType.String) are assumptions and may not be correct without further context.

    7
    Use RecsType.BigInt for entity_id to accommodate potentially large values

    The entity_id fields in components like Army and Building are using RecsType.Number.
    If these IDs are meant to be unique identifiers that could grow large, consider
    using RecsType.BigInt to ensure they can accommodate very large values.

    client/src/dojo/contractComponents.ts [24]

    -entity_id: RecsType.Number
    +entity_id: RecsType.BigInt
     
    Suggestion importance[1-10]: 7

    Why: Changing entity_id to RecsType.BigInt can be beneficial if the IDs are expected to grow large. However, this change should be carefully considered based on the actual requirements and constraints of the application.

    7

    …ns + fix z-index of quests, guilds and leaderboard
    client/src/hooks/helpers/useBuildings.tsx Outdated Show resolved Hide resolved
    client/src/hooks/helpers/useBuildings.tsx Outdated Show resolved Hide resolved
    client/src/hooks/helpers/useGuilds.tsx Outdated Show resolved Hide resolved
    client/src/hooks/helpers/useGuilds.tsx Outdated Show resolved Hide resolved
    client/src/hooks/helpers/useQuests.tsx Outdated Show resolved Hide resolved
    client/src/hooks/helpers/useQuests.tsx Outdated Show resolved Hide resolved
    client/src/hooks/helpers/useStamina.tsx Outdated Show resolved Hide resolved
    client/src/hooks/store/types.ts Outdated Show resolved Hide resolved
    @edisontim edisontim merged commit ddf48dd into v1 Jul 31, 2024
    20 of 21 checks passed
    @edisontim edisontim deleted the feat/v1-client branch July 31, 2024 16:46
    edisontim added a commit that referenced this pull request Aug 1, 2024
    * contract: limit armies and enable deletion
    
    * contracts: delete army if merging kills troop
    
    * upgrade to dojov1.0.0-alpha.1
    
    * upgrade to dojov1.0.0-alpha.1
    
    * scarb fmt
    
    * update tests
    
    * update tests && scarb fmt
    
    * update test utils
    
    * update test utils
    
    * update test utils
    
    * update test utils
    
    * fix inaccurate winner invariant error
    
    * add tool-versions
    
    * contracts: basic army create tests
    
    * contracts: basic army buy tests
    
    * fix reward calculation bug
    
    * more combat tests
    
    * combat: reward bug fix
    
    * scarb fmt
    
    * Merge branch 'v1' of https://github.com/BibliothecaDAO/eternum into combat-tests
    
    * update workflow
    
    * update lockfile
    
    * update workflow
    
    * update workflow
    
    * update lockfile
    
    * update lock file
    
    * bump to alpha 2
    
    * update to cairo 2.7.0
    
    * contracts: update workflow to print resource usage
    
    * update workflow
    
    * contracts: use u32 as primary id
    
    * contracts: use u32 as primary id in tests
    
    * update config and manifests
    
    * contracts: optimization: use IntrospectPacked
    
    * - remove introspectpacked from models with enum
    - use id alias more
    
    * Clean/legacy (#1146)
    
    * Clean legacy code + remove unused exports
    
    * add knip command
    
    * more cleaning
    
    * prettier
    
    * Cleanup + add knip CI
    
    * fix build
    
    * fix knip
    
    * rebase main
    
    * prettier
    
    * fix knip ci
    
    * resolve comments
    
    * feat: Change to V1 + add type aliases for entity ids (#1148)
    
    * Change to V1 + add type aliases for entity ids
    
    * Make elements clickable under right and left navigation modules buttons + fix z-index of quests, guilds and leaderboard
    
    * scarb fmt
    
    * Change bonus_percent to u32 instead of u128
    
    * Address minor PR comments
    
    * prettier
    
    * knip
    
    ---------
    
    Co-authored-by: ponderingdemocritus <[email protected]>
    Co-authored-by: tedison <[email protected]>
    Co-authored-by: Constantin Wastchenko <[email protected]>
    Co-authored-by: tedison <[email protected]>
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    2 participants