Skip to content

Introduce rock paper scissors game. - #1

Open
4b1dden wants to merge 12 commits into
cedeerwe:mainfrom
4b1dden:rock_paper_scissors_game
Open

Introduce rock paper scissors game.#1
4b1dden wants to merge 12 commits into
cedeerwe:mainfrom
4b1dden:rock_paper_scissors_game

Conversation

@4b1dden

@4b1dden 4b1dden commented Mar 20, 2021

Copy link
Copy Markdown

Very basic for now – the game consists of one round only. No cross-game state or strategies. The computer decides its shape randomly.

@cedeerwe cedeerwe left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Zatial to nechcem mergnut. Okrem ineho to vypina moju hru takze to nie je backwards compatible. Tam by som ocakaval dorobenie bud nejakeho command-line argumentu alebo initial otazku o tom, aku hru chce user hrat.

Okrem toho som pridal dost vela komentarov o tom celom. Z mojho pohladu to nie uplne splna to co by som od toho ocakaval, tie commity nedavaju moc velky zmysel. Skus to prerobit na zaklade vsetkych komentarov. Citaj si ich prosim commit - by -commit a nie dokopy.

Bol by som rad, keby na konci toho celeho existoval test. To je aj moj plan, akurat som to robil nad mastrom.

Comment thread src/main.rs Outdated
}

#[derive(Copy, Clone)]
enum Shapes {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nazval by som to Shape, keďže to takto evokuje že ide o viac veci

Comment thread src/main.rs Outdated
impl<W: std::io::Write> RockPaperScissorsGame<W>{
fn new(rng: impl RngCore, writer: W) -> Self {
let mut rng = rand::thread_rng();
fn new(mut rng: impl RngCore, writer: W) -> Self {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Niečo mi hovorí, že to prvé sa ti ani neskompilovlo, je tak? Po každom commote má byť funkčný kód, skús tieto dva commoty mergnut do jedného.

Comment thread src/main.rs
writer: W
}

// probably dont need to return Result<String, ParseErorr> here, as all input is string

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Toto je možno trocha filozofickejsia otázka, ale očakával by som že nová funkcia sa hneď aj nejak použije. Pridať metódu čo sa na nič nepoužíva je také zvláštne. Napríklad by som to už v rámci tohto commoty zapracoval do toho reand and parse u32

Comment thread src/main.rs Outdated

impl<W: std::io::Write> RockPaperScissorsGame<W>{
fn new(mut rng: impl RngCore, writer: W) -> Self {
pub fn new(mut rng: impl RngCore, writer: W) -> Self {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Toto pub je minimálne zvláštne, nevyzerá že to súvisí s tým zvyškom

Comment thread src/main.rs
Paper,
Scissors
Scissors,
Unrecognized,

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Toto by som skôr modelovať ako Result<Shapes, Error>. Do game logiky to unrecognized nepatrí úplne.

Comment thread src/main.rs
}

fn transform_shape_to_int(&mut self, shape: ShapesInput) -> i32 {
fn transform_shape_to_int(&mut self, shape: ShapesInput) -> Option<i32> {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

no tu uz to priamo vidis, ShapesInput nema mat v sebe unrecognized

Comment thread src/main.rs
let left_int = self.transform_shape_to_int(left);
let right_int = self.transform_shape_to_int(right);
fn evaluate_game_as_left(&mut self, left: ShapesInput, right: ShapesInput) -> RoundState {
let left_int = self.transform_shape_to_int(left).unwrap();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

oh. Lepsie, ale still ti tu padne program, ak ti tam niekto vlozi plne validny input. ShapesInput::Unrecognized je validny input podla typingu a ty na nom panic-nes. Malo by ti to napovedat, ze je nieco zle.

Comment thread src/main.rs
f,
"The round was {:}.",
if self == &RoundState::Lose { "lost" } else if self == &RoundState::Draw { "a draw" } else { "won" } )
if self == &RoundState::Lose { "lost" } else if self == &RoundState::Draw { "a draw" } else { "won" }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

nepouzivas rust-analyzer? respektive rustfmt, nie som si isty ci je to automaticky includenute. Taketo commity nemaju existovat. O to sa ma starat tooling automaticky. Ak si naaahodou nieco zle spravil, amendni to v predchadzajucom commite.

Comment thread src/main.rs Outdated

fn main() -> Result<(), std::io::Error> {
// GuessingGame::new(100, rand::thread_rng(), std::io::stdout()).start()
RockPaperScissorsGame::new(rand::thread_rng(), std::io::stdout()).start()

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

dovod preco to bolo ako input je, ze potom to budes vediet testovat, lebo tomu budes vediet dat nahodny generator aky ty chces (fixny). Takze toto je trocha krok zlym smerom. Treba mat ciel za tymi refaktormi. Prvotny ciel vsetkeho takehoto by malo byt, ze chces vediet napisat testy. Potom sa mozme bavit o lepsom zapise.

Comment thread src/main.rs

fn get_random_playable_shape(&mut self, mut rng: impl RngCore) -> ShapesInput {
[ShapesInput::Rock, ShapesInput::Paper, ShapesInput::Scissors][rng.gen_range(0..2)]
[ShapesInput::Rock, ShapesInput::Paper, ShapesInput::Scissors][rng.gen_range(0..2)] // todo: implement in a more rust way

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

ten comment tam predtym uz bol, zase toto patri este do predchadzajuceho commitu

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.

2 participants