flushtodom() called during inspector session outputs old values (values prior to inspector changes)

[kfarr]

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

• when an entity is cloned, first the entity's current values are flushed to the DOM since the clone feature uses the vanilla javascript method cloneNode to clone an entity.
• when flushToDOM is executed, the values returned are those of the entity prior to invoking the inspector. In other words, if I change position or other attribute using the editor, upon pressing d to duplicate, the entity will return to its prior location before making the changes in the editor.

here is the flushtodom for an entity:
https://github.com/aframevr/aframe/blob/v1.4.0/src/core/a-entity.js#L698

for each component in the entity, it runs this flushtodom:
https://github.com/aframevr/aframe/blob/v1.4.0/src/core/component.js#L267

note how it is using this.attrValue instead of getAttribute([componentName], [attributeName]) which may be returning the values in the original html instead of the current values using A-Frame's recommended api

here is a video
https://github.com/aframevr/aframe-inspector/assets/470477/b7cb3adb-1408-4709-9cd2-3a65b4990ca4

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, change the position of the mixin firetruck, while the truck entity is still selected press d, note how the firetruck returns to its original position

expected behavior:

• when cloning an item the original and the duplicated item retain attribute values modified during the current inspector session

[kfarr]

3 related issues on aframevr repo:

• Inspector Mode flushToDOM aframe#4289
• .flushToDOM() does not update DOM values with A-Frame visual inspector  aframe#4084
• clone entity and children with cloneNode and innerHTML not working as expected aframe#2823

[kfarr]

Here is some research on how getAttribute works, since that appears to correctly reflect inspector edits:
https://github.com/aframevr/aframe/blob/96b6f46eebd17655f137a1efdc86594040ff03a4/src/core/a-entity.js#L728

It defaults to object3D values for position, rotation, scale, visible instead of DOM values.

Perhaps we could do the same right here? Either copy the logic from getAttribute or somehow call it. Might be easier to just replicate the logic since it's only those 4 cases of position,rotation,scale,visible.

So perhaps this is actually a pull request on aframevr repo instead?

[vincentfretin]

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

[vincentfretin]

Duplicate entity calls entity.flushToDOM() (https://github.com/aframevr/aframe/blob/v1.4.0/src/core/a-entity.js#L707) that calls for each component component.flushToDOM() (https://github.com/aframevr/aframe/blob/v1.4.0/src/core/component.js#L268), it gets the value from this.attrValue that is the parsed version of what was in the DOM initially or the latest setAttribute call. Here it resets the value from the DOM because this attrValue didn't change, no setAttribute was called when moving via TransformControls from the viewport, the object3D.position is modified directly.
For position, scale, rotation, that's indeed wrong, the latest value is from object3D.position/rotation/scale (this is what is modified when you move the entity from the viewport). So for those cases, instead of using attrValue, we would indeed think that we need to have a similar implementation as getAttribute https://github.com/aframevr/aframe/blob/96b6f46eebd17655f137a1efdc86594040ff03a4/src/core/a-entity.js#L731-L734
But if we do a fix in component.flushToDOM(), that would only works if you had the component initially on the entity. For example for scale, if you didn't have it, the entity.flushToDOM() wouldn't auto add the scale component, and so it wouldn't be present in the components of the duplicated entity either. I don't think that auto creating components from entity.flushToDOM() is a valid fix here.

As mentionned in aframevr/aframe#4084, when you change the position from the right panel and then duplicate the entity, the position is correct. That's because the Vec3Widget onChange actually do a entity.setAttribute('position', {x:1, y:2, z:3}) so it's creating the position component or updating it so this.attrValue is updated.
The correct fix here is really to call setAttribute when editing via the TransformControls.
TransformControls is emitting the objectChange event that is reemitted with a different entityupdate event here

aframe-inspector/src/lib/viewport.js

Lines 61 to 84 in 72fd461

	// Emit update event for watcher.
	let component;
	let value;
	if (evt.mode === 'translate') {
	component = 'position';
	value = `${object.position.x} ${object.position.y} ${object.position.z}`;
	} else if (evt.mode === 'rotate') {
	component = 'rotation';
	const d = THREE.MathUtils.radToDeg;
	value = `${d(object.rotation.x)} ${d(object.rotation.y)} ${d(
	object.rotation.z
	)}`;
	} else if (evt.mode === 'scale') {
	component = 'scale';
	value = `${object.scale.x} ${object.scale.y} ${object.scale.z}`;
	}
	Events.emit('entityupdate', {
	component: component,
	entity: transformControls.object.el,
	property: '',
	value: value
	});
	Events.emit('entitytransformed', transformControls.object.el);

We currently listen on the entityupdate event in CommonComponents.js to force a react update and the right panel is getting directly the value from object3D.position, also for rotation doing the rad2deg conversion.

aframe-inspector/src/components/components/CommonComponents.js

Lines 58 to 67 in 72fd461

	return ['position', 'rotation', 'scale', 'visible'].map((componentName) => {
	const schema = AFRAME.components[componentName].schema;
	var data = entity.object3D[componentName];
	if (componentName === 'rotation') {
	data = {
	x: THREE.MathUtils.radToDeg(entity.object3D.rotation.x),
	y: THREE.MathUtils.radToDeg(entity.object3D.rotation.y),
	z: THREE.MathUtils.radToDeg(entity.object3D.rotation.z)
	};
	}

I propose to change the code there to do the setAttribute in the entityupdate listener.

[vincentfretin]

I believe #695 fixes the issue in the correct way.
For #689 about mixin and gltf-model, that seems to be a completely different issue not related to this one, I didn't start to investigate it yet.

[vincentfretin]

Your linked issue aframevr/aframe#2823 seems to be more related to #689 and not this one.

[vincentfretin]

I moved the fix out of the React component, a proper place is the entityupdate listener in viewport.js that also handle other things. That way the fix is still there if I'm doing a different UI in aframe-editor later. And I simplified it, just using detail.value, we already have the rotation in degrees there.