32bpp assets dont need a palette to be defined. #19445
Merged
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
|
#3692 will be the proper fix, but this PR seems like a reasonable workaround to take before then. 👍 to the idea, but not tested. |
|
It might be worth adding a comment explaining why the null check is needed. |
IceReaper
force-pushed
the
optionalpalette
branch
from
e490d33
to
52dd0ed
Compare
|
Added a small comment to explain. |
pchote
reviewed
Jun 11, 2021
IceReaper
force-pushed
the
optionalpalette
branch
from
52dd0ed
to
2869b14
Compare
IceReaper
force-pushed
the
optionalpalette
branch
from
2869b14
to
515a8e3
Compare
|
Updated and rebased. |
reaperrr
approved these changes
Jun 25, 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.
Currently when you use 32bpp sprites you encounter dozens of crashes quite easily. Most of them are tied to the palettes being null. The dirty workaround was to define a palette in yaml which exists but wont be used on 32bpp assets to get around the crash.
This changes allows palettes to be null, resulting in palettes being not resolved in that case.
There is a note at the end of the SpriteRenderable constructor which sets the palette also to null if an rgba asset is used, so this pr comes along with that behavior and just makes sure the traits do not crash before.