NFL NFT contract #1
Conversation
contracts/Genies.cdc
Outdated
| } | ||
| } | ||
|
|
||
| // An Edition (NFT type) within a Genies collection | ||
| // A top level Edition that contains a Series, Set, and Play | ||
| // | ||
| pub resource Edition { |
There was a problem hiding this comment.
Note that we do not currently enforce unique set/play for Editions. This can be implemented by adding a dictionary of Plays in a Set and checking that before creating, or by creating a UInt64 from the SetID and PlayID then storing in a contract-level dictionary if they need to be checked on-chain.
There was a problem hiding this comment.
I had a stab at this (though using similar logic to the edition function so let me know if it's wacky). I think it makes sense to store the dictionary on a Set to avoid a large global dictionary which could be huge/never-ending. Here's what I added: 3b3db35
My intent was to create the dictionary on the set, then add two functions - one to check if we already have that Play within the dictionary, and one to insert a new Play. Then call these - the check in the pre-condition for creating a new Edition, and the insert before we emit the event.
There was a problem hiding this comment.
Yes that all sounds great.
When testing, keep an eye on storage usage by these structures. They shouldn't get tooooooo big, but it's good to have a benchmark for how much they may cost over time.
There was a problem hiding this comment.
Good call! How do you usually test how much storage a dictionary entry would take up?
There was a problem hiding this comment.
@rheaplex just noticed this comment from a little while back! Is there a good way of testing storage costs of a dictionary?
contracts/Showdown.cdc
Outdated
| self.nextSeriesID = 1 | ||
| self.nextSetID = 1 | ||
| self.nextPlayID = 1 | ||
| self.nextEditionID = 1 |
There was a problem hiding this comment.
I started the FlowIDs at 1 instead of 0 for better ease when dealing with our backend. (eg. entities that we create before minting can legit have a null FlowID, but there's a higher margin of error when we use 0 if it converts from null). Please shout if you feel strongly that this is bad practise!
There was a problem hiding this comment.
@rheaplex we have discussed about this before in Genies. Let us know if you have any opinions about this.
|
@EricLin2004 @rheaplex @judezhu Addressed comments and updated! I've also re-named the contract to |
| return &self.ownedNFTs[id] as &NonFungibleToken.NFT | ||
| } | ||
|
|
||
| // borrowMomentNFT gets a reference to an NFT in the collection |
There was a problem hiding this comment.
Possibly borrowAllDayNFT , although this is fine.
There was a problem hiding this comment.
Product is keen on keeping Moment as our identifier - (they also might change it to something else 😅 ) but I think this way we have a bit more differentiation between a packNFT and momentNFT
|
👍👍👍👍👍🥳 |
First pass at updating the Genies contract for NFL. A few main things:
advancingof a series as we can now have multiple series open at once.I haven't built in the logic for validating that the SetID/PlayID combo has not been used in a previous Edition, or the logic for properly keeping track of minted moments, but wanted to get a sense check of how this looks so far. I've added some comments to a few things I'm a little hazy on.