-
-
Notifications
You must be signed in to change notification settings - Fork 22
Move from enums to tables #44
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
Move from enums to tables #44
Conversation
@kamranayub I am still testing these changes but would appreciate your feedback when you get a chance. |
Sweet! Thanks a ton for getting this started. I'll try to take a look this week. |
Hey, no problem! Thanks for making the SDK in the first place! |
@kamranayub this PR is ready for review. |
I should have some time to review this weekend 👍 |
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.
Pull Request Overview
This PR migrates several enum-based fields to table-based models in order to address deprecation and align with the new IGDB API changes. Key updates include the removal of deprecated enums from Platform, Game, Company, and other models; the introduction of new table model classes for game types, statuses, age ratings, and more; and endpoint updates in IGDBApi to support the new data structures.
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
IGDB/Models/Platform.cs | Removed PlatformCategory enum and added PlatformType property. |
IGDB/Models/GameType.cs | Added new GameType table model. |
IGDB/Models/GameStatus.cs | Added new GameStatus table model. |
IGDB/Models/GameReleaseFormat.cs | Added new GameReleaseFormat table model. |
IGDB/Models/Game.cs | Removed deprecated enums and added IdentityOrValue fields for GameStatus and GameType. |
IGDB/Models/ExternalGame.cs | Removed ExternalCategory enum and updated properties to use new table models. |
IGDB/Models/DateFormat.cs | Added new DateFormat table model. |
IGDB/Models/CompanyWebsite.cs | Removed CompanyWebsiteCategory enum and added a Type property. |
IGDB/Models/Company.cs | Removed enum fields and added new IdentityOrValue fields for date format and company status. |
IGDB/Models/Character.cs | Removed Gender and Species enums and replaced them with CharacterGender and CharacterSpecies properties. |
IGDB/Models/AgeRating*.cs | Migrated from enum based definitions to new table model implementations. |
IGDB/IGDBApi.cs | Updated endpoint constants to reflect the new table-based models. |
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.
The model changes all look good, but I would like some new test coverage for a selection of the new fields. I can add those tonight I think or welcome to add some.
Also the data dumps need to be tested as well with the new fields. I could take a look at that.
Edit: I believe the test failure is expected in GH actions because secrets aren't available in fork PRs but I could enable that
Looks like the enum fields are just Reference IDs ( I'll update the model definitions to be accurate. edit: Hmm, actually, I don't understand because they should follow same convention as other references and be expanded but the test was failing 🤔 From the docs:
So it should work but I am not seeing the expansion, or even the field, being returned in a query. Still looking into it 😅 update 2: OK I am dumb, a |
@codename-irvin I cannot push to your PR branch (you might have needed to enable "Allow maintainers to push to PR/branch" when creating the PR). I opened a new #45 so I can push my changes, but this looks great, thank you a ton!! |
This PR addresses #42. It removes deprecated fields, adds new types, and adds new endpoints to adhere to the changes outlined here.