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

Add rock-paper-scissors game example #1479

Merged
merged 15 commits into from
Aug 27, 2024
Merged

Add rock-paper-scissors game example #1479

merged 15 commits into from
Aug 27, 2024

Conversation

MahtabNorouzi
Copy link
Collaborator

  • Tests added for any new code
  • Documentation added for any new functionality
  • Entries added to the respective CHANGELOG.md for any new functionality
  • Feature table on README.md updated for any listed functionality

Copy link
Collaborator

@bugarela bugarela left a comment

Choose a reason for hiding this comment

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

Hi! Here are some comments. Sending this before the meeting so we can talk about it there

@@ -0,0 +1,109 @@

Copy link
Collaborator

Choose a reason for hiding this comment

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

We can add this line to the start of a Quint file to instruct GitHub to highlight it as bluespec, which is better than no highlighting at all (and we don't have highlighting for Quint in GitHub yet)

Suggested change
// -*- mode: Bluespec; -*-

Comment on lines 39 to 41
(p1State == Rock and p2State == Scissor) or
(p1State == Paper and p2State == Rock) or
(p1State == Scissor and p2State == Paper)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We could use winCondition1 here

// Randomly decide moves for both players from the set of possible moves
action decideMoves() = {
nondet move1 = Set(Rock, Paper, Scissor).oneOf() // Non-deterministically pick a move for player 1
nondet move2 = Set(Rock, Paper, Scissor).oneOf() // Non-deterministically pick a move for player 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
nondet move2 = Set(Rock, Paper, Scissor).oneOf() // Non-deterministically pick a move for player 1
nondet move2 = Set(Rock, Paper, Scissor).oneOf() // Non-deterministically pick a move for player 2

Comment on lines 13 to 15
var p1State : Move
var p2State : Move
var game : GameStatus
Copy link
Collaborator

Choose a reason for hiding this comment

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

In general, modern languages don't have spaces between name and type, so I prefer not to add those

Suggested change
var p1State : Move
var p2State : Move
var game : GameStatus
var p1State: Move
var p2State: Move
var game: GameStatus

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this fits much better under examples/games.

)

// Invariant to check if the game status matches the expected winner
val winInv = (winCondition1 implies (game == Winner(PLAYER1)) or (winCondition2 implies (game == Winner(PLAYER2))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be an and, as both implications need to hold!

Suggested change
val winInv = (winCondition1 implies (game == Winner(PLAYER1)) or (winCondition2 implies (game == Winner(PLAYER2))))
val winInv = (winCondition1 implies (game == Winner(PLAYER1)) and (winCondition2 implies (game == Winner(PLAYER2))))

Copy link
Collaborator

@bugarela bugarela left a comment

Choose a reason for hiding this comment

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

This looks amazing, I really really like the README! Thanks for putting it together. I just added some small stylistic comments and we should be able to merge after that 😄

// Constants for player names
const PLAYER1 : str
const PLAYER2 : str
// Enum types for the possible moves and game status
Copy link
Collaborator

Choose a reason for hiding this comment

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

These are technically sum types (although there's a pretty blurred line between them, we call it sum types in Quint, so it's good to be consistent in the examples)

Suggested change
// Enum types for the possible moves and game status
// Sum types for the possible moves and game status

const PLAYER1 : str
const PLAYER2 : str
// Enum types for the possible moves and game status
type Move = Init | Rock | Paper | scissors
Copy link
Collaborator

Choose a reason for hiding this comment

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

Scissors should be capitalized like the others

Suggested change
type Move = Init | Rock | Paper | scissors
type Move = Init | Rock | Paper | Scissors

(m1 == Paper and m2 == Rock),
(m1 == scissors and m2 == Paper),
}
// Initialize the game with both players in 'Init' state and game status as 'Started'
Copy link
Collaborator

Choose a reason for hiding this comment

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

If you use triple slashes /// instead of double // on comments that immediately precede a definition, that comment is treated as a doc-comment. This means that, in your editor, if you hover over definition names, it will show this comment as the documentation for that name 😄. Not super important, but cool to know.

Suggested change
// Initialize the game with both players in 'Init' state and game status as 'Started'
/// Initialize the game with both players in 'Init' state and game status as 'Started'

p2State' = Init,
status' = Started
}
// Randomly decide moves for both players from the set of possible moves
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Randomly decide moves for both players from the set of possible moves
/// Randomly decide moves for both players from the set of possible moves

// Randomly decide moves for both players from the set of possible moves
action decide_moves() = {
nondet move1 = Set(Rock, Paper, scissors).oneOf() // Non-deterministically pick a move for player 1
nondet move2 = Set(Rock, Paper, scissors).oneOf() // Non-deterministically pick a move for player 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
nondet move2 = Set(Rock, Paper, scissors).oneOf() // Non-deterministically pick a move for player 1
nondet move2 = Set(Rock, Paper, scissors).oneOf() // Non-deterministically pick a move for player 2

p2State' = Init,
status' = Started
}
// Invariant to check if the game status matches the expected winner
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Invariant to check if the game status matches the expected winner
/// Invariant to check if the game status matches the expected winner


module play_rock_paper_scissors {
import rock_paper_scissors(PLAYER1 = "Mahtab", PLAYER2 = "Gabriela").*
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should always have a linebreak at the end of the file to make GitHub happy. I have a config on my editors to automate this

Suggested change
}
}

@romac romac changed the title Add new example to classic/distributed Add rock-paper-scissors game example Aug 19, 2024
Copy link
Collaborator

@bugarela bugarela left a comment

Choose a reason for hiding this comment

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

Awesome! Great work here, this turned out great ✨

@MahtabNorouzi MahtabNorouzi merged commit e748ff0 into main Aug 27, 2024
14 checks passed
@MahtabNorouzi MahtabNorouzi deleted the new-example branch August 27, 2024 21:01
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