Rewrite "rock paper scissors" project #26037

cakegod · 2023-08-03T20:37:37Z

Because

This PR

Improve clarity, wording and punctuation of the "rock paper scissors"

Issue

Closes #XXXXX

Additional Information

Pull Request Requirements

I have thoroughly read and understand The Odin Project Contributing Guide
The title of this PR follows the location of change: brief description of change format, e.g. Intro to HTML and CSS lesson: Fix link text
The Because section summarizes the reason for this PR
The This PR section has a bullet point list describing the changes in this PR
If this PR addresses an open issue, it is linked in the Issue section
If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
If any lesson files are included in this PR, they follow the Layout Style Guide

… project

ManonLef

Thanks for revisiting this lesson. I think it could indeed do with a nice makeover, great work!

I left a few comments. I hope I caught them all but it was quite a lot to review. Let me know your thoughts!

foundations/javascript_basics/project_rock_paper_scissors.md

ManonLef · 2023-08-03T22:43:14Z

Sidenote: You can leave the resolving up to the reviewer usually. That way they can check and resolve their requests for changes once they are committed.

I'll check back tomorrow for it's late here. Thanks for taking this on!

ManonLef · 2023-08-04T20:51:14Z

Please re-request a review once you're ready for it. Thank you!

cakegod · 2023-08-04T20:52:12Z

I've made significant changes to the original, including the requested changes. I'm unsure about using tip note boxes on the assignment; tell me what you think.

ManonLef · 2023-08-04T21:03:02Z

Thanks @cakegod !

Those changes are a lot more than expected. I can't review them today anymore.
I don't think the tips should be in lesson note elements no. They drag too much attention in my opinion with the added color and I believe I liked the previous ones better.

I'll review this lesson once I get some time. Thanks again!

cakegod · 2023-08-04T21:14:37Z

Indeed, this is also my overall feeling.

We could probably have a specific tip note-box-like for the assignment box. Adding tips requires us to add them as bullet points, which is odd since they are not strictly part of the assignment.

ManonLef

That was a lot to review!

I do think this lesson was in need of some clarification judging by the amount of questions surrounding the wording in our discord.

For big changes or full lesson rewrites like this we usually start with an issue first so discussion can happen there to flesh out the exact details regarding areas of improvement.
I expect most of my comments could use some discussing as well, and I welcome your input.

I also would like another maintainer to review this after we have gone through most since it encompasses a full rewrite at this point.

I may have missed things so please go through it carefully but my key points are this:

the different -- lines between sentences are not what we normally use so I removed them
code elements reserved for code and not for boldening or other emphasis
the numbered list should be fully encompassing what the step is. I approached it as: could someone experienced code this without the added steps (not numbered) below?
the steps below are breaking the problem in the numbered list item down.

Let me know what you think!

foundations/javascript_basics/project_rock_paper_scissors.md

cakegod · 2023-08-05T22:21:17Z

That was a lot to review!

I do think this lesson was in need of some clarification judging by the amount of questions surrounding the wording in our discord.

For big changes or full lesson rewrites like this we usually start with an issue first so discussion can happen there to flesh out the exact details regarding areas of improvement. I expect most of my comments could use some discussing as well, and I welcome your input.

I also would like another maintainer to review this after we have gone through most since it encompasses a full rewrite at this point.

I may have missed things so please go through it carefully but my key points are this:

the different -- lines between sentences are not what we normally use so I removed them

code elements reserved for code and not for boldening or other emphasis

the numbered list should be fully encompassing what the step is. I approached it as: could someone experienced code this without the added steps (not numbered) below?

the steps below are breaking the problem in the numbered list item down.

Let me know what you think!

The initial idea was to fix punctuation and make minor improvements. It then evolved into something bigger, sorry!

I like the em dashes; they emphasize certain parts. I used them since I saw nothing in the layout style guide forbidding their use.

In a way, the multiple steps are mixed into each step in the current assignment, so I split them into sub-steps.

ManonLef · 2023-08-05T22:31:52Z

The initial idea was to fix punctuation and make minor improvements. It then evolved into something bigger, sorry!

It happens! It just takes a lot more resources than the initial PR you filed. I do think this project could be enhanced and think you put in a great step to doing so!

I like the em dashes; they emphasize certain parts. I used them since I saw nothing in the layout style guide forbidding their use.

They are not mentioned but also stand out for they are not generally used and I think the sentences don't lose a lot by not inserting them here.

In a way, the multiple steps are mixed into each step in the current assignment, so I split them into sub-steps.

Yes I can see that, and I left the sub-steps. I do think the numbering might need to go for those. And I rewrote the main steps a bit. The substeps are still an elaboration for those who need it. The only thing I wondered while reviewing is if it now doesn't go overboard on handholding, but judging from the amount of issues people have with this project being the first JS one, I think it will be a hard project either way.

cakegod · 2023-08-05T23:04:53Z

Yes I can see that, and I left the sub-steps. I do think the numbering might need to go for those. And I rewrote the main steps a bit. The substeps are still an elaboration for those who need it. The only thing I wondered while reviewing is if it now doesn't go overboard on handholding, but judging from the amount of issues people have with this project being the first JS one, I think it will be a hard project either way.

The current assignment already handholds. We're simply making the assignment clearer by breaking the steps into sub-steps. 😉

I'll commit your changes and re-check everything.

ManonLef · 2023-08-05T23:08:57Z

The current assignment already handholds. We're simply making the assignment clearer by breaking the steps into sub-steps. 😉

I can't say I disagree, it's just more obfuscated by it all being crammed in a sentence I guess 😅

Co-authored-by: Manon <[email protected]>

CBID2 · 2023-08-06T21:52:27Z

foundations/javascript_basics/project_rock_paper_scissors.md

-3.  Your game is going to play against the computer, so begin with a function called `getComputerChoice` that will randomly return either 'Rock', 'Paper' or 'Scissors'.  We'll use this function in the game to make the computer's play. *Tip: use the console to make sure this is returning the expected output before moving to the next step!*
-4.  Write a function that plays a single round of Rock Paper Scissors.  The function should take two parameters - the `playerSelection` and `computerSelection` - and then return a string that declares the winner of the round like so: `"You Lose! Paper beats Rock"`
-    *   Make your function's playerSelection parameter case-insensitive (so users can input `rock`, `ROCK`, `RocK` or any other variation).
+Since this is your first JavaScript program built from scratch, remember the [previous wise words from the problem-solving lesson](https://www.theodinproject.com/lessons/foundations-problem-solving). Before diving into this project, first plan or pseudocode your solution, then write the code, then ensure the code works.


[Proofreading] The last sentence seems to describe some steps, so I provided a suggestion on how to make it clearer.

Suggested change

Since this is your first JavaScript program built from scratch, remember the [previous wise words from the problem-solving lesson](https://www.theodinproject.com/lessons/foundations-problem-solving). Before diving into this project, first plan or pseudocode your solution, then write the code, then ensure the code works.

Since this is your first JavaScript program built from scratch, remember the [previous wise words from the problem-solving lesson](https://www.theodinproject.com/lessons/foundations-problem-solving). Before diving into this project, First, plan or pseudocode your solution. Next, write the code. Lastly, ensure the code works.

Thank you, we are still in the exploratory phase it seems so no final version to review yet.

CBID2 · 2023-08-06T21:56:14Z

foundations/javascript_basics/project_rock_paper_scissors.md

+   console.log(playRound(playerSelection, computerSelection));
+   ~~~
+
+1. Your game plays for 5 rounds. Write a function called `game` which will keep score for 5 rounds, utilizing your 'playRound' function, before it declares a winner:


[Proofreading] Parts of this step were a bit unclear, so I gave some ideas on how to revise it. Check out my suggestions below:

Suggested change

1. Your game plays for 5 rounds. Write a function called `game` which will keep score for 5 rounds, utilizing your 'playRound' function, before it declares a winner:

1. Write a function called `game` which will keep score for 5 rounds and utilize your 'playRound' function before it declares a winner:

I hope you saw my last comment. We're still finding a direction for this so no need to review just yet. Thank you however :)

CBID2

Hi @cakegod! :) I found some parts of your pull request that could be improved, so check out my in-line comments for context. Other than that, your entry is off to a great start! :) Great job! :)

ManonLef · 2023-08-06T21:59:44Z

@cakegod I have converted this lesson to draft for now so people can see it's still a work in progress.

ManonLef · 2023-10-03T09:53:15Z

@cakegod what's the status on this? If you're ready for a review again after making changes, there's a button to request it.

cakegod · 2023-10-03T10:17:03Z

@cakegod what's the status on this? If you're ready for a review again after making changes, there's a button to request it.

Still a work in progress. I have started creating a checklist of pain points, but I have yet to write fixes for them. I'll see if I can finish it this week.

cakegod · 2023-11-11T13:27:54Z

It's time to finish this before I forget again. Tell me what you think about the latest changes. I'm not sure if we should add more tips or leave it as it is.

ManonLef · 2023-11-26T21:02:04Z

I wanted to let you know it will be a while before I get to sit down for this. My focus is on an internship currently and I expect to get back to maintaining at the start of the new year. Thanks for the work you've put in!

NikitaRevenco

I really like this rewrite, I've read and I have some small ideas that could improve the readability

foundations/javascript_basics/project_rock_paper_scissors.md

Co-authored-by: Nikita Revenco <[email protected]>

MaoShizhong

Really like the overall direction this is going in @cakegod!
A couple of suggestions and considerations below, as well as some small fixes.

foundations/javascript_basics/project_rock_paper_scissors.md

Co-authored-by: MaoShizhong <[email protected]>

thatblindgeye

Some additional comments below. Some are nits/things I wouldn't block over so lemme know if you have thoughts on them.

foundations/javascript_basics/project_rock_paper_scissors.md

thatblindgeye · 2024-03-23T14:30:29Z

foundations/javascript_basics/project_rock_paper_scissors.md

+1. Plan or pseudocode your solution.
+1. Write the code.
+1. Test your code to make sure it works.


If this is meant to echo the problem solving lesson, we probably want these items to be about understanding the problem, planning, writing pseudo code, and then dividing and conquering, no?

Maybe it's the wording leading into it, though. "Before you start this project" could maybe be removed, then just say, "For each step in this project, be sure to do the following:"

foundations/javascript_basics/project_rock_paper_scissors.md

thatblindgeye · 2024-03-23T14:53:24Z

foundations/javascript_basics/project_rock_paper_scissors.md

-   }
+1. Create a new function named `playGame`.
+1. Get the human player choice using the `prompt` method. Read this [MDN article about the prompt method](https://developer.mozilla.org/en-US/docs/Web/API/Window/prompt) learn more about it.
+1. Put the previous `playRound` function inside `playGame`.


Is the intent with this line here that users should literally move their playRound declaration inside of playGame, or just that they should call playRound inside of playGame? The wording makes it sound like the former which we probably don't want users to do. We could honestly probably just remove this step since we're already telling users to call playRound 5x on the next line.

This depends on where we want the user to declare the scores. If the scores are declared inside the playGame, we won't be able to increment the scores inside the playRound. We can use the return of playRound to conditionally increment the winner's score variable declared in playGame, but this makes the project much more complicated.

So, I thought about guiding the Oditinite to one of those two outcomes: https://gist.github.com/cakegod/2f5f63295ff606a1a729f67564cd7d43

Or we could keep the outcome return in playRound, but it becomes more messy and complicated without loops: https://gist.github.com/cakegod/269810a537de15cc427a0b35f4b8c39f

Another option would be declaring the scores as global variables, though yeah as the project is currently written and remains written here the intent seems to be that playGame houses the scores.

If you feel having the playRound moved inside of playGame is the better option I'm good with that. Maybe just tweak the verbiage of this instruction to, "Move your playRound function so that it's declared inside of the new playGame function"

foundations/javascript_basics/project_rock_paper_scissors.md

Co-authored-by: Eric Olkowski <[email protected]>

thatblindgeye

One more tweak below. Would you mind also looking at the two unresolved comments from me above?

foundations/javascript_basics/project_rock_paper_scissors.md

Co-authored-by: Eric Olkowski <[email protected]>

MaoShizhong · 2024-04-20T12:41:14Z

@cakegod Mind fixing up the remaining HTML blank lines lint errors? You can ignore the "unchanged files with check annotations" thing, just annoying noise from Github's beta feature.

Then once the blank line lint errors are gone, if @thatblindgeye doesn't have any further change requests, we could get this merged?

thatblindgeye

Other than the above, looks good. Awesome job on this!

cakegod · 2024-04-21T14:05:16Z

I am not sure if we want to include validation on the human player input, as it would require a loop or recursion. 🤔

MaoShizhong · 2024-04-21T14:39:40Z

I am not sure if we want to include validation on the human player input, as it would require a loop or recursion. 🤔

I'd say no need for validation for that exact reason.

Old review - maintainer busy, so has passed this PR to other maintainers to review and approve

Improve clarity, wording and punctuation of the "rock paper scissors"…

17c73c6

… project

github-actions bot added the Content: Foundations Involves the Foundations content label Aug 3, 2023

ManonLef requested changes Aug 3, 2023

View reviewed changes

cakegod added 2 commits August 4, 2023 22:46

Rewrite - first draft

c451481

Merge branch 'main' into project_rock_paper_scissors

cd96056

cakegod requested a review from ManonLef August 4, 2023 20:52

cakegod added 2 commits August 5, 2023 21:46

Rewrite - second draft

a96cbf2

Merge branch 'main' into project_rock_paper_scissors

9e3ec9e

ManonLef previously requested changes Aug 5, 2023

View reviewed changes

ManonLef self-assigned this Aug 5, 2023

Changes from Manon draft v2 review

a173962

Co-authored-by: Manon <[email protected]>

cakegod changed the title ~~Improve clarity, wording and punctuation of the "rock paper scissors" project~~ Rewrite "rock paper scissors" project Aug 5, 2023

CBID2 reviewed Aug 6, 2023

View reviewed changes

ManonLef marked this pull request as draft August 6, 2023 21:59

cakegod added 2 commits August 9, 2023 23:04

Small refinements

cb52cb7

Structure assignment section

b4f3b3f

Improve wording of most phrases

74d4fcf

cakegod marked this pull request as ready for review November 11, 2023 13:23

cakegod requested a review from ManonLef November 11, 2023 13:24

This comment was marked as spam.

Sign in to view

NikitaRevenco reviewed Mar 4, 2024

View reviewed changes

cakegod and others added 5 commits March 4, 2024 19:04

Apply suggestions from ravenco code review

d565ca0

Co-authored-by: Nikita Revenco <[email protected]>

Merge branch 'main' into project_rock_paper_scissors

7b20b6c

fix markdown lint issues

b2b704e

add takina suggestion

4f08dbc

changes some phrases

df489e6

MaoShizhong reviewed Mar 22, 2024

View reviewed changes

cakegod and others added 2 commits March 22, 2024 21:07

Apply suggestions from code review

39b037b

Co-authored-by: MaoShizhong <[email protected]>

apply remaining suggestion

c0ab513

Co-authored-by: MaoShizhong <[email protected]>

thatblindgeye reviewed Mar 23, 2024

View reviewed changes

cakegod and others added 2 commits March 25, 2024 18:35

Apply suggestions from code review

c4ab58d

Co-authored-by: Eric Olkowski <[email protected]>

Apply remaining suggestions

5574e48

Co-authored-by: Eric Olkowski <[email protected]>

thatblindgeye requested changes Mar 26, 2024

View reviewed changes

foundations/javascript_basics/project_rock_paper_scissors.md Outdated Show resolved Hide resolved

add additional steps

e69a11d

Co-authored-by: Eric Olkowski <[email protected]>

thatblindgeye approved these changes Apr 20, 2024

View reviewed changes

cakegod added 2 commits April 21, 2024 15:54

fix md linting issues

28dfc91

fix mistakes

3345fa1

MaoShizhong approved these changes Apr 21, 2024

View reviewed changes

MaoShizhong merged commit 594a36c into TheOdinProject:main Apr 21, 2024
2 of 3 checks passed

MaoShizhong mentioned this pull request May 6, 2024

Rock Paper Scissors: <Link Variables and Operators: How to run Javascript code.> #27457

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rewrite "rock paper scissors" project #26037

Rewrite "rock paper scissors" project #26037

cakegod commented Aug 3, 2023

ManonLef left a comment •

edited

Loading

ManonLef commented Aug 3, 2023

ManonLef commented Aug 4, 2023

cakegod commented Aug 4, 2023

ManonLef commented Aug 4, 2023

cakegod commented Aug 4, 2023

ManonLef left a comment

cakegod commented Aug 5, 2023

ManonLef commented Aug 5, 2023 •

edited

Loading

cakegod commented Aug 5, 2023 •

edited

Loading

ManonLef commented Aug 5, 2023

CBID2 Aug 6, 2023

ManonLef Aug 6, 2023

CBID2 Aug 6, 2023

ManonLef Aug 6, 2023

CBID2 left a comment

ManonLef commented Aug 6, 2023

ManonLef commented Oct 3, 2023

cakegod commented Oct 3, 2023

cakegod commented Nov 11, 2023

ManonLef commented Nov 26, 2023

This comment was marked as spam.

NikitaRevenco left a comment

MaoShizhong left a comment

thatblindgeye left a comment

thatblindgeye Mar 23, 2024

thatblindgeye Mar 23, 2024

cakegod Mar 27, 2024 •

edited

Loading

thatblindgeye Mar 30, 2024

thatblindgeye left a comment

MaoShizhong commented Apr 20, 2024

thatblindgeye left a comment

cakegod commented Apr 21, 2024

MaoShizhong commented Apr 21, 2024

	Since this is your first JavaScript program built from scratch, remember the [previous wise words from the problem-solving lesson](https://www.theodinproject.com/lessons/foundations-problem-solving). Before diving into this project, first plan or pseudocode your solution, then write the code, then ensure the code works.
	Since this is your first JavaScript program built from scratch, remember the [previous wise words from the problem-solving lesson](https://www.theodinproject.com/lessons/foundations-problem-solving). Before diving into this project, First, plan or pseudocode your solution. Next, write the code. Lastly, ensure the code works.

	1. Your game plays for 5 rounds. Write a function called `game` which will keep score for 5 rounds, utilizing your 'playRound' function, before it declares a winner:
	1. Write a function called `game` which will keep score for 5 rounds and utilize your 'playRound' function before it declares a winner:

Rewrite "rock paper scissors" project #26037

Rewrite "rock paper scissors" project #26037

Conversation

cakegod commented Aug 3, 2023

Because

This PR

Issue

Additional Information

Pull Request Requirements

ManonLef left a comment • edited Loading

Choose a reason for hiding this comment

ManonLef commented Aug 3, 2023

ManonLef commented Aug 4, 2023

cakegod commented Aug 4, 2023

ManonLef commented Aug 4, 2023

cakegod commented Aug 4, 2023

ManonLef left a comment

Choose a reason for hiding this comment

cakegod commented Aug 5, 2023

ManonLef commented Aug 5, 2023 • edited Loading

cakegod commented Aug 5, 2023 • edited Loading

ManonLef commented Aug 5, 2023

CBID2 Aug 6, 2023

Choose a reason for hiding this comment

ManonLef Aug 6, 2023

Choose a reason for hiding this comment

CBID2 Aug 6, 2023

Choose a reason for hiding this comment

ManonLef Aug 6, 2023

Choose a reason for hiding this comment

CBID2 left a comment

Choose a reason for hiding this comment

ManonLef commented Aug 6, 2023

ManonLef commented Oct 3, 2023

cakegod commented Oct 3, 2023

cakegod commented Nov 11, 2023

ManonLef commented Nov 26, 2023

This comment was marked as spam.

NikitaRevenco left a comment

Choose a reason for hiding this comment

MaoShizhong left a comment

Choose a reason for hiding this comment

thatblindgeye left a comment

Choose a reason for hiding this comment

thatblindgeye Mar 23, 2024

Choose a reason for hiding this comment

thatblindgeye Mar 23, 2024

Choose a reason for hiding this comment

cakegod Mar 27, 2024 • edited Loading

Choose a reason for hiding this comment

thatblindgeye Mar 30, 2024

Choose a reason for hiding this comment

thatblindgeye left a comment

Choose a reason for hiding this comment

MaoShizhong commented Apr 20, 2024

thatblindgeye left a comment

Choose a reason for hiding this comment

cakegod commented Apr 21, 2024

MaoShizhong commented Apr 21, 2024

ManonLef left a comment •

edited

Loading

ManonLef commented Aug 5, 2023 •

edited

Loading

cakegod commented Aug 5, 2023 •

edited

Loading

cakegod Mar 27, 2024 •

edited

Loading