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

Jump to bottom

decouple equip and unequip #190

Merged
merged 4 commits into from
Jul 28, 2022
Merged

decouple equip and unequip #190

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
deb0b4a
decouple equip and unequip
bmacer Jun 25, 2022
e138584
add helper methods and split ItemAlreadyEquipped
bmacer Jul 8, 2022
6a440e4
Merge branch 'main' into bug/147-decouple-equip-and-unequip
bmacer Jul 8, 2022
82d3f70
Merge 'main' into decouple-equip-and-unequip
ilionic Jul 28, 2022
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
6 changes: 6 additions & 0 deletions pallets/rmrk-core/src/functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,4 +723,10 @@ where
ensure!(nft.transferable, Error::<T>::NonTransferable);
Ok(())
}

/// Helper function for checking if an NFT exists
pub fn nft_exists(item: (CollectionId, NftId)) -> bool {
let (item_collection_id, item_nft_id) = item;
Nfts::<T>::get(item_collection_id, item_nft_id).is_some()
}
}
32 changes: 21 additions & 11 deletions pallets/rmrk-equip/src/functions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,15 @@ impl<T: Config> Pallet<T> {
Ok(current_id)
})
}

/// Helper function for checking if an item is equipped
/// If the Equippings storage contains the Base/Slot for the Collection+NFT ID, the item is
/// already equipped
pub fn slot_is_equipped(item: (CollectionId, NftId), base_id: BaseId, slot_id: SlotId) -> bool {
let (equipper_collection_id, equipper_nft_id) = item;
Equippings::<T>::get(((equipper_collection_id, equipper_nft_id), base_id, slot_id))
.is_some()
}
}

impl<T: Config>
Expand Down Expand Up @@ -149,19 +158,19 @@ where
!pallet_rmrk_core::Pallet::<T>::nfts(item_collection_id, item_nft_id)
.unwrap()
.equipped,
Error::<T>::AlreadyEquipped
Error::<T>::ItemAlreadyEquipped
);

// If the Equippings storage contains the Base/Slot for the Collection+NFT ID, the item is
// already equipped
let item_is_equipped =
Equippings::<T>::get(((equipper_collection_id, equipper_nft_id), base_id, slot_id))
.is_some();
ensure!(!item_is_equipped, Error::<T>::AlreadyEquipped);
ensure!(
!Self::slot_is_equipped((equipper_collection_id, equipper_nft_id), base_id, slot_id),
Error::<T>::SlotAlreadyEquipped
);

// Item must exist
let item_exists =
pallet_rmrk_core::Pallet::<T>::nfts(item_collection_id, item_nft_id).is_some();
pallet_rmrk_core::Pallet::<T>::nft_exists((item_collection_id, item_nft_id));
Copy link
Contributor

Choose a reason for hiding this comment

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

We do what you did on line 166 for this logic here:
ensure!(pallet_rmrk_core::Pallet::<T>::nft_exists((item_collection_id, item_nft_id)), Error::<T>::ItemDoesntExist);

ensure!(item_exists, Error::<T>::ItemDoesntExist);

// Equipper must exist
Expand Down Expand Up @@ -300,13 +309,14 @@ where
pallet_uniques::Error::<T>::Locked
);

let item_is_equipped =
Equippings::<T>::get(((equipper_collection_id, equipper_nft_id), base_id, slot_id))
.is_some();
ensure!(item_is_equipped, Error::<T>::ItemNotEquipped);
ensure!(
Self::slot_is_equipped((equipper_collection_id, equipper_nft_id), base_id, slot_id),
Error::<T>::SlotNotEquipped
);

// Check if the item already exists
let item_exists =
pallet_rmrk_core::Pallet::<T>::nfts(item_collection_id, item_nft_id).is_some();
pallet_rmrk_core::Pallet::<T>::nft_exists((item_collection_id, item_nft_id));

// If item doesn't exist, anyone can unequip it. This can happen because burn_nft can
// happen in rmrk-core, which doesn't know about rmrk-equip.
Expand Down
8 changes: 6 additions & 2 deletions pallets/rmrk-equip/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,12 @@ pub mod pallet {
NoEquippableOnFixedPart,
// No "default" Theme is defined, required prior to defining other themes
NeedsDefaultThemeFirst,
// Equipped item cannot be equipped elsewhere (without first unequipping)
AlreadyEquipped,
// Item is trying to be equipped but is already equipped
ItemAlreadyEquipped,
// Slot to which an item is being equipped already has something equipped there
SlotAlreadyEquipped,
// Slot from which an item is being unequipped has nothing equipped there
SlotNotEquipped,
// Error that should not occur
// TODO is this being used?
UnknownError,
Expand Down
50 changes: 44 additions & 6 deletions pallets/rmrk-equip/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,14 @@ fn equip_works() {
AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0), // Recipient
));

// Sends NFT (0, 2) [flashlight] to NFT (0, 0) [character-0]
assert_ok!(RmrkCore::send(
Origin::signed(ALICE),
1, // Collection ID
0, // NFT ID
AccountIdOrCollectionNftTuple::CollectionAndNftTuple(0, 0), // Recipient
));

// Attempt to equip sword should fail as character-0 doesn't have a resource that is
// associated with this base
assert_noop!(
Expand Down Expand Up @@ -303,7 +311,24 @@ fn equip_works() {
sword_slot_resource_left
));

// Equipping should now work
let flashlight_slot_resource_left = SlotResource {
src: Some(stbd("ipfs://flashlight-metadata-left")),
base: 0, // BaseID
license: None,
metadata: None,
slot: 201, // SlotID
thumb: None,
};

// Add our flashlight left-hand resource to our flashlight NFT
assert_ok!(RmrkCore::add_slot_resource(
Origin::signed(ALICE),
1, // collection id
1, // nft id
flashlight_slot_resource_left
));

// Equipping sword should now work
assert_ok!(RmrkEquip::equip(
Origin::signed(ALICE), // Signer
(1, 0), // item
Expand All @@ -320,10 +345,23 @@ fn equip_works() {
slot_id: 201,
}));

// Equipping flashlight to left-hand should fail (SlotAlreadyEquipped)
assert_noop!(
RmrkEquip::equip(
Origin::signed(ALICE), // Signer
(1, 1), // item
(0, 0), // equipper
0, // ResourceId,
0, // BaseId
201, // SlotId
),
Error::<Test>::SlotAlreadyEquipped
);

// Equipped resource ID 0 should now be associated with equippings for character-0
// on base 0, slot 201
let equipped = RmrkEquip::equippings(((0, 0), 0, 201));
assert_eq!(equipped.clone().unwrap(), 0,);
assert_eq!(equipped.unwrap(), 0,);

// Resource for equipped item should exist
assert!(RmrkCore::resources((1, 0, equipped.unwrap())).is_some());
Expand Down Expand Up @@ -355,10 +393,10 @@ fn equip_works() {
0, // BaseId
202, // SlotId
),
Error::<Test>::AlreadyEquipped
Error::<Test>::ItemAlreadyEquipped
);

// Equipping to left-hand should fail (AlreadyEquipped)
// Equipping to left-hand should fail (ItemAlreadyEquipped)
assert_noop!(
RmrkEquip::equip(
Origin::signed(ALICE), // Signer
Expand All @@ -368,7 +406,7 @@ fn equip_works() {
0, // BaseId
201, // SlotId
),
Error::<Test>::AlreadyEquipped
Error::<Test>::ItemAlreadyEquipped
);

assert_ok!(RmrkEquip::unequip(
Expand Down Expand Up @@ -426,7 +464,7 @@ fn equip_works() {
0, // BaseId
201, // SlotId
),
Error::<Test>::ItemNotEquipped
Error::<Test>::SlotNotEquipped
);

// Equipping to right-hand should work
Expand Down