Hanabi-Live / hanabi-live

A web server that allows people to play Hanab, a cooperative card game of logic and reasoning.
https://hanab.live
GNU General Public License v3.0
178 stars 116 forks source link

Join pre-game with stored join password #2963

Closed progiv closed 4 weeks ago

progiv commented 4 weeks ago

When pre-game url is opened - send join request with stored join password.

I play hanabi mostly with my friends on password protected tables. Every time I open the pre-game link, It tells me: "That is not the correct password for the game". I didn't provide any password, yet. But the game gives me an error message.

This PR fixes my use case, and doesn't break games without passords Tested this way (U1, U2 - users):

  1. U1: Create game without password.
  2. U2: Join the pre-game link
  3. U1, U2 leave table
  4. U1: Create password protected game
  5. U2: Join the pre-game link. Notice the wrong password message
  6. U2: Enters password, successfully joins the table
  7. U1, U2 leave game
  8. U1: Create password proteced game. (Same password)
  9. U2: Join the pre-game link. No errors
  10. U1, U2 leave game
  11. U1 crate game without password
  12. U2 Join the pre-game link. No errors (the stored password is still sent because of this PR)
Zamiell commented 4 weeks ago

what happens when you try to connect to a game that doesn't have a password, but you send a password for the game anyway

progiv commented 4 weeks ago

what happens when you try to connect to a game that doesn't have a password, but you send a password for the game anyway

It just works. The client joins the table (steps 11,12 in the first message) The password is sent, but it is never checked. See server code

if t.PasswordHash != "" && !d.BypassPassword {
    ...

If the game has no password, first part of the if becomes false, and this branch is not executed

CI format failed

This prettier is a nasty thing. I managed to do the PR without installing npm on my computer by fixing(almost) docker-compose file locally, but now I have to install the whole dev environment because the prettier doesn't show what's wrong(((

Zamiell commented 4 weeks ago

This prettier is a nasty thing.

yeah sorry its nasty for tiny one-off pull requests, but in the general case auto-formatters like prettier are so good that they will change your life forever

Zamiell commented 4 weeks ago

It just works. The client joins the table (steps 11,12 in the first message) The password is sent, but it is never checked. See server code

then you should put that as a comment in the PR so it will be clear for people reading the code in the future

progiv commented 4 weeks ago

in the general case auto-formatters like prettier are so good that they will change your life forever

I do love auto-formatters in general. But since it already knows what's wrong in my code, I expect them to show the diff. It seems like an industry standard, this particular prettier doesn't follow. https://github.com/prettier/prettier/issues/6885

Anyway, thank you for keeping this repo clean and readable!

Zamiell commented 4 weeks ago

thanks for the pr