-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Add amalgamation traits for NFT CollectionId and ItemId #13514
Add amalgamation traits for NFT CollectionId and ItemId #13514
Conversation
04fe4b8
to
9fdc72d
Compare
Add a non topic change (perhaps I need update the title?) |
@jsidorenko Could you help to take a look? (don't know how to assign reviewers...) |
Hey @jasl ,
Could you pls provide some code snippet or push your code to GH and put a link here? |
I remeber I tried but not work, |
I pushed minimal reproducible code Background: version 1: https://github.com/jasl/learn_nft/blob/main/src/lib.rs
version 2: https://github.com/jasl/learn_nft/blob/another-case/src/lib.rs
|
Hi @jasl, I'm wondering why don't you want to connect the nfts pallet in a loosely coupled way? |
Sorry, I just checked the second example where you used the traits |
so, the item_id problem I was able able to solve locally by adding |
My another case (https://github.com/jasl/learn_nft/blob/another-case/src/lib.rs) use that way, I'll learn from that PR first. thank you |
@jasl regarding the Copy/Clone issue, I found it doesn't complain if you provide the ItemId/CollectionId via the runtime config: https://github.com/paritytech/substrate/pull/12565/files#diff-d757e0d131cebbebb0176e5448e53b54970ebb64f1c5d7f01537a4e221e587b0R175-R176 |
I'll try this tonight, thank you for metion this. |
6abfae6
to
bc6be4b
Compare
bc6be4b
to
b2e3b88
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need new traits for all of them, we can just use what is already there.
I'll apply all Basti's suggests then squash them |
@bkchr I basically follow how AssetId does, do you think AssetId (and maybe Balance) can apply this? |
14c1b8b
to
cc29fe6
Compare
can you pls run |
Good questions :P IDK, I would need to check the code. |
bot fmt |
@bkchr https://gitlab.parity.io/parity/mirrors/substrate/-/jobs/2504577 was started for your command Comment |
@bkchr Command |
GREAT! Thank you ! |
@bkchr Could you help to merge this? |
bot merge |
I don't know this is the right direction but I believe this would help when people interacting with NFT traits return values in their own pallets
Basically I'm imitating how fungible traits does
Background:
I'm learning NFTs pallet so I'm building my own pallet that invoking pallet_nfts via nonfungibles_v2 interface,
I tried:
then, I found few problems:
create_collection
returnscollection_id
but can't be used in event's payload becausethe trait `Clone` is not implemented for `<<T as pallet::Config>::NonFungibles as frame_support::traits::tokens::nonfungibles_v2::Inspect<<T as frame_system::Config>::AccountId>>::CollectionId
mint_into
requiresItemConfig
but it can be constructed out of pallet_nftsand a question:
How to construct CollectionId and ItemId from a 3rd pallet?