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.

Sign up for GitHub

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

contracts: no lords burn during donkey production for wonders #2164

Merged
merged 5 commits into from
Nov 29, 2024
Merged

contracts: no lords burn during donkey production for wonders #2164

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8563ce6
contracts: no lords burn during donkey production for wonders
credence0x Nov 28, 2024
d67c472
manifest
credence0x Nov 28, 2024
5b24fd6
Merge branch 'next' of https://github.com/BibliothecaDAO/eternum into…
credence0x Nov 28, 2024
eec2479
fix build
credence0x Nov 28, 2024
5291e1a
Merge branch 'next' into wonder
credence0x Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion client/src/dojo/contractComponents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -963,12 +963,13 @@ export function defineContractComponents(world: World) {
produced_resources: RecsType.BigInt,
order: RecsType.Number,
level: RecsType.Number,
has_wonder: RecsType.Boolean,
},
{
metadata: {
namespace: "eternum",
name: "Realm",
types: ["u32", "u32", "u128", "u8", "u8"],
types: ["u32", "u32", "u128", "u8", "u8", "bool"],
customTypes: [],
},
},
Expand Down
46 changes: 23 additions & 23 deletions contracts/manifest_dev.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1518,8 +1518,8 @@
"systems": ["create_bank", "change_owner_amm_fee", "change_owner_bridge_fee", "upgrade"]
},
{
"address": "0x41cd1abd5e28f95785deddcdb0673726f11f88274d5b5eb261f22a57807b05f",
"class_hash": "0x46a4aa354717fb6666137f575010e3532dc156ffc8ce0408c587771aaea7baa",
"address": "0x31ec72d0055d936659a62e5278ffe4d1ea570a17c33f664542ed4f9ee5f6329",
"class_hash": "0x392d53b4d0aed0cb7934418ee0666c571cf7415daf7203746ba53c647572815",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -2010,8 +2010,8 @@
"systems": ["battle_start", "battle_force_start", "battle_join", "battle_leave", "battle_claim", "upgrade"]
},
{
"address": "0x3c212b90cc4f236be2c014e0ee0d870277b2cc313217a73d41387e255e806ed",
"class_hash": "0x5a28b1a27a5ee44be4fa967f57fad5d765663ed475a6b52d8687e43ee786bad",
"address": "0x2d86a5d991bb8bd69322d88e31392ee0732571251b7fd161779b759f25f60a6",
"class_hash": "0x195c5153e1b5373996566a5bff34dd63604287c9361e462be70db1c3c0dc1de",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -2387,8 +2387,8 @@
"systems": ["leave_battle", "leave_battle_if_ended", "upgrade"]
},
{
"address": "0x2447362969fb31d334ffe454be9b86d166a4757b151186c85d095547cb4192",
"class_hash": "0x22a2fdee875dc760bc4f39e2032cbce68f7cc4e8ee14824e39fa8e957efcb6b",
"address": "0x30873baffa16e1606d5450d2464ed4d6b421aefb27364cc4fe4b9f396e3b5cb",
"class_hash": "0x4650c58c01778594c4d99a3536812135986146c3cbaab6812abfacf81a27644",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -5615,8 +5615,8 @@
"systems": ["create", "contribute_to_construction", "set_co_owners", "end_game", "set_access", "upgrade"]
},
{
"address": "0x7a5e4dfaba7aced9add65913d44311d74e12f85c55503ebf903103b102847e5",
"class_hash": "0x49f4633b190e557b7aa8a318f9f14c24ac6fc64f79bd6906a0833195b1d9555",
"address": "0x3cbc91907d4bb99ee29ad4054b7fc1f3cfe534c06bb2a76151a218772cf3cce",
"class_hash": "0x35a4d8f3cba325e81ad3acb7cba0e005daf6ec43940cb89c2c2f26f597a8b95",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -5884,8 +5884,8 @@
"systems": ["add", "remove", "upgrade"]
},
{
"address": "0x42b3545b5a607c6bc9beed10cb5d7853d204f6c26c8d0313d3749e0b5841c5b",
"class_hash": "0x18adb25055ab97db0ce4f51c60d2355257afea6b32327e212b6cba02610ff67",
"address": "0x6b2d8e54ca9dfbb3a8bb30e987298af9e273242505680e74a528c1fec80ad19",
"class_hash": "0x7d2682711fdb757c67dd921ae052b731624a5d2eeb503fc4f602d1a26429a1",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -6147,8 +6147,8 @@
"systems": ["discover_shards_mine", "add_mercenaries_to_structure", "upgrade"]
},
{
"address": "0xcc0c73458864b9e0e884de532b7ebfbe757e7429fb2c736abd5e129e5fb81a",
"class_hash": "0x3c39a126084d2a27caab1d896057bd55c727329a992888c0997545390fa5a81",
"address": "0x797af17351651a5a84e7540026a68315d3c9afe6eb9792a655b44b5be39bd99",
"class_hash": "0x2045846af197fdb9f0936152a490f9983e5e9f49239fd4a46efefac860ffdf4",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -6792,8 +6792,8 @@
"systems": ["transfer_ownership", "upgrade"]
},
{
"address": "0x339b8d5ffaff8b22ef335e8f8479fe61195c30d031614a16480ccb9ee5f5f85",
"class_hash": "0x11c14e0b09ed6ef8053d0fdeaab96f5ed180b27870222510b348afc0a1a06f4",
"address": "0x2d8dc76995f0ac998301a178252a8466525c376da85941018705082da799dee",
"class_hash": "0x30ec799a2b57409928ce52b96f465a8ace9517092467e9a420d6cb182e10f5b",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -7029,8 +7029,8 @@
"systems": ["create", "upgrade_level", "quest_claim", "upgrade"]
},
{
"address": "0x637e708e7d5859f67c3822da1415466e2cd8291cd2b8cc71c0d6398af62769a",
"class_hash": "0x54479ab403ddbb74bcb6fc47b523bfe58776d96d422ee119cc10bea1dc1b225",
"address": "0x5ff5d64d8779a47a23a81f8eced192c091af4d8d9782abff92bd9dd3ccc000",
"class_hash": "0x4a905864cc4fce8cba31eac6e2079490ebf60beb91f91e1fcd4fa754fd4a6e3",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -7332,8 +7332,8 @@
"systems": ["deposit_initial", "deposit", "start_withdraw", "finish_withdraw", "upgrade"]
},
{
"address": "0x6d0c4048d7914be8beddd8ac0809413331e372be92652e864c7937488714455",
"class_hash": "0xa6b7d5941eac670a64e11cb68a884be21baff7ffd1aa11c8e2dae6e83ef9ac",
"address": "0x6b82e6c44c800a74b3580718892a538d5e6070f0d99a30ae548bd6b1bbd7bbb",
"class_hash": "0x1ae054a104d1d567266372d26ae3c993a4efbc504f1bf43fa2371005c4449bf",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -7587,8 +7587,8 @@
"systems": ["approve", "send", "pickup", "upgrade"]
},
{
"address": "0x30a4a6472ff2bcfc68d709802e5a9f31f5ac01d04fa97e37d32ce7568741262",
"class_hash": "0x5eeba32398a22b6aeb873bba9b4c0bbbbae33135c97a4227801e08c4185e885",
"address": "0x77c3936532aae556fdb29d8343b4327f4c1c107d96f5b72755c2a36cacfb790",
"class_hash": "0x2e0ebf327799ce82b7be846f52e01bc35f74263c7c1812374be635f253b4453",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -7828,8 +7828,8 @@
"systems": ["buy", "sell", "upgrade"]
},
{
"address": "0x3a6bbb82f9e670c99647f3b0c4aaf1be82be712e1a393336b79b9dab44cc5",
"class_hash": "0x5d9a4d8ee896efc30b3615011a8ae4566102bd3da75359109dabdda8d230e73",
"address": "0x3e8fe31477bfc800b1910619bc65032bd7b3bca6317896a6682c4cecb5303e8",
"class_hash": "0x6c4b4396234558c6fcdff664e8a42dfdf0ae6b1ea001cd472eb2dfd6e305d63",
"abi": [
{
"type": "impl",
Expand Down Expand Up @@ -9004,7 +9004,7 @@
},
{
"members": [],
"class_hash": "0x626d85491b645c1c377b97979a4efdbc1691d8602a560152d07eaca48e18e90",
"class_hash": "0x6d0fb88608ca69566ec6f4ded79873548e87efbe3771173935a372e935dc844",
"tag": "eternum-Realm",
"selector": "0x68a0f35d5bd894ec6af9417324d5202f5ba23a8e9488fb3556a9dbebcf4b7d7"
},
Expand Down
27 changes: 27 additions & 0 deletions contracts/src/models/buildings.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use eternum::models::position::{Coord, Position, Direction, PositionCustomTrait,
use eternum::models::production::{
Production, ProductionInput, ProductionRateTrait, ProductionInputCustomImpl, ProductionInputCustomTrait
};
use eternum::models::realm::Realm;
use eternum::models::resources::ResourceCustomTrait;
use eternum::models::resources::{Resource, ResourceCustomImpl, ResourceCost};
use eternum::utils::math::{PercentageImpl, PercentageValueImpl};
Expand Down Expand Up @@ -171,6 +172,19 @@ impl BuildingProductionCustomImpl of BuildingProductionCustomTrait {
let (input_resource_type, input_resource_amount) = (
production_input.input_resource_type, production_input.input_resource_amount
);

/// lords will not be consumed during production of donkey
/// if the realm has a wonder
if input_resource_type == ResourceTypes::LORDS {
if resource_production.resource_type == ResourceTypes::DONKEY {
let realm: Realm = world.read_model(self.outer_entity_id);
if realm.has_wonder {
count += 1;
continue;
}
}
}

let mut input_resource: Resource = ResourceCustomImpl::get(
ref world, (self.outer_entity_id, input_resource_type)
);
Expand Down Expand Up @@ -235,6 +249,19 @@ impl BuildingProductionCustomImpl of BuildingProductionCustomTrait {
let (input_resource_type, input_resource_amount) = (
production_input.input_resource_type, production_input.input_resource_amount
);

/// lords will not be consumed during production of donkey
/// if the realm has a wonder
if input_resource_type == ResourceTypes::LORDS {
if resource_production.resource_type == ResourceTypes::DONKEY {
let realm: Realm = world.read_model(self.outer_entity_id);
if realm.has_wonder {
count += 1;
continue;
}
}
}

let mut input_resource: Resource = ResourceCustomImpl::get(
ref world, (self.outer_entity_id, input_resource_type)
);
Expand Down
5 changes: 4 additions & 1 deletion contracts/src/models/owner.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ mod tests {
fn owner_test_entity_owner_get_realm_id() {
let mut world = spawn_eternum();

world.write_model_test(@Realm { entity_id: 1, realm_id: 3, produced_resources: 0, order: 0, level: 0 });
world
.write_model_test(
@Realm { entity_id: 1, realm_id: 3, produced_resources: 0, order: 0, level: 0, has_wonder: false }
);
world.write_model_test(@EntityOwner { entity_id: 2, entity_owner_id: 1 });

let entity_owner: EntityOwner = world.read_model(2);
Expand Down
5 changes: 3 additions & 2 deletions contracts/src/models/realm.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ pub struct Realm {
realm_id: ID,
produced_resources: u128,
order: u8,
level: u8
level: u8,
has_wonder: bool
}


Expand Down Expand Up @@ -330,7 +331,7 @@ mod test_realm_resources_impl {
use super::{RealmResourcesImpl, RealmResourcesTrait, Realm};

fn mock_realm() -> Realm {
Realm { entity_id: 1, realm_id: 1, order: 0, level: 0, produced_resources: 0, }
Realm { entity_id: 1, realm_id: 1, order: 0, level: 0, produced_resources: 0, has_wonder: false }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add test cases for realms with wonders

The mock realm correctly initializes has_wonder to false, but test coverage for wonder-related functionality is missing. Consider adding test cases that:

  1. Create realms with wonders (has_wonder: true)
  2. Verify wonder-related behavior differences
  3. Test consistency between has_wonder and decoded wonder attributes
 fn mock_realm() -> Realm {
     Realm { entity_id: 1, realm_id: 1, order: 0, level: 0, produced_resources: 0, has_wonder: false }
 }

+#[test]
+fn test_realm_with_wonder() {
+    let mut realm = mock_realm();
+    realm.has_wonder = true;
+    assert!(realm.has_wonder, "Realm should have a wonder");
+}
+
+#[test]
+fn test_wonder_attribute_consistency() {
+    // Test that decoded wonder attribute matches has_wonder field
+    // Add implementation
+}

Committable suggestion skipped: line range outside the PR's diff.

}


Expand Down
10 changes: 7 additions & 3 deletions contracts/src/systems/realm/contracts.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ mod realm_systems {
use eternum::models::quest::{Quest, QuestBonus};
use eternum::models::realm::{
Realm, RealmCustomTrait, RealmCustomImpl, RealmResourcesTrait, RealmResourcesImpl,
RealmNameAndAttrsDecodingTrait, RealmNameAndAttrsDecodingImpl
RealmNameAndAttrsDecodingTrait, RealmNameAndAttrsDecodingImpl, RealmReferenceImpl
};
use eternum::models::resources::{
DetachedResource, Resource, ResourceCustomImpl, ResourceCustomTrait, ResourceFoodImpl, ResourceFoodTrait
Expand Down Expand Up @@ -94,7 +94,7 @@ mod realm_systems {
// create realm
let mut coord: Coord = InternalRealmLogicImpl::get_new_location(ref world);
let (entity_id, realm_produced_resources_packed) = InternalRealmLogicImpl::create_realm(
ref world, owner, realm_id, resources, order, 0, coord
ref world, owner, realm_id, resources, order, 0, wonder, coord
);

// collect lords attached to season pass and bridge into the realm
Expand Down Expand Up @@ -278,9 +278,12 @@ mod realm_systems {
resources: Array<u8>,
order: u8,
level: u8,
wonder: u8,
coord: Coord
) -> (ID, u128) {
// create realm

let has_wonder = RealmReferenceImpl::wonder_mapping(wonder.into()) != "None";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Implementation is correct but has a potential issue

The wonder mapping implementation shows that checking against "None" is the correct way to determine if a realm has a wonder, as "None" is indeed returned for wonder value 1. However, there's a potential issue where the function panics for wonder value 0, which might need to be handled differently depending on the business logic.

  • wonder_mapping returns "None" for value 1
  • wonder_mapping returns actual wonder names for values 2 and 3
  • wonder_mapping panics for value 0, which might need attention
🔗 Analysis chain

Verify wonder mapping implementation

The implementation checks if the wonder mapping returns "None" to determine if a realm has a wonder. Let's verify the wonder mapping implementation.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the wonder mapping implementation
# Look for the wonder mapping function definition and its possible return values

ast-grep --pattern 'fn wonder_mapping($wonder: $_) -> $_'

Length of output: 59

Script:

#!/bin/bash
# Let's try a broader search for wonder_mapping using ripgrep
rg "wonder_mapping" -A 5 -B 5

Length of output: 1772

let realm_produced_resources_packed = RealmResourcesImpl::pack_resource_types(resources.span());
let entity_id = world.dispatcher.uuid();
let now = starknet::get_block_timestamp();
Expand All @@ -302,7 +305,8 @@ mod realm_systems {
realm_id,
produced_resources: realm_produced_resources_packed,
order,
level
level,
has_wonder
}
);
world.write_model(@Position { entity_id: entity_id.into(), x: coord.x, y: coord.y, });
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/utils/testing/general.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fn spawn_realm(ref world: WorldStorage, realm_id: ID, coord: Coord) -> ID {
let produced_resources = array![];
let order = 1;
let (realm_entity_id, _realm_produced_resources_packed) = InternalRealmLogicImpl::create_realm(
ref world, owner, realm_id, produced_resources, order, 0, coord.into()
ref world, owner, realm_id, produced_resources, order, 0, 1, coord.into()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Make the wonder parameter configurable and document its purpose.

The hardcoded value 1 for the wonder parameter limits the testing utility's flexibility. Since this is a testing helper, it should support creating realms both with and without wonders.

Consider this improvement:

-        ref world, owner, realm_id, produced_resources, order, 0, 1, coord.into()
+        ref world, owner, realm_id, produced_resources, order, 0, has_wonder: bool, coord.into()

And update the function signature:

-fn spawn_realm(ref world: WorldStorage, realm_id: ID, coord: Coord) -> ID {
+/// Spawns a test realm
+/// # Arguments
+/// * `world` - The world storage
+/// * `realm_id` - The ID of the realm to spawn
+/// * `coord` - The coordinates where to spawn the realm
+/// * `has_wonder` - Whether the realm has a wonder (affects lords burning during donkey production)
+fn spawn_realm(ref world: WorldStorage, realm_id: ID, coord: Coord, has_wonder: bool) -> ID {

Additionally, consider adding a convenience helper:

/// Spawns a test realm with a wonder
fn spawn_realm_with_wonder(ref world: WorldStorage, realm_id: ID, coord: Coord) -> ID {
    spawn_realm(ref world, realm_id, coord, true)
}

);

realm_entity_id
Expand Down
3 changes: 2 additions & 1 deletion landing/src/dojo/contractComponents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -963,12 +963,13 @@ export function defineContractComponents(world: World) {
produced_resources: RecsType.BigInt,
order: RecsType.Number,
level: RecsType.Number,
has_wonder: RecsType.Boolean,
},
{
metadata: {
namespace: "eternum",
name: "Realm",
types: ["u32", "u32", "u128", "u8", "u8"],
types: ["u32", "u32", "u128", "u8", "u8", "bool"],
customTypes: [],
},
},
Expand Down
46 changes: 4 additions & 42 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading