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

Replace FillInExtraData system with a new DataContext object in IDataConvertableFrom #495

Merged
merged 5 commits into from
May 15, 2024

Conversation

jvyden
Copy link
Member

@jvyden jvyden commented May 14, 2024

This PR modifies a significant chunk of the codebase for endpoints, so for an easier review I recommend just looking at only the files regarding the DataContext itself and the service for it.

Key things to consider:

  • A change to GameDatabaseContext.GetAssetFromHash to return null for assets that will never be stored in the database. Simplifies some code and makes it faster.
  • Since the DataContext service depends on most services, it must be added as the last service, always.
  • DataContext.Game and DataContext.Platform both assume Website if a token is not present. This shouldn't ever be hit in Game API code but it's something to be aware of.
  • To simplify code when making a new DataContext, DataContextService.StealInjection does some illegal things by passing null for some parameters. This should be okay for the services that are already there, but I recommend looking at the services AddParameterToEndpoint function when doing this just to make sure.
  • Most of this refactor was done automatically with Rider's refactoring tools. Unused parameters in endpoints are not considered, and ordering of parameters in endpoints are also not considered. I'm not willing to manually give each little endpoint a correction of this, because it doesn't matter in the long run.

@jvyden jvyden requested a review from Beyley May 14, 2024 20:41
Copy link
Member

@Beyley Beyley left a comment

Choose a reason for hiding this comment

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

absolutely loving this refactor even more now that i see it in code, finally one of the biggest warts of the codebase is gone

@jvyden jvyden merged commit 0d13e8f into LittleBigRefresh:main May 15, 2024
2 checks passed
@jvyden jvyden deleted the data-context branch May 15, 2024 02:35
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.

2 participants