cloning an entity with mixin value will sometimes not apply the mixin correctly to the cloned entity

[kfarr]

This is the second of 2 errors encountered using the duplicate (clone) entity feature (keyboard shortcut d)

• when an entity is cloned, the value(s) for the mixin attribute are copied. See flushtodom() called during inspector session outputs old values (values prior to inspector changes) #688 for a deeper explanation.
• sometimes, but not always, when cloning an entity the mixin will not apply correctly to the newly cloned entity. For example, if the mixin is a gltf model sometimes the newly cloned entity will not display the gltf model.
• sometimes, but not always, when choosing a new mixin value for an existing entity the new mixin value will not apply correctly. For example, if the existing mixin is a gltf model A and a user chooses gltf model B, sometimes the newly cloned entity will not display any gltf model and appear empty.

Workaround:
We encountered this issue in both cases in 3DStreet for some models. We came up with a workaround where we set the mixin value first to '' and then re-set the mixin value to the desired value. This appears to work in all cases that we have tested.

Here are the 2 cases where we use this workaround in 3DStreet:

• When updating a single mixin*: https://github.com/3DStreet/3dstreet-editor/blob/master/src/components/components/Mixins.js#L66
• When cloning an entity: https://github.com/3DStreet/3dstreet-editor/blob/master/src/lib/entity.js#L142

here is a video of a duplicated entity not showing the mixin value as expected. the UI workaround is to remove the mixin and re-apply the same mixin:
https://github.com/aframevr/aframe-inspector/assets/470477/b7cb3adb-1408-4709-9cd2-3a65b4990ca4

*note that a single mixin concept is unique to 3dstreet, default aframe-inspector uses select multiple

here is the URL to reproduce:
https://mixin-gltf-url-error.glitch.me/

here are instructions to reproduce:
press ctl-alt-i to invoke the inspector, select the mixin firetruck, while the truck entity is selected press d, note how the firetruck is not visible in the newly cloned entity.

expected behavior:

• when cloning an item the duplicated item displays the cloned entity's mixin as expected

[vincentfretin]

@3DStreet sponsored me to work on this issue. Thank you! c-frame/sponsorship#3

[vincentfretin]

aframevr/aframe#2823 is probably related to this issue.

[vincentfretin]

Removing the mixin is removing the gltf-model component but you don't see it disappears in the right panel (that's another issue, a forceUpdate is missing on the ComponentsContainer React component for the entityupdate event with component "mixin"), you can select another entity and select the previous entity back to see it. Another issue is the UI is not updating with the mixin name at the top of the right panel and in the left panel when you add or remove a mixin.
When you add the mixin back, it adds the gltf-model component again. That's why your workaround work.

I see in the DOM that when gltf-model is set with a mixin, that gltf-model (empty) is shown.
If I set a gltf-model component with an url on an entity without mixin, the DOM shows the gltf-model="theurl"
The main issue I think may be that gltf-model="" appears in the DOM.
I don't know at this point if this is something we can fix in aframe, related to mixin, to not show components in DOM so that native cloneNode(true) api works correctly, or if we'll need workaround like you did after cloning. I need to dig this further.
Also currently in the right panel we don't visualize that a component is coming from a mixin and that removing a mixin will remove associated components.

[vincentfretin]

You said that you reproduce the issue sometimes, but not always. I reproduce it every time. Maybe you don't reproduce it when the gltf-model component is really set on the entity, overriding the mixin, even if it's the same url, you can see it in with Chrome inspector, if you see the url, then the gltf-model component is set directly and duplicating the entity will show the model.

[vincentfretin]

Some other notes:
In the DOM it shows
<a-entity position="0 0 -9" mixin="fire-truck-rig" gltf-model=""></a-entity>
or
<a-entity position="0 0 -9" mixin="fire-truck-rig" gltf-model="thesameurlasmixin"></a-entity> if you touched the url via the right panel

If you use "copy entity HTML to clipboard" button, you get
<a-entity position="0 0 -9" mixin="fire-truck-rig"></a-entity>

[vincentfretin]

@kfarr Looking at getEntityClipboardRepresentation function that is used for the "copy entity HTML to clipboard" feature, that is using prepareForSerialization and optimizeComponents functions

aframe-inspector/src/lib/entity.js

Lines 157 to 219 in 72fd461

	export function getEntityClipboardRepresentation(entity) {
	var clone = prepareForSerialization(entity);
	return clone.outerHTML;
	}
	/**
	* Returns a copy of the DOM hierarchy prepared for serialization.
	* The process optimises component representation to avoid values coming from
	* primitive attributes, mixins and defaults.
	*
	* @param {Element} entity Root of the DOM hierarchy.
	* @return {Elment} Copy of the DOM hierarchy ready for serialization.
	*/
	function prepareForSerialization(entity) {
	var clone = entity.cloneNode(false);
	var children = entity.childNodes;
	for (var i = 0, l = children.length; i < l; i++) {
	var child = children[i];
	if (
	child.nodeType !== Node.ELEMENT_NODE ||
	(!child.hasAttribute('aframe-injected') &&
	!child.hasAttribute('data-aframe-inspector') &&
	!child.hasAttribute('data-aframe-canvas'))
	) {
	clone.appendChild(prepareForSerialization(children[i]));
	}
	}
	optimizeComponents(clone, entity);
	return clone;
	}
	/**
	* Removes from copy those components or components' properties that comes from
	* primitive attributes, mixins, injected default components or schema defaults.
	*
	* @param {Element} copy Destinatary element for the optimization.
	* @param {Element} source Element to be optimized.
	*/
	function optimizeComponents(copy, source) {
	var removeAttribute = HTMLElement.prototype.removeAttribute;
	var setAttribute = HTMLElement.prototype.setAttribute;
	var components = source.components || {};
	Object.keys(components).forEach(function (name) {
	var component = components[name];
	var result = getImplicitValue(component, source);
	var isInherited = result[1];
	var implicitValue = result[0];
	var currentValue = source.getAttribute(name);
	var optimalUpdate = getOptimalUpdate(
	component,
	implicitValue,
	currentValue
	);
	var doesNotNeedUpdate = optimalUpdate === null;
	if (isInherited && doesNotNeedUpdate) {
	removeAttribute.call(copy, name);
	} else {
	var schema = component.schema;
	var value = stringifyComponentValue(schema, optimalUpdate);
	setAttribute.call(copy, name, value);
	}
	});
	}

it does all the logic of removing attributes if it's inherited from mixin (so the gltf-model component).
The function documentation is exactly this: "Removes from copy those components or components' properties that comes from
primitive attributes, mixins, injected default components or schema defaults."
It actually does a clone of each child one by one and getting the outerHTML of the clone.

I think for the duplicate feature we should really just change the line
const clone = entity.cloneNode(true);
by
const clone = prepareForSerialization(entity);
This solves the issue completely.

[vincentfretin]

For your single mixin feature, you should just keep your workaround, that's fine. It indeed force it to remove the gltf-model component and create another one.

[vincentfretin]

PR #696 fixes the issue.
For UI parts that are not updated, I created another issue #697 and #698.

[kfarr]

confirming this works as expected for me, can close once @dmarcos merges #696