-
Notifications
You must be signed in to change notification settings - Fork 38
Enhance BoardGameModel and BoardGameGeekAPI: Add age, boardgame famil… #216
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
base: master
Are you sure you want to change the base?
Conversation
…ies, boardgame mechanics, awards, description, language dependence, best with players, recommended with players
const publishers = Array.from(boardgame.querySelectorAll('boardgamepublisher')) | ||
.map(n => n.textContent) | ||
.filter(n => n !== null); | ||
const boardGameFamilies = Array.from(boardgame.querySelectorAll('boardgamefamily')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to include some not very relevant categories for some games.
const minPlayers = Number.parseFloat(boardgame.querySelector('minplayers')?.textContent ?? '0'); | ||
const maxPlayers = Number.parseFloat(boardgame.querySelector('maxplayers')?.textContent ?? '0'); | ||
const playtime = (boardgame.querySelector('playingtime')?.textContent ?? 'unknown') + ' minutes'; | ||
const age = Number.parseFloat(boardgame.querySelector('age')?.textContent ?? '0'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
age
is a quite generic term. Is this the recommended age, or the minimum age to play?
|
||
const bestWithPlayers = | ||
response.querySelector('poll-summary[name=suggested_numplayers] result[name=bestwith]')?.attributes.getNamedItem('value')?.value ?? ''; | ||
const recommendedWithPlayers = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think only including one of those two would be better, as we already have min and max players.
playtime: string; | ||
age: number; | ||
publishers: string[]; | ||
boardGameFamilies?: string[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the new properties could be better sorted. E.g. all the player counts together.
…ies, boardgame mechanics, awards, description, language dependence, best with players, recommended with players