Fix asserts/crashes caused by missing component serialized identifiers. #16973
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
Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>
forkercat
approved these changes
Oct 26, 2023
AMZN-Gene
approved these changes
Oct 26, 2023
lemonade-dm
approved these changes
Oct 26, 2023
AMZN-alexpete
approved these changes
Oct 26, 2023
AMZN-alexpete
pushed a commit
to aws-lumberyard-dev/o3de
that referenced
this pull request
Dec 14, 2023
…s. (o3de#16973) Signed-off-by: Mike Balfour <82224783+mbalfour-amzn@users.noreply.github.com>
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.
What does this PR do?
Fixes #16807 .
PR #15985 previously fixed some of the issues related to disabled/pending components and how they work relative to component aliases, but there was still a case where loading a prefab containing a disabled or pending component after an Editor restart would assert and crash.
The reason for this is that while editing, the component would get added to the component, then get its SerializedIdentifier set, then get added to the PendingComponent map. If it meets all of its requirements, it then gets removed from the list and added to the entity. If not, it stays in the pending map.
However, when serializing out the component, the SerializedIdentifier doesn't directly get serialized out with the component. It gets serialized as the key for the PendingComponent map, but it wasn't getting added back as a SerializedIdentifier. Consequently, when the entity gets activated and tries to add components from the PendingComponent list, it fails to successfully look it up in the IsComponentPending() call due to the missing SerializedIdentifier, which ultimately leads to asserts and crashes.
(All of the above also applies to disabled components)
The fix is to set the SerializedIdentifier at the point of deserialization, so that when the component is instantiated in the PendingComponent map, it gets a SerializedIdentifier that matches the key to the map, which ensures that it can be found.
NOTE: If the prefab is hand-edited and the serialized identifier is modified to something that clashes with other components on the entity, chaos may ensue, since the assumption is that the read-in identifier is correct and unique.
How was this PR tested?
Added pending and disabled components to an entity and saved the level. Closed the Editor. Restarted the Editor. Loaded the level and verified that the components could be deleted, re-added, enabled, disabled, without any asserts or crashes.