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

Elins Guess who? #332

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Elins Guess who? #332

wants to merge 7 commits into from

Conversation

Soygirt
Copy link

@Soygirt Soygirt commented Sep 10, 2023

Here's my Guess who game! 😄

Copy link

@BeckieMorton BeckieMorton left a comment

Choose a reason for hiding this comment

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

Nice job, Elin! Your guess who game works perfect for me and in my opinion you have all of the requirements :-) I'm sure you could tackle some of the stretch goals if you had time. Perhaps using more comments would make reading your code smoother (but perhaps I comment too much!!). You had good function and variable names which made sense and well set out if statements.

Your find out button doesn't work when you first load the page (need to change your select dropdown first), perhaps making a disabled option that is first displayed eg. "Choose an option here..." would force the user to chose before hitting find out on brown eyes first go. Well done for this week :-)

<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Project Name</title>

Choose a reason for hiding this comment

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

Would be cool to write something like "Guess Who?" here

// Randomly select a person from the characters array and set as the value of the variable called secret
const setSecret = () => {
secret = charactersInPlay[Math.floor(Math.random() * charactersInPlay.length)]

//console.log(`secret person is ${secret.name}`); //helped me to see who the secret person is.

Choose a reason for hiding this comment

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

I did the same here! and I kept it in my code too, it was vital to have this information when debugging and testing


if (category === 'hair' || category === 'eyes') {
keep = value === secret[category];

Choose a reason for hiding this comment

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

I don't really understand this line, with 2 equals = values, but it works!

// If the player wants to guess, invoke the checkMyGuess function.
const isConfirmed = confirm(`Are you sure you want to guess that ${personToConfirm} is the secret person?`);

if (isConfirmed) {

Choose a reason for hiding this comment

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

i'm wondering if this if statement needs what happens if they do not confirm?

} else {
winOrLose.style.display = "flex";
board.style.display = 'none';
winOrLoseText.innerHTML += `<h1>Your guess is unfortunately wrong 😞 Do you wanna try again?</h1>`

Choose a reason for hiding this comment

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

this is great, works well, is succinct and easy to understand

start();

const replay = () => {
location.reload(); //The location.reload refreshes the page, which was a easy solution to make the game start again.

Choose a reason for hiding this comment

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

nice one! I have not used .reload before, you are right it is an easy solution

restartButton.addEventListener('click', start)
restartButton.addEventListener('click', start); //resets the board when the restart button is clicked.

questions.addEventListener('click', selectQuestion); //Does this work? Error in console on website.

Choose a reason for hiding this comment

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

im also wondering about this, doesn't select question get called when you click the findout button?

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