[Examples/Front-end] Tic-tac-toe

[amnn]

Description

Build front-ends for shared and multi-sig tic-tac-toe, using
create-react-dapp.

Test plan

Manually tested (screenshots below).

New Shared game

New MultiSig game

Game in-progress

Game won, viewed by player

Game won, viewed by spectator (no wallet connected)

Delete game dialog

Stack

• [Examples/Move] Tic-tac-toe #18525

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
sui-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 11, 2024 10:52am

3 Skipped Deployments

Name	Status	Preview	Comments	Updated (UTC)
multisig-toolkit	⬜️ Ignored (Inspect)	Visit Preview		Jul 11, 2024 10:52am
sui-kiosk	⬜️ Ignored (Inspect)	Visit Preview		Jul 11, 2024 10:52am
sui-typescript-docs	⬜️ Ignored (Inspect)	Visit Preview		Jul 11, 2024 10:52am

[amnn]

Leaving some notes to help direct the review.

[amnn]

Refetching ... I'm using refetching in react-query to make sure the tic tac toe board gets updated when someone else (another player) makes a move. At the moment, I have to go to each query that's feeds the main game screen and add a refetch interval, which is not great. I would rather tell the root query (this one) to refetch, and then have the effects of the cascade everywhere (e.g. when the refetch spots that the game has changed, we trigger the other refetches).

[manolisliolios]

So there are two different ways that probably answer this question:

1. Prepend a "key" element into all query keys (so when you invalidate that, you also invalidate all the nested ones)

e.g. queryKey: ['core', 'inner', 'more']

When you invalidate core, it will invalidate all queries that start with core.

2. You can add global settings when you instatiate the QueryClient on your main.tsx file.

new QueryClient({
  defaultOptions: {
    queries: { ... }
  }
});

[amnn]

I like option (1) a fair bit, but it doesn't play too nicely with how dapp-kit currently handles keys (you can only supply a key suffix, not a prefix) -- I wonder if there's a way we can tweak the way dapp-kit works to allow these kind of patterns, cc @hayes-mysten . (2) seems less ideal to me, because it sounds like we would switch things around to make refetching the default, and then have to manually turn it off in places, which would work for our case here, but I'm not sure how generalisable that is.

What I was originally hoping was that there was a way to describe the data dependency, so that react-query could detect that there was a change in the query input and therefore re-run it, in which case we could treat our game as a redundant input to all the other queries, and use that mechanism to trigger the refetches -- is that feasible at all?

[hayes-mysten]

Going to defer to @williamrobertson13 here who has some more react/front-end expertise

[hayes-mysten]

this is definitely something we should have a good (and well documented) solution for

[amnn]

Is there a better way to serialize a Uint8Array as a vector<u8> parameter?

[manolisliolios]

I think what you did is the recommended way. @hayes-mysten to confirm!

[hayes-mysten]

This is mostly the correct way to do this, but the last toBytes isn't needed: tx.pure(bcs.vector(bcs.u8()).serialize(admin.toRawBytes()))

bcs.bytes() is an existing method that is used for fixedLength bytes, so I think also calling vector bytes would be confusing.

I just opened a PR to make this a little better though: https://github.com/MystenLabs/sui/pull/18561/files

This will add tx.pure.vector('u8', admin.toRawBytes())

[amnn]

My attempt at trying to fetch a single object from among an address' owned objects. Relying on a vanilla useQuery instead of useSuiClientQuery of the infinite variety because I want to read potentially multiple pages in one query to find the value I want -- let me know if there's a better way.

[amnn]

The lines on the board are actually the background of the .board class showing through (and the white squares are the background of each cell). I'm a CSS/flexbox noob, so this seemed to me the cleanest way to do this, but let me know if there's a more straightforward way.

[amnn]

@hayes-mysten, as promised, here's the experience report from modernising this demo!

I'm going to focus on the areas where I struggled, but I wanted to start by saying that dApp Kit, Radix UI, React, and React Query were great -- they significantly sped things up for me, and I'm confident I wouldn't have been able to deliver anything near the quality I was able to without them. A big 👍 for starting with an opinionated stack of tools and libraries that work well together, because they taught me a lot about what's available, and what the best practices are.

Some of the things I found tricky are specific to me being a front-end/TypeScript noob. I tried to filter those things out, but I may not have succeeded (don't know what I don't know!), but here goes:

Wallet <-> NetworkConfig syncing

This was an early stumbling block because NetworkConfig defaults to testnet and I was building on localnet, so without a way to switch networks, I was a bit stuck. Initially, I was able to unblock myself by changing the default to localnet, but this may not be a sensible default. Eventually, I added some logic to query the current account, its network, check whether we have a config for that network, and set it if so (or error otherwise).

I wonder whether we can package this behaviour up as a default? So that out of the box, the app will switch its config according to the network the wallet sets.

Similarly, I had to roll my own script to deploy a package, and update the dApp's configs to pick up the newly deployed package -- is this something we can bake into create-react-dapp? (I used a bash script, but I imagine if we were to do it in create-react-dapp, it would be written in TypeScript).

Address <-> App Stickiness

This one's a known issue, I got stuck trying to connect some other address to the app during testing. My understanding is that you need to go into the wallet settings and dissociate the app from that address and then reconnect.

My expectation here is that once you connect a wallet, you should be able to select addresses from that wallet to connect to the app using the drop-down, but only one address ever shows here.

Query Invalidation

Main issue here was that it was not clear what the default query key was -- I was supplying a key as a parameter, but I didn't realise that it was being prefixed with some other stuff. There's a couple of ways I could see this being solved:

• Docs -- a doc comment would have saved a lot of trouble here.
• Not adding the prefix if the queryKey parameter is set.
• Abstracting away query invalidation (this is what I ended up doing). I ended up adopting a pattern where query functions returned a tuple -- one element being the results, and the other being a function that invalidates the query. I found this pretty easy to work with, but it maybe falls down if you want to invalidate multiple queries simultaneously. Maybe a middle ground is to have the function return the query key it used?

Docs in LSP

Finding docs in the LSP was a struggle, and it is made more complicated by the fact that the symbols the language server like to jump to are usually just type declarations, and there are some pretty exotic types here. This wasn't a problem that was unique to our libraries though -- while the standard library and maybe core React hooks had good documentation, many other libraries didn't, which is a shame.

Transforming an Object on Load

When querying the Game object over RPC, I needed to perform some extra checks and transformations. In some cases the checks would yield errors. I ended up doing this in a custom hook, but it might be nice to allow users to provide a transformation function in a call to useSuiClientQuery, (it's particularly useful when the extra work/transformation may yield an error).

Radix UI is now at 3.x

create-react-dapp depends on Radix at 2.x, but it's at 3.x now. Radix doesn't keep around legacy docs so I ended up upgrading it to make sure the APIs matched what I was seeing on their site -- can we bump it in the template?

Importing modules

I found the pattern of always using relative imports to be quite jarring -- is that the best practice? In the end, I installed a vite plugin that would let me depend on modules relative to the src directory, which I found much more manageable when dealing with dependencies across different sub-directories. If that seems reasonable, can we add that template and the associated baseUrl configuration to the template?

Aliased types

There are some types (the ones I can think of are MultiSigPublicKey and PublicKey, but there may be more) that appear under sui/client, sui/cryptography, and sui/multisig. This means that sometimes the language server can end up suggesting the wrong one for import -- it would be great if we could deduplicate these.

vector<u8> transaction inputs

The new tx.pure API is miles better than what it was before, but I still ended up tripping over the old issue of whether or not to pre-serialize an input when I was holding a Uint8Array and I wanted to pass it to a vector<u8> parameter. In the end, I manually serialized it using the bcs library, but it seems reasonable to add a tx.pure.bytes or something for this case?

Wallet Standards's signAndExecuteTransaction vs SuiClient.signAndExecuteTransaction

I didn't actually trip up on this one during my own development, but noticed someone on Telegram had this issue recently and got tripped up on it that way.

When would someone choose to use the execution via wallet standard vs their own SuiClient? It seems like we're offering two similar strategies for wallets and apps to communicate with each other, but I can't really see a clear differentiator (where one is more appropriate than the other), and the choice itself can be confusing.

Wallet Standard Public Keys

The Wallet Standard returns public keys as raw bytes, and as far as I could tell, there was not a way to figure out what the signature scheme was for that public key. In this demo, I assumed ED25519, but if a wallet is using some other kind of key, this could spell trouble. We have a byte form that is annotate ("Sui" Bytes), can we use that instead?

EDIT One more paper cut.
EDIT 2 Remembered another paper cut!

[amnn]

Here useCurrentAccount has returned some bytes for the public key, and they seem to be raw -- how would a wallet return the public key when the signature scheme was not ED25519?

[hayes-mysten]

the walletStandard doesn't expose enough information to make this work. Generally dapps are built around addresses, not public keys, and some wallet accounts won't return meaningful publicKey bytes (eg stashed).

That being said, you theoretically could try each supported schema, and see if it produces a pubKey that has the same address as the account

[amnn]

Would it be feasible to have the wallet standard return a public key in "SuiBytes" form? (I believe it's just an enum, so it's the raw bytes prefixed with a variant index), or add an optional signature scheme field, so I could write something like:

const { address, publicKey: bytes, keySignature } = useCurrentAccount() || {};
const publicKey = keySignature && bytes && publicKeyFromRawBytes(keySignature, bytes);

[manolisliolios]

What might be convenient is to export all queryKey names so you could theoretically invalidate keys more generic throughout the app, if you needed to. E.g. QueryKeys { GameEndState, ...}, instead of having to expose an "invalidate" function on each of these.

Returning the "useQuery" hook as is, also exports a refetch function that you can use to re-fetch the values, instead of re-defining a new invalidate function on every hook.

[amnn]

I thought about this, but I think again it kind of runs counter to the way dapp-kit is working, which abstracts away the query keys in its queries. Maybe if we can align on that, this would be a good way to go.

[manolisliolios]

Thanks a lot Ashok. Approving to unblock, overall this looks amazing. UX is super smooth! 🚀

Added some comments / suggestions here and there!

[amnn]

Just finishing up addressing the other comments, but I'm noticing that something is still not quite right with the workspace set-up, so could use some advice on that @manolisliolios / @hayes-mysten .

It looks like pnpm install --frozen-lockfile still believes the package.json in examples/tic-tac-toe/ui is part of the workspace, even though it has its own lock-file.

I was trying to follow the path that the trading example took, which didn't seem to include anything to exclude it from the workspace, but now I'm wondering whether that was just a happy coincidence -- i.e. it happens to use all the same dependencies.

Is the right thing to do here to exclude all the examples from the workspace, and ensure they each have their own lock files?