Closed aymericdelab closed 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 9:31pm |
โฑ๏ธ Estimated effort to review [1-5] | 2 |
๐งช Relevant tests | No |
๐ Security concerns | No |
โก Key issues to review |
Data Consistency: Ensure that the increase in quest rewards from 1 to 5 and the change from ResourcesIds.Wheat to ResourcesIds.Earthenshard are consistent with the game's balance and intended resource availability. |
Indentation Changes: Verify that the changes in indentation in the getLevelingCost function do not affect the execution or readability of the code in a negative way. |
Category | Suggestion | Score |
Maintainability |
Refactor repeated resource entries using a loop to enhance maintainability___ **Consider using a loop or a map function to generate the repeated resource entries fordifferent quest types. This will make the code more maintainable and easier to update in the future.** [sdk/packages/eternum/src/constants/resources.ts [893-899]](https://github.com/BibliothecaDAO/eternum/pull/1056/files#diff-03877f9a7de964bc35d454531a120cf998aca1a8b8ebd1d0471d35393c38ac4aR893-R899) ```diff -[QuestType.Travel]: [{ resource: ResourcesIds.Earthenshard, amount: 5 }], -[QuestType.Population]: [{ resource: ResourcesIds.Earthenshard, amount: 5 }], -[QuestType.Market]: [{ resource: ResourcesIds.Earthenshard, amount: 5 }], -[QuestType.Mine]: [{ resource: ResourcesIds.Earthenshard, amount: 5 }], -[QuestType.Pillage]: [{ resource: ResourcesIds.Earthenshard, amount: 5 }], -[QuestType.Hyperstructure]: [{ resource: ResourcesIds.Earthenshard, amount: 5 }], -[QuestType.Contribution]: [{ resource: ResourcesIds.Earthenshard, amount: 5 }] +const questTypes = [QuestType.Travel, QuestType.Population, QuestType.Market, QuestType.Mine, QuestType.Pillage, QuestType.Hyperstructure, QuestType.Contribution]; +questTypes.forEach(type => { + QUEST_RESOURCES[type] = [{ resource: ResourcesIds.Earthenshard, amount: 5 }]; +}); ``` Suggestion importance[1-10]: 9Why: This suggestion significantly improves code maintainability by reducing redundancy and making future updates easier. The use of a loop to handle repeated resource entries is a good practice. | 9 |
Possible issue |
Add validation to ensure the integrity of resource IDs and amounts___ **Ensure that the resource IDs and amounts are correctly mapped and validated to preventpotential runtime errors due to incorrect data.** [sdk/packages/eternum/src/constants/resources.ts [398]](https://github.com/BibliothecaDAO/eternum/pull/1056/files#diff-03877f9a7de964bc35d454531a120cf998aca1a8b8ebd1d0471d35393c38ac4aR398-R398) ```diff -[1, 756000, 2, 594097, 3, 577816, 4, 398426, 5, 334057, 6, 262452, 7, 177732] +[1, 756000, 2, 594097, 3, 577816, 4, 398426, 5, 334057, 6, 262452, 7, 177732].map((id, index) => { + return { resourceId: id, amount: index % 2 === 0 ? null : id }; +}) ``` Suggestion importance[1-10]: 3Why: While the suggestion aims to add validation, the provided improved code introduces a logical error by setting the amount to null for even indices. This does not align with the original code's intent and could cause runtime issues. | 3 |
PR Type
Enhancement
Description
getLevelingCost
function for better readability.ResourcesIds.Wheat
toResourcesIds.Earthenshard
and increased the amount from 1 to 5.Changes walkthrough ๐
resources.ts
Update quest rewards and adjust resource tier indentation
sdk/packages/eternum/src/constants/resources.ts
ResourcesIds.Wheat
toResourcesIds.Earthenshard
and increased the amount.