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 XSkull#applySkin with name or UUID inputs (#254) #256

Merged
merged 4 commits into from
May 30, 2024

Conversation

Condordito
Copy link
Contributor

@Condordito Condordito commented May 22, 2024

Additions

  • Add support for offline-mode servers, as textures might not cache correctly.
  • Implement asynchronous texture fetching option.

Changes

  • Remove MOJANG_SHA_FAKE_ID_ENUMERATOR to prevent item stacking issues after restarts (UUIDs are now generated using texture hashes).
  • Store UUIDs in usercache.json and cache textures on the first request.
  • Ensure cached textures remain consistent across plugin interactions.

Fixes

  • UUID requests returning HTTP response code: 429.
  • CraftMetaSkull complaining about missing serializedProfile field.

Screenshot 2024-05-22 at 11 07 10 AM

- Add support for offline-mode servers
- Implement asynchronous texture fetching option
- Remove MOJANG_SHA_FAKE_ID_ENUMERATOR to prevent item stacking issues after restarts (UUIDs are now generated using texture hashes)
- Store UUIDs in usercache.json and cache textures on first request
- Ensure cached textures remain consistent across plugin interactions
@CryptoMorin
Copy link
Owner

CryptoMorin commented May 23, 2024

I think we need to make an inner class that makes dynamically configurable requests like ParticleDisplay.
For example, we could make the logging optional and propagate the error instead, or add two sync and async methods to allow the user to decide whether they want to keep the request in the same thread or not (in case the item meta is already being built in an async thread from the user)

Or we could just use the XSkull class itself instead of creating a new inner class.

public static Builder of(ItemStack item) { return new Builder(item); }
public static final class Builder {
    private ItemStack item;
    private SkullMeta meta;
    private SkullValue value;
    private GameProfile defaultValue = DEFAULT_PROFILE; // Or maybe a default SkullValue instead.
    private boolean propagateError = false;

    public Builder propogateError() {
        this.propogateError = true;
        return this;
    }

    public Builder fromString(String string) {
        value = /* detect it */;
        return this;
    }

    public Builder fromUsername(String name) {
        // fromUUID, fromBase64 etc
        value = new SkullValue(ValueType.NAME, name);
        return this;
    }

    public SkullMeta apply() {
        return applySkin(meta, value); // should make a separate method for this
    }

    public CompletableFuture<SkullMeta> applyAsync() {
        return CompletableFuture.supplyAsync(this::apply, PROFILE_EXECUTOR);
    }
}

Also, which versions did you test this with? It should support 1.8+

@CryptoMorin CryptoMorin added bug Something isn't working enhancement New feature or request performance Performance improvements labels May 23, 2024
@Condordito
Copy link
Contributor Author

Condordito commented May 26, 2024

@CryptoMorin

I made it more configurable and moved it into a separate package. It now logs exceptions using the debug level. So far, I've tested it on 1.8.9, 1.12.2, 1.19.4, and 1.20.4. I still need to document it and check for any additional remaps

@Condordito Condordito marked this pull request as draft May 26, 2024 21:45
- Fix `XSkull.TEXTURES` constant (old clients require the HTTP protocol to render properly).
- Add 1.8 remap for MinecraftServer getter.
- Remove `XSkull.applySkin` in favor of `XSkull#of`.
- Remove cache for profiles generated from base64.
- Log exceptions using debug level.
@Condordito Condordito marked this pull request as ready for review May 27, 2024 05:00
@CryptoMorin
Copy link
Owner

Thank you very much! I'll have to test this a little and see if more changes are needed.

@CryptoMorin CryptoMorin merged commit 8f5e6e6 into CryptoMorin:master May 30, 2024
1 check passed
@Condordito Condordito deleted the fix/xskull-caching branch May 30, 2024 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request performance Performance improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants