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

Random seeds and daily challenges #125

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

rowboat1
Copy link
Contributor

@rowboat1 rowboat1 commented Jul 2, 2024

Adds a seed function to the RNG in getRandomItem method so that players can deterministically play the same deck.

Adds a "Daily" game mode that uses today's date as the RNG seed, facilitating a daily challenge.

Generates a seed for every new game and displays it at the bottom of the screen so players can share their random games.

Adds the seed to the copied text in the share button.

Adds a "Join Game" button on home screen so that players can use the seed to join games.

Closes #82, closes #102, closes #65

@rowboat1
Copy link
Contributor Author

rowboat1 commented Jul 2, 2024

I'm a little new to React, so I'm not sure if the onKeyPress if key === "Enter" thing is the right way to do things

@tom-james-watson
Copy link
Owner

Hi! This is great - there are some really cool ideas here.

This is pretty hard to review though because there are so many new things going on. There are quite a lot of things that I would redesign, so I don't think I'll be able to merge this without me myself also needing to spend quite a lot of time on it, which I honestly don't have right now.

I'll give you some high level thoughts, though, and maybe we can work on it together. I can't promise I'll have the time though, so I don't want to make you waste your time.

  • I would remove the "Join" functionality entirely. Sharing a game should just be sharing a URL, i.e. wikitrivia.tomjwatson.com?seed=13409134. No need to put the 🫘 thing in the share text in that case. If the share link is for a specific game, it also doesn't make sense to show "best streak" - it should just be the score for that specific seed.
  • I would make daily be a one-time thing. You shouldn't be able to do it multiple times.
  • You should see your current score for the current day if you reload the page
  • I would change "Random" to "Practice"
  • You shouldn't be able to play games in the future by inputting future dates as the seed
  • The text at the bottom of the game doesn't look great. I would not put it there and it needs some design work.

@rowboat1
Copy link
Contributor Author

rowboat1 commented Jul 3, 2024

Awesome! I am glad to have your feedback!

I can definitely do the first five points, and if you have some design wisdom for the text saying what game number it is, then I can do that too. People might want to, for instance, race to 20 or something, and so having easy access to the seed number would be good for that case.

As far as the lack of time, would it be helpful if I split practice seeds into a separate PR?

@tom-james-watson
Copy link
Owner

Nah, it's not that much code. It's ok to be one PR.

Makes practice seeds appear in URL when  you press the share button
@rowboat1
Copy link
Contributor Author

rowboat1 commented Jul 10, 2024

Removed text at the bottom and moved the seed functionality to be done through query params.

Haven't done anything about disallowing reattempt of the daily / resuming the daily when you return yet

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.

Multiplayer mode One set of card per day Share my last game (Share button improvements)
2 participants