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.

Sign up for GitHub

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

Jump to bottom

feat(api): introduce ResourcePackRequest #981

Merged
merged 10 commits into from
Nov 20, 2023
Merged

feat(api): introduce ResourcePackRequest #981

merged 10 commits into from
Nov 20, 2023

Conversation

yusshu
Copy link
Contributor

@yusshu yusshu commented Oct 7, 2023

No description provided.

@kashike kashike added this to the 4.15.0 milestone Oct 7, 2023
zml2008
zml2008 requested changes Oct 7, 2023
View reviewed changes
Copy link
Member

@zml2008 zml2008 left a comment

Choose a reason for hiding this comment

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

Thanks for the contributions! A few design things here, but overall pretty good

api/src/main/java/net/kyori/adventure/resourcepack/ResourcePack.java Outdated Show resolved Hide resolved
api/src/main/java/net/kyori/adventure/resourcepack/ResourcePack.java Outdated Show resolved Hide resolved
api/src/main/java/net/kyori/adventure/resourcepack/ResourcePackImpl.java Outdated Show resolved Hide resolved
api/src/main/java/net/kyori/adventure/resourcepack/ResourcePackImpl.java Outdated Show resolved Hide resolved
api/src/main/java/net/kyori/adventure/resourcepack/package-info.java Outdated Show resolved Hide resolved
@Machine-Maker
Copy link
Member

Is ResourcePack a good name? I feel like you aren't really dealing with a resourcepack, like the object that might exist on the client, but rather ResourcePackInfo or something like that.

@yusshu
Copy link
Contributor Author

yusshu commented Oct 8, 2023

Is ResourcePack a good name? I feel like you aren't really dealing with a resourcepack, like the object that might exist on the client, but rather ResourcePackInfo or something like that.

I'm not sure what its name should be, but I agree that it feels like we are not working with an actual resource pack.
I call this class "ResourcePackRequest" in my plugin

@zml2008
Copy link
Member

zml2008 commented Oct 8, 2023

Please remember to resolve the codestyle issues in this PR (first run ./gradlew spotlessApply, then ./gradlew check and fix any issues that remain)

kezz
kezz approved these changes Oct 10, 2023
View reviewed changes
Copy link
Member

@kezz kezz left a comment

Choose a reason for hiding this comment

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

Should have an accompanying PR on adventure-docs, but otherwise LGTM! Thanks for the contribution :)

@yusshu yusshu changed the title feat(api): introduce ResourcePack feat(api): introduce ResourcePackRequest Oct 12, 2023
@zml2008 zml2008 self-assigned this Nov 20, 2023
@zml2008 zml2008 dismissed kashike’s stale review November 20, 2023 17:21

changes have been made

@zml2008 zml2008 added this pull request to the merge queue Nov 20, 2023
Merged via the queue into KyoriPowered:main/4 with commit 323990f Nov 20, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kezz kezz kezz approved these changes

@zml2008 zml2008 zml2008 approved these changes

@kashike kashike kashike left review comments

Assignees

@zml2008 zml2008

Labels
area: api type: enhancement
Projects
None yet
Milestone
4.15.0
Development

Successfully merging this pull request may close these issues.
5 participants
@yusshu @Machine-Maker @zml2008 @kezz @kashike