IDataContainer::GetElement (for variant) must return valid element when given DefaultElementCrc
Issue #, if available: N/A
Description of changes: The following code (InstanceDataHierarchy.cpp:263) which handles adding elements to containers through their IDataContainer implementation assumes this is true:
const AZ::SerializeContext::ClassElement* containerClassElement = container->GetElement(container->GetDefaultElementNameCrc());
AZ_Assert(containerClassElement != NULL, "We should have a valid default element in the container, otherwise we don't know what elements to make!");
It’s easy enough to add an early exit if the assertion fails, however, this doesn’t allow the variant data container (or any other data container which returns null from GetElement when given GetDefaultElementNameCrc) to actually function as a container.
Other contains implement this GetElement function by returning an element only for the default element name; and the contents of the element are “synthetic” in the sense that they contain meta-data which describes what kind of elements can go into the container, how to display those elements, how to query the user for datums of that element type, etc., as opposed to describing a “physical” element which is really stored in the parent datum (which by necessity would have things like a real offset, a non-dummy name, etc.). (Aside: SerializeContext::ClassElement probably shouldn’t be used for this purpose, but that’s for another issue).
This merge request changes the variant IDataContainer to conform to these semantics.
Note that this patch uses SerializeGenericTypeInfo of the variant type inside the IDataContainer implementation, which would cause an infinite loop; so the GetClassTypeId implementation is changed to avoid this. Unlike most containers, variant cannot return a class element here whose type indicates the element type, because variant has many different element types, and there is not necessarily a preferred choice - return the leftmost element type or anything else which gives preferences to a particular alternative will only require more special casing in other places which consume such “synthetic” class elements. I’ve chosen to implement this synthetic element with a m_typeId which is precisely that of the variant class, because it makes the most sense to me. However, no choice of type ID “really” makes sense here - SerializeContext::ClassElement is simply the wrong datum to return from this function.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Hi @yuriy0, thanks for contributing to LY we'll take a look at the change.