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

Spectator mode #172

Merged
merged 4 commits into from
Jan 17, 2022
Merged

Spectator mode #172

merged 4 commits into from
Jan 17, 2022

Conversation

ChristophNiehoff
Copy link
Collaborator

This pull request introduces a spectator mode, i.e. a link to a readonly view of the game indented for a moderator.

The idea behind this is described in this documentation: If the playerID is undefined then boardgame.io defines a readonly view. The "player" of this view cannot participate in the game.

For the spectator mode, the following changes where added:

  • When creating a new game, a dedicated set of spectator credentials are generated on the server, that are later used to secure API requests from the spectator
  • A spectator link is created along with the normal player links (The "Copy All" button on the create page does not contain the spectator link)
  • When entering the spectator link a boardgame client with playerID=undefined is generated in the react app
  • The board does not render player specific components (i.e. the card deck)
  • API requests are authenticated using the spectator credentials
  • On the server, a koajs middleware checks for correct player credentials or correct spectator credentials

Resolves #74

Copy link
Contributor

@ghost91- ghost91- left a comment

Choose a reason for hiding this comment

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

Looks good! I just added a minor question.

On a side note, it probably makes sense to start using a proper logger instead of just console at some point. I'll create an issue for that.

return await next();
}
} catch (err) {
console.error(
Copy link
Contributor

Choose a reason for hiding this comment

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

What kind of error cases can be caught here? Is it only actual errors that represent bugs, or can the user also cause errors here by incorrectly using the software?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I believe it is both. E.g. a user could send a request without Authorization header. Then credentials === undefined and calling credentials.name would raise an exception.
But yes, by just using the software (correctly or incorrectly) one should not run into this.

Copy link
Contributor

@ghost91- ghost91- Jan 17, 2022

Choose a reason for hiding this comment

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

If this is the case, I think we should differentiate the cases. Problems resulting from users incorrectly using the software should not be logged on the error level. Only actual errors in the software that require intervention / bugfixing should be logged in that level.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed. Changed it such that user-errors are logged on log-level and unexpected errors are logged on error-level.

@ChristophNiehoff ChristophNiehoff merged commit 033e894 into dehydr8:master Jan 17, 2022
@ghost91- ghost91- deleted the spec_mode branch February 10, 2022 19:36
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.

Spectator Mode
3 participants