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

Introduce GameProfileCache, and extract existing cache method to it #1068

Merged
merged 1 commit into from
Mar 5, 2016
Merged

Introduce GameProfileCache, and extract existing cache method to it #1068

merged 1 commit into from
Mar 5, 2016

Conversation

kashike
Copy link
Contributor

@kashike kashike commented Feb 10, 2016

API | Common

This introduces a replaceable GameProfileCache that is used (internally) for resolving profiles and profile related data.

@ryantheleach
Copy link
Contributor

Whats a usecase of being able to replace the cache?

@kashike
Copy link
Contributor Author

kashike commented Feb 10, 2016

@ryantheleach Looking up from your own profile cache instead of hitting Mojang's API, which is rate limited.

@JBYoshi
Copy link
Member

JBYoshi commented Feb 10, 2016

@kashike I believe the built-in implementation uses a local cache to avoid making multiple requests for the same profile in a short amount of time, so that shouldn't be needed.

@kashike
Copy link
Contributor Author

kashike commented Feb 11, 2016

I'm well aware of the internal cache. Keep in mind networks that have many servers can utilise a central cache (database, etc), instead of requesting from Mojang.

* the cache did not contain a profile with the provided id
*/
default Optional<GameProfile> get(UUID uniqueId) {
checkNotNull(uniqueId, "unique id");
Copy link
Contributor

Choose a reason for hiding this comment

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

uniqueId, name this the same as the parameter

@me4502
Copy link
Contributor

me4502 commented Feb 13, 2016

This is just my opinion and should probably not be done until others agree, but it may make more sense to have this as an overridable service. That way by default it can use the vanilla behaviour, but another plugin can provide an implementation that uses their own database etc..

Seeing as we already have a system like that in place, it only makes sense to use it.

@gabizou Thoughts?

@kashike
Copy link
Contributor Author

kashike commented Feb 13, 2016

@me4502 The reason this isn't a service is because it would be very different from others - this provides (assuming vanilla-based in this context) Mojang's implementation, by default, with getDefaultCache.

A possible "solution" would be to keep getDefaultCache where it is now and turn the rest into a service

@gabizou gabizou merged commit 8a1f85c into SpongePowered:bleeding Mar 5, 2016
gabizou added a commit that referenced this pull request Mar 5, 2016
@kashike kashike deleted the feature/titanic branch February 25, 2017 19:26
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.

7 participants