-
-
Notifications
You must be signed in to change notification settings - Fork 322
feat: Added player count metadata from SS with display and filter #2872
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
…nd added a filter under search for player counts.
Summary of ChangesHello @DevOldSchool, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the application's game metadata capabilities by incorporating player count information. It addresses the need to store and display player count data from the SS endpoint, making it visible on game detail pages and enabling users to filter games based on this new criterion. The changes span both backend database schema and API modifications, as well as frontend UI updates to support the new feature. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request successfully adds player count metadata from ScreenScraper, including display on the game detail screen and a new search filter. The changes are comprehensive, touching the database, backend API, and frontend components.
My review has identified a few areas for improvement:
- In the backend, there's a logical inconsistency with the
player_countsfilter implementation which includes an unusedmatch_allparameter. - The database model for
RomMetadatahas a minor type mismatch for theplayer_countdefault value. - In the frontend, the new player count filter has a bug where clearing the filter incorrectly applies a default filter. There's also a minor issue in how filter options are populated.
I've provided specific comments and suggestions for each of these points. Overall, this is a great addition to the application's filtering capabilities.
frontend/src/components/Gallery/AppBar/common/FilterDrawer/Base.vue
Outdated
Show resolved
Hide resolved
frontend/src/components/Gallery/AppBar/common/FilterDrawer/Base.vue
Outdated
Show resolved
Hide resolved
gantoine
left a comment
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.
good start! can you also integrate IGDB's multiplayer API (https://api-docs.igdb.com/#multiplayer-mode)? es-de also supports a <players> tag with an integer and should be included in gamelist_handler.py
Looks like the gamelist_handler already correctly handles the tag from gamelist.xml. IGDB data doesn't seem very good. [
{
"id": 134709,
"name": "Grand Theft Auto V"
},
{
"id": 1020,
"multiplayer_modes": [
{
"id": 1045,
"offlinecoop": false,
"offlinemax": 0,
"onlinecoop": true,
"onlinemax": 0,
"platform": {
"id": 9,
"name": "PlayStation 3"
}
},
{
"id": 12794,
"offlinecoop": false,
"offlinemax": 0,
"onlinecoop": true,
"onlinemax": 30,
"platform": {
"id": 49,
"name": "Xbox One"
}
},
{
"id": 12795,
"offlinecoop": false,
"onlinecoop": true,
"platform": {
"id": 6,
"name": "PC (Microsoft Windows)"
}
}
],
"name": "Grand Theft Auto V"
},
{
"id": 239064,
"multiplayer_modes": [
{
"id": 24400,
"offlinecoop": false,
"onlinecoop": true,
"onlinemax": 30,
"platform": {
"id": 49,
"name": "Xbox One"
}
},
{
"id": 24401,
"offlinecoop": false,
"onlinecoop": true,
"onlinemax": 30,
"platform": {
"id": 169,
"name": "Xbox Series X|S"
}
}
],
"name": "Grand Theft Auto V"
}
]It also introduces online/offline multiplayer concepts which I had not considered as my library is retro consoles. How should we handle this? |
Map On that topic, seems mobygames supports "Number of Players Supported" via the API, but we've more or less deprecated it so don't worry about supporting it at the moment. |
Ended up doing this except for the > 4 logic. I just let it use the max value. |
|
@gantoine, can you review again? |
Added player count metadata from SS. Displays on game detail screen and added a filter under search for player counts.
Description
This change brings in the player count (joueurs) field from the SS endpoint. This data was already available but was not being saved in the ss_metadata field.
The player count field is displayed on the game detail page as "Players" with a clickable filter for the value.
An additional search filter was added for player count to filter games by the selected values.
Note; the player count is defaulted to 1 when no data exists. (An existing library will need a scan to update this value)
Checklist
Please check all that apply.
Screenshots (if applicable)
Detail screen:

Search filter:

Questions for reviewers