Enh/modular config

[cwastche]

User description

Thought: Should we make a separate manager for the client that fetches config values onchain directly ?

---

PR Type

enhancement, configuration changes

---

Description

• Implemented ConfigManager class for centralized configuration management across the project.
• Replaced direct usage of configuration constants with ConfigManager instance in various components and hooks.
• Added new configuration methods and constants to support the new ConfigManager.
• Updated configuration-related logic in UI components, hooks, and provider.
• Removed redundant configuration functions and constants.

---

Changes walkthrough 📝

	Relevant files
Enhancement	15 files index.ts Implemented `ConfigManager` for centralized configuration management sdk/packages/eternum/src/config/index.ts Introduced ConfigManager class for managing configuration. Added methods to handle various configurations like production, building, population, and more. Removed old configuration functions. +358/-253 resources.ts Refactored resource constants and added `ResourceMultipliers` sdk/packages/eternum/src/constants/resources.ts Removed redundant constants and functions. Added ResourceMultipliers constant. +28/-90  SelectPreviewBuilding.tsx Integrated `ConfigManager` in SelectPreviewBuilding component client/src/ui/components/construction/SelectPreviewBuilding.tsx Replaced direct config usage with ConfigManager instance. Updated building cost and population checks. +36/-25  useLeaderBoardStore.tsx Integrated `ConfigManager` in LeaderBoard store                    client/src/hooks/store/useLeaderBoardStore.tsx Replaced direct config usage with ConfigManager instance. Updated resource multiplier and contribution points calculation. +16/-48  StructureConstructionMenu.tsx Integrated `ConfigManager` in StructureConstructionMenu component client/src/ui/components/structures/construction/StructureConstructionMenu.tsx Replaced direct config usage with ConfigManager instance. Updated structure cost and population checks. +23/-23  Combat.tsx Integrated `ConfigManager` in Combat hints component          client/src/ui/components/hints/Combat.tsx Replaced direct config usage with ConfigManager instance. Updated troop configuration display. +16/-25  index.ts Cleaned up utility functions and imports                                  sdk/packages/eternum/src/utils/index.ts Removed redundant imports and constants. Simplified utility functions. +2/-86    global.ts Updated global configuration constants                                      sdk/packages/eternum/src/constants/global.ts Consolidated global configuration constants. Added new constants for ConfigManager. +40/-21  HyperstructurePanel.tsx Integrated `ConfigManager` in HyperstructurePanel component client/src/ui/components/hyperstructures/HyperstructurePanel.tsx Replaced direct config usage with ConfigManager instance. Updated hyperstructure cost and points display. +9/-9      TopMiddleNavigation.tsx Integrated `ConfigManager` in TopMiddleNavigation component client/src/ui/modules/navigation/TopMiddleNavigation.tsx Replaced direct config usage with ConfigManager instance. Updated population and tick interval display. +10/-7    index.ts Added travel stamina cost configuration method in provider sdk/packages/eternum/src/provider/index.ts Added method to set travel stamina cost configuration. Removed commented-out code. +10/-25  useRealm.tsx Integrated `ConfigManager` in useRealm hook                            client/src/hooks/helpers/useRealm.tsx Replaced direct config usage with ConfigManager instance. Updated quest resources retrieval. +7/-4      WorldStructures.tsx Integrated `ConfigManager` in WorldStructures hints component client/src/ui/components/hints/WorldStructures.tsx Replaced direct config usage with ConfigManager instance. Updated hyperstructure creation and construction cost display. +14/-13  config.ts Added `EternumConfig` interface                                                    sdk/packages/eternum/src/types/config.ts Added new EternumConfig interface for configuration management. +78/-0    TheMap.tsx Integrated `ConfigManager` in TheMap hints component          client/src/ui/components/hints/TheMap.tsx Replaced direct config usage with ConfigManager instance. Updated exploration and travel cost display. +8/-11
Additional files (token-limit)	2 files stamina.cairo ...                                                                                                            contracts/src/models/stamina.cairo ... +4/-5      contracts.cairo ...                                                                                                            contracts/src/systems/map/contracts.cairo ... +3/-2

---

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

[vercel]

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	Aug 24, 2024 7:51pm

[github-actions]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Singleton Pattern The ConfigManager class is implemented as a singleton. This pattern can lead to issues in testing and may hinder scalability. Consider if a singleton is necessary or if another design pattern could be more appropriate. Logging The use of console.log for logging instance creation and existence checks (lines 19 and 22) might not be suitable for production environments. Consider implementing a more robust logging approach or using environment-based logging levels. Error Handling The method setConfigs (lines 77-91) involves multiple asynchronous operations without specific error handling. Consider adding try-catch blocks or specific error handling for each asynchronous operation to manage exceptions more gracefully.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add safety check for array access to prevent runtime errors Ensure that the buildingPopulation array access is safe by checking if the building index exists in the array to avoid potential runtime errors. client/src/ui/utils/realms.tsx [95] -return (realm?.population || 0) + buildingPopulation[building] <= basePopulationCapacity + (realm?.capacity || 0); +return (realm?.population || 0) + (buildingPopulation[building] ?? 0) <= basePopulationCapacity + (realm?.capacity || 0); Suggestion importance[1-10]: 10 Why: This suggestion prevents potential runtime errors by ensuring safe access to the buildingPopulation array, which is crucial for application stability.	10
	Add error handling for JSON parsing to prevent application crashes Add error handling for JSON parsing to catch and handle potential exceptions from malformed JSON data. config/index.ts [30] -configData = JSON.parse(rawData); +try { + configData = JSON.parse(rawData); +} catch (parseError) { + console.error(`Error parsing JSON data: ${parseError}`); + configData = {}; +} Suggestion importance[1-10]: 10 Why: Adding error handling for JSON parsing is essential to prevent the application from crashing due to malformed JSON data, significantly improving the robustness of the code.	10
	Add nullish coalescing to provide default values and prevent runtime errors Consider using a nullish coalescing operator (??) when accessing properties from the configManager to provide default values in case properties are undefined. This can prevent runtime errors due to undefined values. client/src/ui/components/worldmap/structures/StructureListItem.tsx [17-19] -const hyperstructurePointsPerCycle = configManager.getConfig().hyperstructurePointsPerCycle; -const resourceOutputsScaled = configManager.getResourceOutputsScaled(); -const resourcePrecision = configManager.getResourcePrecision(); +const hyperstructurePointsPerCycle = configManager.getConfig().hyperstructurePointsPerCycle ?? 0; +const resourceOutputsScaled = configManager.getResourceOutputsScaled() ?? {}; +const resourcePrecision = configManager.getResourcePrecision() ?? 1; Suggestion importance[1-10]: 9 Why: The suggestion addresses a potential runtime error by providing default values using the nullish coalescing operator. This is a significant improvement for code robustness.	9
	Add a check to ensure resourceInputsScaled[resourceId] is an array before spreading Ensure that resourceInputsScaled[resourceId] is an array before spreading it to avoid runtime errors. client/src/ui/components/construction/SelectPreviewBuilding.tsx [109] -const cost = [...buildingCostsScaled[BuildingType.Resource], ...resourceInputsScaled[resourceId]]; +const resourceCosts = Array.isArray(resourceInputsScaled[resourceId]) ? resourceInputsScaled[resourceId] : []; +const cost = [...buildingCostsScaled[BuildingType.Resource], ...resourceCosts]; Suggestion importance[1-10]: 8 Why: Ensuring that resourceInputsScaled[resourceId] is an array before spreading it prevents potential runtime errors, making the code more robust and reliable.	8
	Round the result to avoid precision issues To avoid potential issues with floating-point precision, consider rounding the result of the division operation when calculating the time in minutes from seconds. client/src/ui/components/hints/Combat.tsx [13] -armiesTickIntervalInSeconds / 60 +Math.round(armiesTickIntervalInSeconds / 60) Suggestion importance[1-10]: 8 Why: Rounding the result of the division operation when calculating time in minutes from seconds helps avoid potential floating-point precision issues, which is a significant improvement.	8
	Refactor percentage calculation to ensure accurate and isolated updates within the map function Refactor the calculation of percentage to ensure it is updated correctly within the map function and not outside, to avoid potential bugs with asynchronous state updates. client/src/hooks/helpers/useHyperstructures.tsx [108-119] const allProgresses = hyperstructure_total_costs_scaled.map(({ resource, amount: resourceCost }) => { let foundProgress = progresses.find((progress) => progress!.resource_type === resource); let progress = { hyperstructure_entity_id: hyperstructureEntityId, amount: foundProgress ? foundProgress.amount : 0, percentage: foundProgress ? Math.floor((foundProgress.amount / EternumGlobalConfig.resources.resourcePrecision / resourceCost!) * 100) : 0, costNeeded: resourceCost, }; - percentage += (progress.amount * resourceMultipliers[progress.resource_type]) / totalContributableAmount; - return progress; + return { + ...progress, + percentageContribution: (progress.amount * resourceMultipliers[progress.resource_type]) / totalContributableAmount + }; }); +const totalPercentage = allProgresses.reduce((acc, curr) => acc + curr.percentageContribution, 0); Suggestion importance[1-10]: 7 Why: The refactoring suggestion ensures that the percentage calculation is accurate and isolated, which can prevent potential bugs with asynchronous state updates. However, the original code does not necessarily indicate a bug, so the improvement is more about code robustness.	7
	Ensure correct typing for loop variable Ensure that the resourceId is properly typed as ResourcesIds in the loop to avoid potential runtime errors due to incorrect types. client/src/ui/components/hints/Resources.tsx [61] -for (const resourceId of Object.keys(resourceInputsScaled) as unknown as ResourcesIds[]) +for (const resourceId of Object.keys(resourceInputsScaled) as ResourcesIds[]) Suggestion importance[1-10]: 7 Why: Ensuring the correct typing for the loop variable resourceId as ResourcesIds[] helps prevent potential runtime errors, improving code reliability.	7
Performance	Improve performance and error handling by using asynchronous file reading and JSON parsing Consider using asynchronous file reading and JSON parsing to avoid blocking the main thread. This can be achieved using fs.promises.readFile and then parsing the JSON in an asynchronous manner. client/src/App.tsx [14-16] -const rawData = fs.readFileSync(configPath, "utf-8"); -if (rawData.trim()) { - configData = JSON.parse(rawData); -} +fs.promises.readFile(configPath, "utf-8") + .then(rawData => { + if (rawData.trim()) { + return JSON.parse(rawData); + } + return {}; + }) + .then(parsedData => { + configData = parsedData; + }) + .catch(error => { + console.warn(`Failed to load config: ${error}`); + }); Suggestion importance[1-10]: 9 Why: The suggestion to use asynchronous file reading and JSON parsing is valid and improves performance by avoiding blocking the main thread. It also provides better error handling with promises.	9
Possible issue	Ensure correct logging for singleton instance creation To ensure that the singleton pattern is correctly implemented, the log statement "Instance already exists" should only be executed if the instance actually exists. This can be achieved by moving the log statement inside the if block that checks for the existence of the instance. sdk/packages/eternum/src/config/index.ts [22] -console.log("Instance already exists"); +if (!ConfigManager._instance) { + console.log("Create instance"); + ConfigManager._instance = new ConfigManager(customConfig); +} else { + console.log("Instance already exists"); +} Suggestion importance[1-10]: 9 Why: This suggestion corrects a logical error in the singleton pattern implementation, ensuring that the log message "Instance already exists" is only printed when the instance actually exists. This improves code correctness and debugging clarity.	9
	Add a check to prevent division by zero in the calculation of effective contribution Replace direct division by resourcePrecision with a safer check to prevent division by zero errors. client/src/hooks/store/useLeaderBoardStore.tsx [24] -const effectiveContribution = (qty / resourcePrecision) * getResourceMultiplier(resourceType); +const effectiveContribution = resourcePrecision !== 0 ? (qty / resourcePrecision) * getResourceMultiplier(resourceType) : 0; Suggestion importance[1-10]: 8 Why: Adding a check to prevent division by zero is a crucial improvement that enhances the robustness of the code and prevents potential runtime errors.	8
Best practice	Use integer arithmetic to avoid floating-point precision issues To avoid potential issues with floating-point precision, consider using integer arithmetic for resource calculations, especially when dealing with financial or resource management systems. This can be achieved by scaling all relevant values by a constant factor (such as 100 or 1000) to ensure all calculations are performed with integers. sdk/packages/eternum/src/config/index.ts [119] -amount: resource_outputs_scaled[resourceId], +amount: Math.round(resource_outputs_scaled[resourceId] * 100), Suggestion importance[1-10]: 8 Why: This suggestion addresses potential floating-point precision issues, which can be critical in financial or resource management systems. However, it should be applied consistently across all relevant calculations for maximum effectiveness.	8
	Replace direct console logging with a configurable logging framework It's a good practice to avoid direct logging in production code, especially for sensitive operations. Consider using a logging framework that supports different levels of logging (debug, info, warn, error) and is configurable based on the environment (development, production). sdk/packages/eternum/src/config/index.ts [134] -console.log(`Configuring resource production ${tx.statusReceipt}...`); +logger.debug(`Configuring resource production ${tx.statusReceipt}...`); Suggestion importance[1-10]: 7 Why: Using a configurable logging framework is a good practice for production code, allowing for better control over logging levels and outputs. This enhances maintainability and security.	7
	Use strict equality for comparisons to avoid type coercion When filtering uniqueResourceInputs, it's safer to use strict equality (!==) for comparison to avoid unintended type coercions. This ensures that the comparison is made both in terms of value and type. sdk/packages/eternum/src/config/index.ts [95] -(id) => id != ResourcesIds.Wheat && id != ResourcesIds.Fish, +(id) => id !== ResourcesIds.Wheat && id !== ResourcesIds.Fish, Suggestion importance[1-10]: 6 Why: Using strict equality (!==) is a best practice to avoid unintended type coercions, ensuring more reliable comparisons. This is a minor but important improvement for code robustness.	6
Enhancement	Improve string readability using template literals Use template literals for better readability and maintainability when constructing strings that include variables. client/src/ui/modules/navigation/TopMiddleNavigation.tsx [177] -{population.population} population / {population.capacity + basePopulationCapacity} capacity +`${population.population} population / ${population.capacity + basePopulationCapacity} capacity` Suggestion importance[1-10]: 7 Why: Using template literals enhances readability and maintainability of the code, making it easier to understand and modify in the future.	7
	Use template literals for string construction To enhance code readability and maintainability, consider using template literals for constructing strings that include variables. client/src/ui/components/hints/Transfers.tsx [26] -Donkey carry capacity: <strong>{donkeyCarryCapacity}kg</strong> +Donkey carry capacity: <strong>{`${donkeyCarryCapacity} kg`}</strong> Suggestion importance[1-10]: 5 Why: Using template literals for string construction enhances code readability and maintainability, although the improvement is minor.	5
Maintainability	Improve variable naming for clarity Consider using a more descriptive variable name instead of troopConfig for better code readability. The name troopConfig could be more specific to reflect its usage related to troop settings or configurations. client/src/ui/components/hints/Combat.tsx [94] -const troopConfig = configManager.getConfig().troop; +const troopSettings = configManager.getConfig().troop; Suggestion importance[1-10]: 6 Why: The suggestion to rename troopConfig to troopSettings improves code readability and maintainability by making the variable's purpose clearer.	6

[mentatbot]

This pull request introduces a more modular and flexible configuration system for the Eternum project. It moves away from hardcoded constants to a more dynamic configuration approach, allowing for easier customization and maintenance. The changes span across the client, contracts, and SDK, ensuring consistency throughout the project.

Key changes include:

1. Introduction of a ConfigManager class for handling configuration data.
2. Removal of hardcoded constants in favor of configuration-based values.
3. Updates to various systems (e.g., stamina, travel) to use the new configuration approach.
4. Addition of new configuration options and restructuring of existing ones.

While the changes generally improve the project's flexibility, there are a few areas that need attention, such as the use of fs in the client-side code and potential inconsistencies in stamina cost calculations. These have been noted in the inline comments.

Overall, this is a significant improvement in the project's architecture, but care should be taken to ensure all systems are updated correctly to use the new configuration system.

Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!