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

Reworked pets to deprecate enums & use pet endpoint #557

Merged
merged 3 commits into from
Jun 26, 2023

Conversation

roro1506HD
Copy link
Contributor

@roro1506HD roro1506HD commented Aug 9, 2022

Reworked pets API to use the endpoint added by @ConnorLinfoot (#495 (comment)) and to keep a backwards compatibility for people to be able to change before it gets removed.

The only breaking change is the return type of PetStat#getAllPets which now returns a Map<IPetType, Pet> instead of Map<PetType, Pet>.

The right way of getting pets of a player is to get the pet repository first (HypixelAPI#getPetRepository()) then use that repository in the Player's Player#getPetStats(IPetRepository) method. Providing a null repository or not providing one at all (for backwards compatibility, see BackwardsCompatibilityPetRepositoryImpl), will use the old PetType and Rarity enums instead. Unknown pets and null values are expected through the pets api when using the deprecated enums.

For the repositories, even though there are several specific getters (by name, by rarity.. etc), pets are only stored in a single collection. I have made the choice of sacrificing some computing time when using those getters to save some RAM as these methods are not expected to be used widely.

For a complete example, see GetPetsExample in the examples modules.

The javadoc hasn't been done, I expect to have feedbacks on the rework before losing my head doing the javadocs

Copy link
Member

@ConnorLinfoot ConnorLinfoot left a comment

Choose a reason for hiding this comment

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

Looks good to me! Sorry for the delay in this being reviewed it's not often we get the chance to look at changes on this repo.

If you do have the time to write any java docs that'd be great, I'll likely be looking at merging this in the next few days or so.

@roro1506HD
Copy link
Contributor Author

Latest commit introduces the javadoc for almost every api component (the implementations doesn't have javadocs as they would be the same as the interfaces).

It also renames the new PetStats#getAllPets to PetStats#listAllPets and brings back the old PetStats#getAllPets to fully maintain backwards compatibility.

Lastly it introduces a new method (IPetRepository#hasPlayerUnlocked) which can be used to check if a player has unlocked a certain pet type. This method is implemented in the AbstractPetRepositoryImpl class.

Looking forward to seeing your feedback for this commit!

@ConnorLinfoot ConnorLinfoot merged commit f0d46a6 into HypixelDev:master Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants