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

Fix contributor head being pullable #4072

Merged
merged 2 commits into from
Dec 27, 2023
Merged

Fix contributor head being pullable #4072

merged 2 commits into from
Dec 27, 2023

Conversation

WalshyDev
Copy link
Member

@WalshyDev WalshyDev commented Dec 25, 2023

Description

In GitHubTask we're doing Optional<String> skin = Optional.of(future.get().toString()); where future.get() returns PlayerSkin. However, PlayerSkin does not have a toString() function and therefore this was resulting in the normal Object#toString meaning skin was like io.github.thebusybiscuit.slimefun4.libraries.dough.skins.PlayerSkin@abcdef123 instead of the base64 texture.

Proposed changes

This PR updates dough and a small change in our contributor code to fix #4071

Unfortunately, this change cannot be easily unit-tested as the whole GitHubTask is private and changing that wouldn't make sense. We could run the Runnable but we shouldn't, it'll do a bunch of network requests and generally just be bad practice to test a single function.

Dough PR for this change: Slimefun/dough#240

Related Issues (if applicable)

Fixes #4071

Checklist

  • I have fully tested the proposed changes and promise that they will not break everything into chaos.
  • I have also tested the proposed changes in combination with various popular addons and can confirm my changes do not break them.
  • I have made sure that the proposed changes do not break compatibility across the supported Minecraft versions (1.16.* - 1.20.*).
  • I followed the existing code standards and didn't mess up the formatting.
  • I did my best to add documentation to any public classes or methods I added.
  • I have added Nonnull and Nullable annotations to my methods to indicate their behaviour for null values
  • I added sufficient Unit Tests to cover my code.

Merge checklist

  • Dough PR merged
  • Dough version updated

Copy link
Contributor

Your Pull Request was automatically labelled as: "✨ Fix"
Thank you for contributing to this project! ❤️

Sfiguz7
Sfiguz7 previously approved these changes Dec 25, 2023
@Sfiguz7 Sfiguz7 self-assigned this Dec 25, 2023
@WalshyDev WalshyDev changed the title [WIP] Fix contributor head being pullable Fix contributor head being pullable Dec 25, 2023
@WalshyDev
Copy link
Member Author

WalshyDev commented Dec 25, 2023

Server owners hitting this should bust the cache after updating Slimefun. File is located at: plugins/Slimefun/cache/github/skins.yml

Upon testing we saw only a few heads loading per start, not sure why or what's happening there. Once the cache is filled up though, it'll be fine.
Not gonna investigate that for now

Copy link

@WalshyDev WalshyDev marked this pull request as ready for review December 25, 2023 09:40
@WalshyDev WalshyDev requested review from a team as code owners December 25, 2023 09:40
Copy link
Contributor

Slimefun preview build

A Slimefun preview build is available for testing!
Commit: be74ed09

https://preview-builds.walshy.dev/download/Slimefun/4072/be74ed09

Note: This is not a supported build and is only here for the purposes of testing.
Do not run this on a live server and do not report bugs anywhere but this PR!

@J3fftw1
Copy link
Contributor

J3fftw1 commented Dec 25, 2023

Why do we need authlib as a dependency

@WalshyDev
Copy link
Member Author

Why do we need authlib as a dependency

Java compilation fails otherwise. Since we now do getProfile and get the CustomGameProfile which inherits from GameProfile.
it should have been picking it up from dough, I'm still not 100% sure why it didn't but alas. It's a provided dependency so it doesn't add anything extra.

pom.xml Show resolved Hide resolved
@WalshyDev WalshyDev merged commit 6b03850 into master Dec 27, 2023
24 checks passed
@WalshyDev WalshyDev deleted the fix/contributor-head branch December 27, 2023 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ Fix This Pull Request fixes an issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

You can take head out of slimefun guide
3 participants