BIP2 #14
Merged
BIP2 #14
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This format can store an arbitrary number of resolutions of the image in one file. At the moment we're just doing two though, the first one for icons (32x32), the second for images (full size).
I'm not 100% satisfied with this code, but it does work.
Well actually I converted them from the original JPG / PNG files, but the result is the same.
I think this is a bit more elegant.
Removed some newlines which I felt didn't belong. Renamed count to length when talking about (width * height), because that's what I call it elsewhere in the code too, and count refers to the amount of images in a BIP file. Updated an assert message that I forgot to update earlier. Changed how the data dictionary is used, so now icon_size and icon_pixels are set on its creation, and only overwritten in if the icon needed resizing.
I did the same for the loader and I like to be consistent.
Functionally the same but more explicit.
Just to show that previews can be used as icons now.
coreprocess
suggested changes
May 3, 2021
This code is a bit opaque otherwise.
Interestingly, the loader seems to support BIP2 files with only one resolution, even though it expects two. I haven't tested it, but just reading the code, that's how it probably works.
coreprocess
approved these changes
May 3, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I'd like a second opinion, because I'm not sure whether this is ready.