Clean sdk

[cwastche]

PR Type

enhancement, configuration changes

---

Description

• Removed DonkeyFarm from various building-related configurations and constants in the SDK.
• Updated manifest files to reflect the removal of DonkeyFarm and adjusted various class_hash and original_class_hash values.
• Adjusted block_number in manifest files from 19 to 17.

---

Changes walkthrough 📝

	Relevant files
Enhancement	SelectPreviewBuilding.tsx Remove `DonkeyFarm` from building selection menu                  client/src/ui/components/construction/SelectPreviewBuilding.tsx - Removed `DonkeyFarm` from the list of building keys. +0/-1      config.tsx Remove `DonkeyFarm` from building images configuration      client/src/ui/config.tsx Removed DonkeyFarm from the BUILDING_IMAGES_PATH configuration. +0/-1      buildings.ts Remove `DonkeyFarm` from building constants                            sdk/packages/eternum/src/constants/buildings.ts - Removed `DonkeyFarm` from various building-related constants. +0/-4      resources.ts Remove `DonkeyFarm` from building costs configuration        sdk/packages/eternum/src/constants/resources.ts - Removed `DonkeyFarm` from the `BUILDING_COSTS` configuration. +0/-1      structures.ts Remove `DonkeyFarm` from building type definitions              sdk/packages/eternum/src/constants/structures.ts Removed DonkeyFarm from the BuildingType enum and related mappings. +21/-28  buildings.cairo Remove `DonkeyFarm` from building category definitions      contracts/src/models/buildings.cairo Removed DonkeyFarm from the BuildingCategory enum and related implementations. +5/-9
Configuration changes	manifest.json Update manifest hashes and remove `DonkeyFarm`                      contracts/manifests/dev/manifest.json Updated various class_hash and original_class_hash values. Adjusted block_number from 19 to 17. Removed DonkeyFarm related entries. +37/-61  manifest.toml Update manifest hashes and remove `DonkeyFarm`                      contracts/manifests/dev/manifest.toml Updated various class_hash and original_class_hash values. Adjusted block_number from 19 to 17. Removed DonkeyFarm related entries. +37/-37

---

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

[vercel[bot]]

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 5, 2024 4:29pm

[github-actions[bot]]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	3
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Possible Bug: Ensure that the removal of DonkeyFarm does not affect other dependencies or functionalities that might still be expecting this building type or its associated resources.
	Data Consistency: Verify that all references to DonkeyFarm across various files and configurations have been consistently and completely removed to avoid runtime errors or inconsistencies in the application's behavior.

[github-actions[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Update the MAX_BUILDING_TYPE to match the highest building type index ___ **Update the value of MAX_BUILDING_TYPE to reflect the highest building type index after the changes. The current value is 14, but it should be updated to 15 as FragmentMine is now at index 15.** [sdk/packages/eternum/src/constants/structures.ts [29]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-13ebd7e505f23bb2b17f155849dcc315987a100a7cd1d9bba99237adc8b0ac98R29-R29) ```diff -export const MAX_BUILDING_TYPE = 14; +export const MAX_BUILDING_TYPE = 15; ``` Suggestion importance[1-10]: 10 Why: This suggestion addresses a potential bug by ensuring that `MAX_BUILDING_TYPE` correctly reflects the highest building type index, which is crucial for the correct functioning of the code.	10
	Verify and correct any mismatches in enum to string mappings ___ **Ensure that the indices and values in BuildingStringToEnum match the updated BuildingType enum to maintain consistency and avoid potential mismatches or errors in mapping.** [sdk/packages/eternum/src/constants/structures.ts [90-96]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-13ebd7e505f23bb2b17f155849dcc315987a100a7cd1d9bba99237adc8b0ac98R90-R96) ```diff +TradingPost: 9, +WorkersHut: 10, +WatchTower: 11, +Walls: 12, +Storehouse: 13, +Bank: 14, +FragmentMine: 15, - ``` Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential bug by ensuring that the indices and values in `BuildingStringToEnum` match the updated `BuildingType` enum, maintaining consistency and preventing errors.	9
	Ensure the class_hash and original_class_hash accurately reflect different contract states if required ___ **The class_hash and original_class_hash values are identical in multiple entries, which might be incorrect if they are supposed to represent different states or versions of the contract. Ensure that these hashes correctly represent the contract states as intended.** [contracts/manifests/dev/manifest.json [1023-1024]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-bc6816b30ea64e261f152a070b6e767d29d900fcbacb95c684f1c14a82dff5a8R1023-R1024) ```diff "class_hash": "0x2281f217a4d6994a2d4203859bb8ff70b2375898bbd5285ba0bbd27520d4b2b", -"original_class_hash": "0x2281f217a4d6994a2d4203859bb8ff70b2375898bbd5285ba0bbd27520d4b2b", +"original_class_hash": "0x70bd946b5a24b94692690fec3837fc4f3330e3164c5650ddb2823ed82c92513", ``` Suggestion importance[1-10]: 7 Why: Identical `class_hash` and `original_class_hash` values might indicate an error if they are supposed to represent different states. This suggestion is important for maintaining accurate contract state representation.	7
	Reintroduce the 'DonkeyFarm' to the building categories if it was removed by mistake ___ **Ensure that all necessary building categories are included in the enum, as 'DonkeyFarm' was removed.** [contracts/src/models/buildings.cairo [58-63]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-d98a082b4990bd2f45a454b6fbeef40ee7acbd1650edbdcb1657071706a34ca7R58-R63) ```diff Market, ArcheryRange, Stable, +DonkeyFarm, TradingPost, WorkersHut, WatchTower, ``` Suggestion importance[1-10]: 6 Why: The suggestion is valid if 'DonkeyFarm' was removed unintentionally. However, without context, it's unclear if the removal was deliberate, making this a lower priority.	6
Possible issue	Correct the block number to match the intended current value ___ **Ensure that the block_number is updated to reflect the correct and current block number, as it seems to have been decreased.** [contracts/manifests/dev/manifest.toml [8]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-43499a9633d551acabe80100dc29d8995890cd62b12169122658233caea498b3R8-R8) ```diff -block_number = 17 +block_number = 19 ``` Suggestion importance[1-10]: 9 Why: Ensuring the block number is correct is crucial for the integrity of the manifest. The suggestion addresses a potential issue where the block number was decreased, which could lead to inconsistencies.	9
	Verify and correct the block_number to ensure it accurately reflects the intended blockchain state ___ **The block_number has been changed from 19 to 17. This could be an error if the block number is supposed to be incremental or represent a specific state in the blockchain. Verify if the new block number is correct or if it should be reverted or updated to a different value.** [contracts/manifests/dev/manifest.json [1004]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-bc6816b30ea64e261f152a070b6e767d29d900fcbacb95c684f1c14a82dff5a8R1004-R1004) ```diff -"block_number": 17, +"block_number": 19, ``` Suggestion importance[1-10]: 8 Why: The `block_number` change from 19 to 17 could be a significant issue if it is meant to represent a specific state in the blockchain. Verifying this change is crucial to ensure the integrity of the data.	8
Compatibility	Consider the implications of removing "DonkeyFarm" from the ABI and ensure compatibility or provide migration paths ___ **The removal of "DonkeyFarm" from the contract's ABI might lead to compatibility issues if existing clients or systems depend on this contract. If this is an intentional removal, consider versioning the ABI or providing migration paths for clients.** [contracts/manifests/dev/manifest.json [1804]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-bc6816b30ea64e261f152a070b6e767d29d900fcbacb95c684f1c14a82dff5a8R1804-R1804) ```diff +{ + "name": "DonkeyFarm", + "type": "()" +}, { "name": "TradingPost", "type": "()" ``` Suggestion importance[1-10]: 9 Why: Removing "DonkeyFarm" from the ABI could lead to compatibility issues. This suggestion addresses a potential major issue by recommending careful consideration of the removal's impact.	9
Maintainability	Ensure consistent naming between enum and string mappings ___ **Ensure consistency in naming conventions for the building types in the BuildingEnumToString mapping. The name "Shards Mine" should be updated to "Fragment Mine" to match the enum name in BuildingType.** [sdk/packages/eternum/src/constants/structures.ts [77]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-13ebd7e505f23bb2b17f155849dcc315987a100a7cd1d9bba99237adc8b0ac98R77-R77) ```diff -15: "Shards Mine", +15: "Fragment Mine", ``` Suggestion importance[1-10]: 8 Why: This suggestion improves maintainability by ensuring consistency in naming conventions, which helps avoid confusion and potential errors.	8
	Verify the updated class_hash values to ensure they are consistent with the intended contract updates ___ **The class_hash values have been updated across multiple contracts. It's crucial to ensure that these updates are consistent with the intended changes in the contract logic or state and that they do not introduce discrepancies in the contract's behavior.** [contracts/manifests/dev/manifest.json [1971-1972]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-bc6816b30ea64e261f152a070b6e767d29d900fcbacb95c684f1c14a82dff5a8R1971-R1972) ```diff "class_hash": "0x2946eae00c656130645895ecf115128de132cd86673e9cf8ae5ae186a99811", -"original_class_hash": "0x2946eae00c656130645895ecf115128de132cd86673e9cf8ae5ae186a99811", +"original_class_hash": "0x4bded07a66b8b262e64fe7b61f0f421ddca2e9f069d56912ca4a89320298cd3", ``` Suggestion importance[1-10]: 7 Why: Ensuring that the updated `class_hash` values are consistent with the intended contract changes is important for maintaining the integrity and expected behavior of the contracts.	7
	Replace repeated literal strings with a variable for maintainability ___ **Consider using a variable for the repeated base class hash to ensure consistency and ease future updates.** [contracts/manifests/dev/manifest.toml [118]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-43499a9633d551acabe80100dc29d8995890cd62b12169122658233caea498b3R118-R118) ```diff -base_class_hash = "0x22f3e55b61d86c2ac5239fa3b3b8761f26b9a5c0b5f61ddbd5d756ced498b46" +base_class_hash = BASE_CLASS_HASH ``` Suggestion importance[1-10]: 7 Why: Using a variable for the repeated base class hash improves maintainability and reduces the risk of errors during future updates. However, it is not a critical issue.	7
Enhancement	Adjust enum indices to reflect the removal of an item for consistency ___ **Update the enum indices to maintain the correct sequence after removing an item.** [contracts/src/models/buildings.cairo [80-84]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-d98a082b4990bd2f45a454b6fbeef40ee7acbd1650edbdcb1657071706a34ca7R80-R84) ```diff +BuildingCategory::TradingPost => 9, +BuildingCategory::WorkersHut => 10, +BuildingCategory::WatchTower => 11, +BuildingCategory::Walls => 12, +BuildingCategory::Storehouse => 13, - ``` Suggestion importance[1-10]: 8 Why: Updating the enum indices after removing an item ensures the correct sequence and prevents potential bugs related to incorrect indexing.	8
	Reorder enum values to maintain logical grouping and improve maintainability ___ **Consider reordering the enum values in BuildingType to maintain a logical grouping or ordering, such as grouping all resource-related buildings together, which can improve readability and maintainability.** [sdk/packages/eternum/src/constants/structures.ts [18-27]](https://github.com/BibliothecaDAO/eternum/pull/1060/files#diff-13ebd7e505f23bb2b17f155849dcc315987a100a7cd1d9bba99237adc8b0ac98R18-R27) ```diff +Market = 6, +ArcheryRange = 7, +Stable = 8, +TradingPost = 9, +WorkersHut = 10, +WatchTower = 11, +Walls = 12, +Storehouse = 13, +Bank = 14, +FragmentMine = 15, - ``` Suggestion importance[1-10]: 6 Why: While this suggestion can improve readability and maintainability, it is not crucial for the functionality of the code. It is more of an enhancement for better code organization.	6

[aymericdelab]

would be careful merging this rn, because it might have some unwanted consequences if we migrate this in the deployed eternum world. where people have been constructing buildings already.

maybe we can close this for for now or put is as a draft, and merge it end of the week when we create new world ?