Sisyphus smiling/player custodied async moves

[sisyphusSmiling]

Closes: #19 #20

Description

Changes in this PR attempt to provide player-mediated asynchronous gameplay as well as showcase mutation of local NFT attributes by the game contract.

We've moved from a centrally mediate Match via a game administrator, to a disintermediated Match where players can create matches & submit their own moves. The notion of a game lobby is brought into play by MatchLobbyActions which enables a player to escrow their NFT into a Match passing also their NonFungibleToken.Receiver and a reference to their GamePlayerID Capability. The former Capability allows the game to return their NFT directly, and the latter reference is used to tie the NFT to some notion of identity (similar to the notion of a player handle). Upon escrow, the player is given a MatchPlayerActions Capability which can be used to submit their move. A reference to their GamePlayerID is required again on move submission to prevent a player from submitting another player's move as an attempt to cheat the game.

On the NFT side, the GamePieceNFT contract adds gameMoves attribute to its NFT and an NFTEscrow interface enabling implementing resources to add and remove moves from escrowed NFTs. This construction showcases how we might enable external contracts to mutate local NFT attributes. Instead of trusting a game to honestly provide its name when altering NFT data, we have the game register its desired name with the GamePieceNFT contract, returning a GameRegistrationTicket upon registration containing the name which must be provide by reference whenever a game wants to act on an NFTs gameMoves. This limits the scope an external actor has edit access to and mitigates a namespace spam vector by requiring a fee upon registration.

A note to add here is that the NFTEscrow resource includes default implementations that give implementing resources exposure to access(contract) methods in GamePieceNFT. This is the mechanism used to enable external actors the ability to edit NFT attributes. However, recent conversations with the Cadence team have revealed that default implementations may be changed to only allow only view methods. While there aren't formal plans for these changes at this time, I'm hoping to open discussion around alternative mechanisms enabling authorized parties to edit NFT attributes.

P.S. The transaction diagrams in the README will be updated in the next day.

---

For contributor use:

• Targeted PR against master branch
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the standards mentioned here.
• Updated relevant documentation
• Re-reviewed Files changed in the Github PR explorer
• Added appropriate labels

[joshuahannan]

I think that since this version of the contract is illustrating a different way of doing things (using attachments instead of centrally stored metadata), we should create different directories for each use case each with their own README so we can educate people on the different options.

Also, I said this in a comment, but I think it is worth mentioning here too that it would probably make sense to remove any imports and mentions of GamePieceNFT from this contract, because I think we're trying to make it possible to play a game with any resource that implements the correct metadata attachments, correct? Making it only compatible with our GamepieceNFT doesn't really seem right. This may be something I missed in the first version of the contract too, so that might warrant another look there

[joshuahannan]

I would hope that we could turn these into views somehow. There is not much point in defining special metadata attachments if we cannot access them easily with metadata views

[sisyphusSmiling]

Given that we're generalizing GamePieceNFT to receive @AnyResource{GamingMetadataViews.Attachment}, I'm not sure how to resolve views on the NFT. Maybe we create an AttachmentView, and return a dictionary of attachments?

The intention in generalizing attachments was to make the GamePieceNFT as generic as possible. A game could attach game moves or power ups or gameplay achievement badges - really anything the game thought would be relevant to its gameplay.

The problem from within the NFT contract then is how to resolve a specific view for that Attachment? Maybe we can have Attachment implementations define their own View? Then getView() can take the Type of View and resolve if it exists on the NFT?

That could look something like:

pub resource MyAttachment : GamingMetadataViews.Attachment {
  pub let attachmentFor: [Type]
  pub let name: String
  init() {
    self.attachmentFor = [Type<@GamePieceNFT.NFT>]
    self.name = "Gio"
  }
}

pub struct MyAttachmentView {
  pub let id: UInt64
  pub let name: String
  init(id: UInt64, name: String) {
    self.id = id
    self.name = name
  }
}

pub resource NFT : NonFungibleToken.INFT, MetadataViews.Resolve, GamingMetadataViews.Attachable {
  pub let id: UInt64
  access(contract) let attachments: @{Type: AnyResource{GamingMetadataViews.Attachment}}
  pub let attachmentViews: {Type: AnyStruct}

  pub fun getViews(): [Type] {
    return self.attachmentViews.keys
  }

  pub fun resolveView(_ view: Type): AnyStruct? {
    return self.attachedViews[view]
  }

  pub fun addView(view: Type, viewStruct: AnyStruct) {
    self.attachmentViews.insert(key: view, viewStruct)
  }
}

Alternatively, what if we add a resolveView() method in the Attachment interface? Then we could resolveView() in the NFT on the Type of Attachment, something like:

pub resource NFT : NonFungibleToken.INFT, MetadataViews.Resolve, GamingMetadataViews.Attachable {
...
  pub fun resolveView(_ view: Type): AnyStruct? {
    if let attachment = self.attachments[type] {
      return attachment.resolveView()
    }
    return nil
  }
...
}

I'm more partial to the second approach.

[sisyphusSmiling]

I've been thinking about this, and I'm finding it difficult to implement a solution with the standards we've come to expect from @{NonFungibleToken.INFT, MetadataViews.Resolver}. This is because, while we can implement Resolver in the attachments themselves, the fact that the NFT can have multiple attachments that support the same view leads to ambiguity.

For example, let's say I have attachments {DynamicNFT.Attachment, MetadataViews.Resolver, GamingMetadataViews.BasicWinLossRetriever} as follows:

pub resource AttachmentA : Attachment, MDViews.Resolver {
    pub fun getViews(): [Type] {
        return [Type<SomeStruct>()]
    }
    pub fun resolveView(_ view: Type): AnyStruct? {
        switch {
            case Type<SomeStruct>():
                return SomeStruct(name: "AttachmentA")
        }
    }
}

pub resource AttachmentB : Attachment, MDViews.Resolver {
    pub fun getViews(): [Type] {
        return [Type<SomeStruct>()]
    }
    pub fun resolveView(_ view: Type): AnyStruct? {
        switch {
            case Type<SomeStruct>():
                return SomeStruct(name: "AttachmentB")
        }
    }
}

Assuming both are attached to the NFT, how would we know which to view to resolve when we call nft.resolveView(Type<SomeStruct>()) unless we also define the attachment we want the view from?

With that in mind, I'm thinking we have two options:

1. Include a ViewResolver interface in which we also pass the type of the attachment for which we want to resolve the view...something like pub fun resolveAttachmentView(attachmentType: Type, view: Type): AnyStruct?. We should probably also implement a getAttachmentViews() method in this case as well.
2. Implement MetadataViews.Resolver in our implementations of Attachment so that the attachment themselves can resolve views via their reference. This will be necessary whether we proceed with the first option or not.

Appreciate any thoughts on this!

[joshuahannan]

We want the view resolver on the NFTs to return all the usual NFT views, like Display, Serial, Edition, etc, but we'd also want a view that shows what attachments are on the NFT. That can be an array, a dictionary, or a struct that contains the attachments. it is up to you.

[joshuahannan]

In the attachment view struct you define, instead of just having an array of types, you can also have an array of view resolvers for each type or something

[alilloig]

We are doing the same on ExampleNFT but the LS server is pointing out that this casting is unnecessary, I'll check it when we have tests!

[alilloig]

nice work on the views Gio! Could it be possible that we want to create a View for exposing the GameContractMetadata so it's easy to know which games the NFT is being used for?

[sisyphusSmiling]

@alilloig Good idea. I'll add the functionality shortly

[alilloig]

Since we are not using this function anymore, can we get rid of it? I 100% agree on escrowing the NFT straight to the match

[sisyphusSmiling]

Oh right, I refactored to use escrowNFTToMatch() but forgot to remove escrowNFT(). Good catch!

[alilloig]

Could we change the name of this tx to something like "signup_to_match", "signup_to_existing_match", "join_match", "join_match_and_escrow_nft"...

[alilloig]

Tests are passing, there's always the chance that we are skipping some edge case but we have addressed a bunch of potential problems. I will love to have a more detailed test logic in one of the cases @satyamakgec @sisyphusSmiling do you know if there's something such as shallFail on the js-testing for easily check if a tx that should not succeed effectively doesn't?

Probably we can merge this and update and focus on the child-account branch to ensure everything about the wallet-less onboarding is ready.

[sisyphusSmiling]

Thanks for getting the tests done! @alilloig I believe you're looking for the shallRevert() jest helper, yeah?

[alilloig]

yes, thank you @sisyphusSmiling !! Now the full test case includes a player trying to cheat and failing

[joshuahannan]

I still think that the name Attachments should be changed to something else because it conflicts with the attachments Cadence language feature. It can be confusing. Or maybe just wait until the language feature is live and just use it instead if these attachments are similar enough. But I understand if we're in a hurry

Also, would it be possible to make this proposal public so we can get community feedback about it before committing to it? I think the core features and especially the metadata views really need more feedback from real game developers before we can be confident about them

[joshuahannan]

The comments should also make it more clear which resource should implement this interface.

I think this comment applies to most of the interfaces defined here. it is a little confusing trying to piece where these fit in without more guidance

[sisyphusSmiling]

Thanks for the thorough review @joshuahannan!
Re: opening up for discussion on Metadata views - I'm open to that, though that feels like a refinement that can happen in later iterations. Our primary task this sprint is to get the core functionality to a stable place so the client-side work can start to make progress. Beyond the core functionality contained in this branch's suite of contracts, we'll want to add support for child accounts as an additional layer which is a dependency for front end work.
Re: Attachments - Given the compressed DSS sprint timeline, I'm thinking that I'll work on implementing that this week for use with flow-cli 0.43.0, though it will likely break our tests.

So, to clarify, this version won't contain the child account layer, but will contain the updated Attachments. As you said, creating our own attachment-like resources is confusing. And, since the feature will be released soon and the demo will be released around the upcoming spork, we may as well refactor to implement the native Attachment feature. I'm planning on submitting the child account suite of contracts as a separate PR after this one is merged as it builds on the core functionality contained here.

[satyamakgec]

The rest of the things look fine to me.

[alilloig]

@joshuahannan we are merging this PR for having it as the base for the two incoming PRs for native attachments and auth account cap, we made an issue #30 for fixing the troll receiver when returning NFTs from a match