Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jul 11, 2024
Merged

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

merged 5 commits into from
Jul 11, 2024

Conversation

amnn
Copy link
Member

@amnn amnn commented Jul 4, 2024

Description

Port Move code for the tic tac toe example into the sui/examples directory, modernising it in the process:

  • Standardise the implementation between the owned, shared and multi-sig variants, so that the only differences are relevant to what they need to do differently. In doing this, the owned and multi-sig variants become identical (at least at the level of Move code).
  • Use transfer-to-object in the "owned" example, to send potential moves to the Game object, instead of the admin that owns the game.
  • Use more Move 2024 features (syntax(index), receiver function aliases, etc).
  • Improve test coverage

Test plan

New unit tests:

tic_tac_toe/move$ sui move test

Release notes

Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.

For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.

  • Protocol:
  • Nodes (Validators and Full nodes):
  • Indexer:
  • JSON-RPC:
  • GraphQL:
  • CLI:
  • Rust SDK:

@amnn amnn requested review from nikos-terzo and a team July 4, 2024 14:36
@amnn amnn self-assigned this Jul 4, 2024
Copy link

vercel bot commented Jul 4, 2024

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:50am
3 Skipped Deployments
Name Status Preview Comments Updated (UTC)
multisig-toolkit ⬜️ Ignored (Inspect) Visit Preview Jul 11, 2024 10:50am
sui-kiosk ⬜️ Ignored (Inspect) Visit Preview Jul 11, 2024 10:50am
sui-typescript-docs ⬜️ Ignored (Inspect) Visit Preview Jul 11, 2024 10:50am

edition = "2024.beta"

[dependencies]
Sui = { local = "../../../crates/sui-framework/packages/sui-framework" }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we depend on the local framework here? I remember on the trading demo, we did depend on a live sui branch (testnet iirc) to make these easy to copy + play with outside of the monorepo. I think whichever route we choose, we should do it consistently!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should depend on the local one everywhere for now, so that CI doesn't have to refetch the sui repo. This problem should go away once we eliminate the sui framework as an explicit dependency.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can make the change to the trading contracts!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +93 to +91
const MARK__: u8 = 0;
const MARK_X: u8 = 1;
const MARK_O: u8 = 2;

// Trophy status
const TROPHY_NONE: u8 = 0;
const TROPHY_DRAW: u8 = 1;
const TROPHY_WIN: u8 = 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we wait a few weeks and just use enums?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should wait a few weeks to land the docs change, but we should come back and update it once enums is generally available, yes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flagging for @tzakian

examples/tic-tac-toe/move/sources/owned.move Outdated Show resolved Hide resolved
Comment on lines +261 to +260
if (game.turn % 2 == 0) {
(game.x, game.o, MARK_X)
} else {
(game.o, game.x, MARK_O)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (game.turn % 2 == 0) {
(game.x, game.o, MARK_X)
} else {
(game.o, game.x, MARK_O)
}
if (game.turn % 2 == 0) (game.x, game.o, MARK_X)
else (game.o, game.x, MARK_O)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm not a huge fan of this change, because not having the two cases aligned makes it difficult to note the differences between them.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (game.turn % 2 == 0) {
(game.x, game.o, MARK_X)
} else {
(game.o, game.x, MARK_O)
}
if (game.turn % 2 == 0) (game.x, game.o, MARK_X)
else (game.o, game.x, MARK_O)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kidding of course :p
And fair enough. I would always say to do what you think is more readable

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't tempt me, in a past life, I've been known to do worse.

Copy link
Contributor

@tnowacki tnowacki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, especially if we are going to do another pass in a few weeks (nudge nudge @tzakian)

MARK__, MARK__, MARK__,
MARK__, MARK__, MARK__,
],

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

MARK__, MARK__, MARK__,
MARK__, MARK__, MARK__,
],

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change

amnn added 5 commits July 11, 2024 11:36
## Description

Port Move code for the tic tac toe example into the `sui/examples`
directory, modernising it in the process:

- Standardise the implementation between the owned, shared and
  multi-sig variants, so that the only differences are relevant to what
  they need to do differently. In doing this, the owned and multi-sig
  variants become identical (at least at the level of Move code).
- Use transfer-to-object in the "owned" example, to send potential
  moves to the `Game` object, instead of the admin that owns the game.
- Use more Move 2024 features (`syntax(index)`, receiver function
  aliases, etc).
- Improve test coverage

## Test plan

New unit tests:

```
tic_tac_toe/move$ sui move test
```
@amnn amnn force-pushed the amnn/eg-ttt-move branch from bc26e69 to 16836c6 Compare July 11, 2024 10:49
@amnn amnn merged commit 8a259b3 into main Jul 11, 2024
51 checks passed
@amnn amnn deleted the amnn/eg-ttt-move branch July 11, 2024 11:36
amnn added a commit that referenced this pull request Jul 11, 2024
## 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
![shared
game](https://github.com/MystenLabs/sui/assets/332275/a7b5adcf-38b9-40b6-a01a-a0d0e1fe1836)

#### New MultiSig game
![multi-sig
game](https://github.com/MystenLabs/sui/assets/332275/6682c0e8-973a-408d-9696-1cc6fafcccbd)

#### Game in-progress
![game
in-progress](https://github.com/MystenLabs/sui/assets/332275/ee102324-ce07-4855-a52d-56376a812e0f)

#### Game won, viewed by player
![game
win](https://github.com/MystenLabs/sui/assets/332275/e1ede3f8-fe3c-4d3a-a189-6eacdb6967ae)

#### Game won, viewed by spectator (no wallet connected)
![game win
spectator](https://github.com/MystenLabs/sui/assets/332275/ae6f9eba-bb0b-4223-b08d-9435cce2938d)

#### Delete game dialog
![delete
game](https://github.com/MystenLabs/sui/assets/332275/29169cd7-544d-4591-ac68-6ddda6bb7ef7)

## Stack

- #18525
amnn added a commit that referenced this pull request Jul 12, 2024
## Description

Write a CLI to talk to the shared and multi-sig tic-tac-toe example
Move packages.

## Test plan

Manual testing:

```
$ tic-tac-toe view 0xab06bb92cc1e6c95f9e6eed7e6c01fade6bb5eaa241e2100eb54d914597295ed
                                X | O |
                               ---+---+---
                                O | X |
                               ---+---+---
                                  |   | X

    X: <address>
 -> O: <address>
 GAME: 0xab06bb92cc1e6c95f9e6eed7e6c01fade6bb5eaa241e2100eb54d914597295ed
ADMIN: <public key>

                                 X WINS!
```

Also, confirmed that the CLI works with the React front-end.

## Stack

- #18525 
- #18526 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL:
- [ ] CLI: 
- [ ] Rust SDK:
amnn added a commit that referenced this pull request Jul 12, 2024
## Description 

A README for the tic-tac-toe app example, derived from:

https://github.com/MystenLabs/multisig_tic-tac-toe/blob/main/README.md

And updates to the docs for the tic-tac-toe app example to reflect
changes to
modernise the guide and bring all its source code into the `sui`
mono-repo.

This is the last major example that lived in the `sui_programmability`
directory.

## Test plan 

:eyes:

## Stack

- #18525 
- #18526 
- #18557 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:

---------

Co-authored-by: Ronny Roland <[email protected]>
amnn added a commit that referenced this pull request Jul 12, 2024
…8595)

## Description

Replace all references to Move packages in `sui_programmability` with
equivalents in `./examples/move`, in preparation for deleting the
programmability directory.

In the process, I also:

- removed the tic-tac-toe example from the Sui SDK, as it has been
replaced by a more full-featured E2E example.
- ported some modernised versions of the `basics` packages into the new
`examples/move/basics` for use in tests.

## Test plan

CI and,

```
sui$ cargo simtest
```

## Stack

- #18525 
- #18526 
- #18557 
- #18558 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:

---------

Co-authored-by: Ronny Roland <[email protected]>
amnn added a commit that referenced this pull request Jul 12, 2024
…18609)

## Description

Port over the following examples from sui_programmability/examples:

- crypto/sources/ecdsa.move
- crypto/sources/groth16.move
- ~games/sources/drand_lib.move~
- ~games/sources/drand_based_lottery.move~
- ~games/sources/drand_based_scratch_card.move~
- games/sources/vdf_based_lottery.move

Modernising and cleaning them up in the process:

- Applying wrapping consistently at 100 characters, and cleaning up
comments.
- Removing unnecessary use of `entry` functions, including returning
values instead of transfering to sender in some cases.
- Using receiver functions where possible.
- Standardising file order and adding titles for sections.
- Standardising use of doc comments vs regular comments.
- Using clever errors.

This marks the final set of examples to be moved out of
sui-programmability, which will then be deleted.

## Test plan

```
sui-framework-tests$ cargo nextest run -- run_examples_move_unit_tests
```

## Stack

- #18525 
- #18526 
- #18557 
- #18558
- #18595 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:

---------

Co-authored-by: Ronny Roland <[email protected]>
amnn added a commit that referenced this pull request Jul 12, 2024
## Description

Remove the `sui_programmability` folder as all examples have been ported
and modernised to `examples/move`, or elsewhere.

## Test plan

CI

## Stack
- #18525 
- #18526 
- #18557 
- #18558
- #18595 
- #18609 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:

---------

Co-authored-by: Ronny Roland <[email protected]>
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
## Description

Port Move code for the tic tac toe example into the `sui/examples`
directory, modernising it in the process:

- Standardise the implementation between the owned, shared and multi-sig
variants, so that the only differences are relevant to what they need to
do differently. In doing this, the owned and multi-sig variants become
identical (at least at the level of Move code).
- Use transfer-to-object in the "owned" example, to send potential moves
to the `Game` object, instead of the admin that owns the game.
- Use more Move 2024 features (`syntax(index)`, receiver function
aliases, etc).
- Improve test coverage

## Test plan

New unit tests:

```
tic_tac_toe/move$ sui move test
```

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
## 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
![shared
game](https://github.com/MystenLabs/sui/assets/332275/a7b5adcf-38b9-40b6-a01a-a0d0e1fe1836)

#### New MultiSig game
![multi-sig
game](https://github.com/MystenLabs/sui/assets/332275/6682c0e8-973a-408d-9696-1cc6fafcccbd)

#### Game in-progress
![game
in-progress](https://github.com/MystenLabs/sui/assets/332275/ee102324-ce07-4855-a52d-56376a812e0f)

#### Game won, viewed by player
![game
win](https://github.com/MystenLabs/sui/assets/332275/e1ede3f8-fe3c-4d3a-a189-6eacdb6967ae)

#### Game won, viewed by spectator (no wallet connected)
![game win
spectator](https://github.com/MystenLabs/sui/assets/332275/ae6f9eba-bb0b-4223-b08d-9435cce2938d)

#### Delete game dialog
![delete
game](https://github.com/MystenLabs/sui/assets/332275/29169cd7-544d-4591-ac68-6ddda6bb7ef7)

## Stack

- MystenLabs#18525
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
## Description

Write a CLI to talk to the shared and multi-sig tic-tac-toe example
Move packages.

## Test plan

Manual testing:

```
$ tic-tac-toe view 0xab06bb92cc1e6c95f9e6eed7e6c01fade6bb5eaa241e2100eb54d914597295ed
                                X | O |
                               ---+---+---
                                O | X |
                               ---+---+---
                                  |   | X

    X: <address>
 -> O: <address>
 GAME: 0xab06bb92cc1e6c95f9e6eed7e6c01fade6bb5eaa241e2100eb54d914597295ed
ADMIN: <public key>

                                 X WINS!
```

Also, confirmed that the CLI works with the React front-end.

## Stack

- MystenLabs#18525 
- MystenLabs#18526 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL:
- [ ] CLI: 
- [ ] Rust SDK:
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
## Description 

A README for the tic-tac-toe app example, derived from:

https://github.com/MystenLabs/multisig_tic-tac-toe/blob/main/README.md

And updates to the docs for the tic-tac-toe app example to reflect
changes to
modernise the guide and bring all its source code into the `sui`
mono-repo.

This is the last major example that lived in the `sui_programmability`
directory.

## Test plan 

:eyes:

## Stack

- MystenLabs#18525 
- MystenLabs#18526 
- MystenLabs#18557 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:

---------

Co-authored-by: Ronny Roland <[email protected]>
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
…stenLabs#18595)

## Description

Replace all references to Move packages in `sui_programmability` with
equivalents in `./examples/move`, in preparation for deleting the
programmability directory.

In the process, I also:

- removed the tic-tac-toe example from the Sui SDK, as it has been
replaced by a more full-featured E2E example.
- ported some modernised versions of the `basics` packages into the new
`examples/move/basics` for use in tests.

## Test plan

CI and,

```
sui$ cargo simtest
```

## Stack

- MystenLabs#18525 
- MystenLabs#18526 
- MystenLabs#18557 
- MystenLabs#18558 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:

---------

Co-authored-by: Ronny Roland <[email protected]>
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
…ystenLabs#18609)

## Description

Port over the following examples from sui_programmability/examples:

- crypto/sources/ecdsa.move
- crypto/sources/groth16.move
- ~games/sources/drand_lib.move~
- ~games/sources/drand_based_lottery.move~
- ~games/sources/drand_based_scratch_card.move~
- games/sources/vdf_based_lottery.move

Modernising and cleaning them up in the process:

- Applying wrapping consistently at 100 characters, and cleaning up
comments.
- Removing unnecessary use of `entry` functions, including returning
values instead of transfering to sender in some cases.
- Using receiver functions where possible.
- Standardising file order and adding titles for sections.
- Standardising use of doc comments vs regular comments.
- Using clever errors.

This marks the final set of examples to be moved out of
sui-programmability, which will then be deleted.

## Test plan

```
sui-framework-tests$ cargo nextest run -- run_examples_move_unit_tests
```

## Stack

- MystenLabs#18525 
- MystenLabs#18526 
- MystenLabs#18557 
- MystenLabs#18558
- MystenLabs#18595 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:

---------

Co-authored-by: Ronny Roland <[email protected]>
tx-tomcat pushed a commit to tx-tomcat/sui-network that referenced this pull request Jul 29, 2024
## Description

Remove the `sui_programmability` folder as all examples have been ported
and modernised to `examples/move`, or elsewhere.

## Test plan

CI

## Stack
- MystenLabs#18525 
- MystenLabs#18526 
- MystenLabs#18557 
- MystenLabs#18558
- MystenLabs#18595 
- MystenLabs#18609 

---

## Release notes

Check each box that your changes affect. If none of the boxes relate to
your changes, release notes aren't required.

For each box you select, include information after the relevant heading
that describes the impact of your changes that a user might notice and
any actions they must take to implement updates.

- [ ] Protocol: 
- [ ] Nodes (Validators and Full nodes): 
- [ ] Indexer: 
- [ ] JSON-RPC: 
- [ ] GraphQL: 
- [ ] CLI: 
- [ ] Rust SDK:

---------

Co-authored-by: Ronny Roland <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants