aframevr / aframe

:a: Web framework for building virtual reality experiences.
https://aframe.io/
MIT License
16.56k stars 3.93k forks source link

Components with dependencies attach extra components to entities even when component of same type is attached. #5536

Open Omegahed opened 3 months ago

Omegahed commented 3 months ago

Description:

A-Frame components with dependencies add additional components to entities even when that type of component is already attached to the entity. It appears that the extra attachment happens when the entity has the component using an ID (e.g., foo__1) but does not have an attached component using just the base component name (e.g., foo).

Expected: Components using the ID should be recognized as satifying the dependency so that extra components are not attached.

Note: The animation component is used as an example, but the same thing happens with other components.

Test scene:

<html>
  <head>
    <script src="https://aframe.io/releases/1.6.0/aframe.min.js"></script>
    <script>
      AFRAME.registerComponent("test-component-with-dependency", {
        dependencies: ["animation"],
        init: function () {
          console.log("components:", this.el.id, this.el.components);
        },
      });
    </script>
    <script>
      AFRAME.registerComponent("test-component-without-dependency", {
        init: function () {
          console.log("components:", this.el.id, this.el.components);
        },
      });
    </script>
  </head>
  <body>
    <!-- Will attach additional animation component to entity even though
    animation components are already attached. -->
    <a-entity
      id="with-dependency"
      test-component-with-dependency
      animation__0
      animation__1
    ></a-entity>
    <!-- Will not attach additional animation component to entity. -->
    <a-entity
      id="without-dependency"
      test-component-without-dependency
      animation__0
      animation__1
    ></a-entity>
    <a-scene>
      <a-box position="-1 0.5 -3" rotation="0 45 0" color="#4CC3D9"></a-box>
      <a-sphere position="0 1.25 -5" radius="1.25" color="#EF2D5E"></a-sphere>
      <a-cylinder
        position="1 0.75 -3"
        radius="0.5"
        height="1.5"
        color="#FFC65D"
      ></a-cylinder>
      <a-plane
        position="0 0 -4"
        rotation="-90 0 0"
        width="4"
        height="4"
        color="#7BC8A4"
      ></a-plane>
      <a-sky color="#ECECEC"></a-sky>
    </a-scene>
  </body>
</html>

Resulting console output demonstrating extra animation component attached:

components: with-dependency 
  Object
    animation: n {el: a-entity#with-dependency, id: undefined, attrName: 'animation', evtDetail: {…}, initialized: true, …}
    animation__0: n {el: a-entity#with-dependency, id: '0', attrName: 'animation__0', evtDetail: {…}, initialized: true, …}
    animation__1: n {el: a-entity#with-dependency, id: '1', attrName: 'animation__1', evtDetail: {…}, initialized: true, …}
    test-component-with-dependency: n {el: a-entity#with-dependency, id: undefined, attrName: 'test-component-with-dependency', evtDetail: {…}, initialized: true, …}
    [[Prototype]]: Object 

components: without-dependency 
  Object
    animation__0: n {el: a-entity#without-dependency, id: '0', attrName: 'animation__0', evtDetail: {…}, initialized: true, …}
    animation__1: n {el: a-entity#without-dependency, id: '1', attrName: 'animation__1', evtDetail: {…}, initialized: true, …}
    test-component-without-dependency: n {el: a-entity#without-dependency, id: undefined, attrName: 'test-component-without-dependency', evtDetail: {…}, initialized: true, …}
    [[Prototype]]: Object
dmarcos commented 3 months ago

Yeah. I don't think dependencies consider multiple components instances. Can you share in what exact context you found the issue? I'm assuming was not with animation.

Omegahed commented 3 months ago

I originally found it trying to enable functionality where something like

var fooComponents = this.el.components.foo;

would return all of the foo components attached to an entity, rather than just the first one. It was at that point that I realized that if the component only had instances with IDs (e.g., foo__0, foo__1, etc.), that specifying foo in the dependencies of another component would then add an extra foo component to the entity.

Based on my understanding of the dependencies documentation I did not expect it to attach an extra instance to the entity; and I would expect that the dependency requirement is satisfied even if an ID is specified. I assumed that it would throw an error or a warning if the component is not attached, not that it would automatically attach one. This behavior probably provides a lot of convienience in a lot of situations, but if you're not expecting it (i.e., it's not documented) it might cause some confusion; but it only seems to add an extra instance when you have an instance attached using an ID and it doesn't recognize it.

The multiple documentation gives the following example

<a-scene>
  <a-entity
    sound="src: url(sound.mp3)"
    sound__beep="src: url(beep.mp3)"
    sound__boop="src: url(boop.mp3)"
  ></a-entity>
</a-scene>

which correctly avoids adding an extra sound component. I do think it's useful when having multiple component instances on an entity for them to all have unique IDs to keep them better organized. In that case I would have something more like:

<a-scene>
  <a-entity
    sound__beep="src: url(beep.mp3)"
    sound__boop="src: url(boop.mp3)"
    sound__blop="src: url(blop.mp3)"
  ></a-entity>
</a-scene>
Omegahed commented 2 months ago

I was thinking about this a little more, and I think the other side of this is that if you have multiple components of the same type attached and that component type is specified in the dependencies that only the component without the IDs get loaded in the specified order.

For example if you had a component with:

dependencies: [ "sound", "animation" ]

and the following scene:

<a-scene>
  <a-entity
    animation
    sound="src: url(sound.mp3)"
    sound__beep="src: url(beep.mp3)"
    sound__boop="src: url(boop.mp3)"
    foo
    animation__1
  ></a-entity>
</a-scene>

I think the order the components will load is sound, animation, sound__beep, sound__boop, foo, animation__1.

I would expect it to load all of the sound components first, and then all the animation components resulting in a loading order of sound, sound__beep, sound__boop, animation, animation__1, foo.

If we change it up a little further like:

<a-scene>
  <a-entity
    foo
    animation__0
    sound__beep="src: url(beep.mp3)"
    sound__boop="src: url(boop.mp3)"
    sound__blop="src: url(blop.mp3"
    animation__1
  ></a-entity>
</a-scene>

I think that same component with the dependencies: [ "sound", "animation" ] will load in the order of foo, animation__0, sound__beep, sound_boop, sound__blop, animation__1. This ends up effectively ignoring the dependency order.

In fairness, there's nothing in the documentation that explicitly supports my assumptions. But I think the mental model is much clearer if it takes IDs into account.