switch to auto-increment ResourceId

[bmacer]

addresses #138
One issue that remains is that id is part of the Resource struct. I don't think this is necessary generally. For example, Nft and Collection don't have their respective IDs as part of their struct. However, we need to extract the resource ID when doing an iter_prefix_values for the CollectionID, NftId inside the do_equip function (we do this when checking for the existence of a BaseId+SlotId on the resources for a Slot Resource -- that is, ensuring that there exists a Slot Resource matching the BaseId and SlotId passed to the equip extrinsic).

rmrk-substrate/pallets/rmrk-equip/src/functions.rs

Lines 255 to 268 in 218fe30

	let resources_matching_base_iter =
	pallet_rmrk_core::Resources::<T>::iter_prefix_values((item_collection_id, item_nft_id));
	for resource in resources_matching_base_iter {
	match resource.resource {
	ResourceTypes::Slot(res) => {
	if res.slot == slot_id && res.base == base_id {
	found_base_slot_resource_on_nft = true;
	to_equip_resource_id = resource.id;
	}
	},
	_ => (),
	}
	}

We could either figure out how to get the key value when iterating through with iter_prefix_values (I asked the question in StackExchange here), or find a more graceful way of checking altogether.

For now, holding the id in the struct works.

[HashWarlock]

Overall looks good.

[HashWarlock]

addresses #138 One issue that remains is that id is part of the Resource struct. I don't think this is necessary generally. For example, Nft and Collection don't have their respective IDs as part of their struct. However, we need to extract the resource ID when doing an iter_prefix_values for the CollectionID, NftId inside the do_equip function (we do this when checking for the existence of a BaseId+SlotId on the resources for a Slot Resource -- that is, ensuring that there exists a Slot Resource matching the BaseId and SlotId passed to the equip extrinsic).

rmrk-substrate/pallets/rmrk-equip/src/functions.rs

Lines 255 to 268 in 218fe30

	let resources_matching_base_iter =
	pallet_rmrk_core::Resources::<T>::iter_prefix_values((item_collection_id, item_nft_id));
	for resource in resources_matching_base_iter {
	match resource.resource {
	ResourceTypes::Slot(res) => {
	if res.slot == slot_id && res.base == base_id {
	found_base_slot_resource_on_nft = true;
	to_equip_resource_id = resource.id;
	}
	},
	_ => (),
	}
	}

We could either figure out how to get the key value when iterating through with iter_prefix_values (I asked the question in StackExchange here), or find a more graceful way of checking altogether.

For now, holding the id in the struct works.

I think the do_equip function handles too much and can be confusing for downstream in the case. We can end up making this function easier to handle by decoupling the unequip logic into its own flow. I'll try to think of a way to better optimize the storage logic of resources. I have a couple questions:

• Do Resources have the ability to be more than just a Base(Composable), Slot or Media(Basic)?
  • If that's the case then 2 resource types would be able to be assigned a priority Base & Media, right?

I think we can separate these into their own structs then have an enum for the ResourceType & then make the structs optionals in the ResourceInfo struct. I'll create an issue & try to create a diagram & code to explain what it would look like.

[mrshiposha]

@bmacer regarding the extracting Ids when doing iter_prefix_values: there is another function iter_prefix that gives access to all remaining keys and a value.

I think you can do something like this:

let resources_matching_base_iter =
			pallet_rmrk_core::Resources::<T>::iter_prefix((item_collection_id, item_nft_id));

for (id, resource) in resources_matching_base_iter {
    match resource.resource {
	ResourceTypes::Slot(res) =>
	    if res.slot == slot_id && res.base == base_id {
                  found_base_slot_resource_on_nft = true;
		  to_equip_resource_id = id;
	    },
	_ => (),
    }
}

[ilionic]

Since iteration over current unbounded Resources storage is suboptimal ( as pointed out by Shawn in the linked comment ) we should consider switching it to BoundedVec property of NFT / Base itself so it's safe.

[bmacer]

Since iteration over current unbounded Resources storage is suboptimal ( as pointed out by Shawn in the linked comment ) we should consider switching it to BoundedVec property of NFT / Base itself so it's safe.

I added a commit 20235f9 that can address this, if it's viable.

I added Storages for ComposableResources and SlotResources. This enables us to check these storages swiftly when equipping. One potential downside to this is that it requires adding a resource_id parameter to the equip function, meaning you can't just equip a base+slot and have the code figure out what Resource to equip. Additionally, this adds storage (because the Resource itself is still stored in the Resources Storage) though this additional storage is "existential", and doesn't store/check for anything besides existence of a [in the case of Composable] CollectionID + NFTId + BaseId, or [in the case of Slot] CollectionId + NFTId + ResourceId + BaseId + SlotId.

[HashWarlock]

Couldn't we keep src, metadata, license & thumb in here? I was thinking ResourceInfo would have the ResourceType that we could match on then get the contents from the nested struct then handle logic from there.

[ilionic]

Looking good, much cleaner. I think that's an acceptable tradeoff having additional storage to avoid unbounded iterations but curious to hear more opinions.

[ilionic]

probably worth adding few extra docs comments about purpose of this storage ( from your other comment )

[ilionic]

Would be safe using TwoX here instead?

[ilionic]

minor thing, purely readability - how about renaming resource to more explicit ie resource_type

[ilionic]

Merging to avoid conflicts. @HashWarlock feel free to extract extra comments to separate issue. @bmacer CC