Skip to content

RFC: Embed non-standard skin parts in json skin files and load embedded parts #3142

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

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Robyt3
Copy link
Contributor

@Robyt3 Robyt3 commented Apr 7, 2022

See #2271.

Skin saving

Embed the png files of non-standard skin parts into the json skin files as base64 encoded strings when saving the skin.

Standard (SKINFLAG_STANDARD) means that the skin part was loaded from the save path (the first path listed in storage.cfg).

Skin loading

Skin parts are first loaded based on the filename. If the filename search fails and embedded data is present, skin parts are decoded from the embedded data.

Base64 implementation

  • Adapted from public domain code.
  • The encode function should produce valid base64 according to section 4 of RFC 4648.
  • The decode function is not strictly accoding to RFC 4648, as it is most lenient and allows (silently skips) all non-base64-alphabet characters (including spaces, line breaks and misplaced padding characters), as this is the easiest and most performant to implement. It is also the most convenient for users of the function, as it requires no error handling at the call site.
  • The decode function will also null-terminate the data, as this makes usage more convenient (for testing) with null-terminated strings.
  • Using the full size 256 lookup table for decoding results in a 4.6x speedup compared to using a lookup function.
    • The second half of the lookup table is all 64, so it could be cut down by adding one if-statement instead, but that would reduce the speedup to 3.8x when compared to using a lookup function.
    • Performance was tested by decoding realistic inputs (i.e. encoded skin parts).

PNG loading

  • Add LoadPNG method to load a png file from memory using pnglite, by implementing a pnglite read callback that operates on a memory buffer.
  • Fix a memory leak in pnglite when png data cannot be loaded.
  • Improve error messages.

Open issues / Questions

  • Should embedded skin parts be extracted into the skin parts folder as png files?
    • Extracted skin parts should go in a different folder than the user's skin parts.
    • A skin part md5 hash should be added to the json file when embedding skin parts.
  • Saving skins with embedded parts will get rid of the embedded part, as embedding currently requires the skin part png file to be available. I can imagine the following solutions, ordered by how easy they would be to realise in my opinion:
    • Solution 1: Do not allow changing/saving skins that contain any embedded parts. Original authors can still save the skin, as the have the skin part files locally and the embedded parts are therefore not loaded.
    • Solution 2: Keep embedded skin parts in memory, so they can be embedded from memory when necessary.
    • Solution 3: Extract the embedded skin parts as png files into the skin parts folder, so they can be embedded from file when necessary (see above).
    • Solution 4: When saving a skin with embedded parts, load the embedded parts from the skins in which they are embedded. Requires keeping track of which part was loaded from which file and is problematic when skin files that contain embedded parts are overridden.

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.

1 participant