Fix componentAdded/componentRemoved notifications dispatch to components

When a component is added to an entity, two change
notifications were sent but they both were directed
to the entity.
This makes sure one message is sent to the entity
and one to the component, as was intended.

Same for remove notifications.

Fixed the unit tests which where incorrectly testing for the
subject id of the change notification.

Change-Id: Ic697c08e92e884abf28e675e4ae2b44a93ca45d5
Reviewed-by: Paul Lemire <paul.lemire@kdab.com>

6 files changed, 57 insertions, 10 deletions

diff --git a/src/core/changes/qcomponentaddedchange.cpp b/src/core/changes/qcomponentaddedchange.cpp
index e00c4ffab..41f7a2340 100644
--- a/src/core/changes/qcomponentaddedchange.cpp
+++ b/src/core/changes/qcomponentaddedchange.cpp
@@ -75,7 +75,7 @@ QComponentAddedChangePrivate::QComponentAddedChangePrivate(const QEntity *entity
  */
 
 /*!
- * Constructs a new QComponentAddedChange with with \a entity and  \a component.
+ * Constructs a new QComponentAddedChange which will notify \a entity that \a component was added
  */
 QComponentAddedChange::QComponentAddedChange(const QEntity *entity,
                                              const QComponent *component)
@@ -84,6 +84,16 @@ QComponentAddedChange::QComponentAddedChange(const QEntity *entity,
 {
 }
 
+/*!
+ * Constructs a new QComponentAddedChange which will notify \a component that it was added to \a entity
+ */
+QComponentAddedChange::QComponentAddedChange(const QComponent *component,
+                                             const QEntity *entity)
+    : QSceneChange(*new QComponentAddedChangePrivate(entity, component),
+                   ComponentAdded, component->id())
+{
+}
+
 QComponentAddedChange::~QComponentAddedChange()
 {
 }
diff --git a/src/core/changes/qcomponentaddedchange.h b/src/core/changes/qcomponentaddedchange.h
index a62cac116..e7676a026 100644
--- a/src/core/changes/qcomponentaddedchange.h
+++ b/src/core/changes/qcomponentaddedchange.h
@@ -55,6 +55,8 @@ class QT3DCORESHARED_EXPORT QComponentAddedChange : public QSceneChange
 public:
     explicit QComponentAddedChange(const QEntity *entity,
                                    const QComponent *component);
+    explicit QComponentAddedChange(const QComponent *component,
+                                   const QEntity *entity);
     ~QComponentAddedChange();
 
     QNodeId entityId() const Q_DECL_NOTHROW;
diff --git a/src/core/changes/qcomponentremovedchange.cpp b/src/core/changes/qcomponentremovedchange.cpp
index 5e5dfa5aa..e2129169a 100644
--- a/src/core/changes/qcomponentremovedchange.cpp
+++ b/src/core/changes/qcomponentremovedchange.cpp
@@ -75,7 +75,7 @@ QComponentRemovedChangePrivate::QComponentRemovedChangePrivate(const QEntity *en
  */
 
 /*!
- * Constructs a new QComponentRemovedChange with \a entity and  \a component.
+ * Constructs a new QComponentRemovedChange which will notify \a entity that \a component was removed.
  */
 QComponentRemovedChange::QComponentRemovedChange(const QEntity *entity,
                                                  const QComponent *component)
@@ -84,6 +84,17 @@ QComponentRemovedChange::QComponentRemovedChange(const QEntity *entity,
 {
 }
 
+/*!
+ * Constructs a new QComponentRemovedChange which will notify \a component that it was removed from \a entity
+ */
+QComponentRemovedChange::QComponentRemovedChange(const QComponent *component,
+                                                 const QEntity *entity)
+    : QSceneChange(*new QComponentRemovedChangePrivate(entity, component),
+                   ComponentRemoved, component->id())
+{
+
+}
+
 QComponentRemovedChange::~QComponentRemovedChange()
 {
 }
diff --git a/src/core/changes/qcomponentremovedchange.h b/src/core/changes/qcomponentremovedchange.h
index 3c57fe26f..66743b1ae 100644
--- a/src/core/changes/qcomponentremovedchange.h
+++ b/src/core/changes/qcomponentremovedchange.h
@@ -55,6 +55,8 @@ class QT3DCORESHARED_EXPORT QComponentRemovedChange : public QSceneChange
 public:
     explicit QComponentRemovedChange(const QEntity *entity,
                                      const QComponent *component);
+    explicit QComponentRemovedChange(const QComponent *component,
+                                     const QEntity *entity);
     ~QComponentRemovedChange();
 
     QNodeId entityId() const Q_DECL_NOTHROW;
diff --git a/src/core/nodes/qcomponent.cpp b/src/core/nodes/qcomponent.cpp
index 8e337adf6..f4e59e058 100644
--- a/src/core/nodes/qcomponent.cpp
+++ b/src/core/nodes/qcomponent.cpp
@@ -73,7 +73,7 @@ void QComponentPrivate::addEntity(QEntity *entity)
         m_scene->addEntityForComponent(m_id, entity->id());
     }
 
-    const auto componentAddedChange = QComponentAddedChangePtr::create(entity, q);
+    const auto componentAddedChange = QComponentAddedChangePtr::create(q, entity);
     notifyObservers(componentAddedChange);
     Q_EMIT q->addedToEntity(entity);
 }
@@ -86,7 +86,7 @@ void QComponentPrivate::removeEntity(QEntity *entity)
 
     m_entities.removeAll(entity);
 
-    const auto componentRemovedChange = QComponentRemovedChangePtr::create(entity, q);
+    const auto componentRemovedChange = QComponentRemovedChangePtr::create(q, entity);
     notifyObservers(componentRemovedChange);
     Q_EMIT q->removedFromEntity(entity);
 }
diff --git a/tests/auto/core/nodes/tst_nodes.cpp b/tests/auto/core/nodes/tst_nodes.cpp
index 49618821c..717bb3200 100644
--- a/tests/auto/core/nodes/tst_nodes.cpp
+++ b/tests/auto/core/nodes/tst_nodes.cpp
@@ -958,15 +958,22 @@ void tst_Nodes::appendingParentlessComponentToEntity()
         // return early in such a case.
 
         // Check that we received ComponentAdded
-        for (const auto event: { entitySpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentAddedChange>(),
-                                 componentSpy.events.takeLast().change().dynamicCast<Qt3DCore::QComponentAddedChange>() })
         {
+            const auto event = entitySpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentAddedChange>();
             QCOMPARE(event->type(), Qt3DCore::ComponentAdded);
             QCOMPARE(event->subjectId(), entity->id());
             QCOMPARE(event->entityId(), entity->id());
             QCOMPARE(event->componentId(), comp->id());
             QCOMPARE(event->componentMetaObject(), comp->metaObject());
         }
+        {
+            const auto event = componentSpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentAddedChange>();
+            QCOMPARE(event->type(), Qt3DCore::ComponentAdded);
+            QCOMPARE(event->subjectId(), comp->id());
+            QCOMPARE(event->entityId(), entity->id());
+            QCOMPARE(event->componentId(), comp->id());
+            QCOMPARE(event->componentMetaObject(), comp->metaObject());
+        }
     }
 }
 
@@ -997,15 +1004,22 @@ void tst_Nodes::appendingComponentToEntity()
         QVERIFY(comp->parentNode() == entity.data());
         QCOMPARE(entitySpy.events.size(), 1);
         QVERIFY(entitySpy.events.first().wasLocked());
-        for (const auto event: { entitySpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentAddedChange>(),
-             componentSpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentAddedChange>() })
         {
+            const auto event = entitySpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentAddedChange>();
             QCOMPARE(event->type(), Qt3DCore::ComponentAdded);
             QCOMPARE(event->subjectId(), entity->id());
             QCOMPARE(event->entityId(), entity->id());
             QCOMPARE(event->componentId(), comp->id());
             QCOMPARE(event->componentMetaObject(), comp->metaObject());
         }
+        {
+            const auto event = componentSpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentAddedChange>();
+            QCOMPARE(event->type(), Qt3DCore::ComponentAdded);
+            QCOMPARE(event->subjectId(), comp->id());
+            QCOMPARE(event->entityId(), entity->id());
+            QCOMPARE(event->componentId(), comp->id());
+            QCOMPARE(event->componentMetaObject(), comp->metaObject());
+        }
     }
 }
 
@@ -1040,14 +1054,22 @@ void tst_Nodes::removingComponentFromEntity()
         QCOMPARE(entitySpy.events.size(), 1);
         QVERIFY(entitySpy.events.first().wasLocked());
         QCOMPARE(componentSpy.events.size(), 1);
-        for (const auto event: { entitySpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentRemovedChange>(),
-                                 componentSpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentRemovedChange>() }) {
+        {
+            const auto event = entitySpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentRemovedChange>();
             QCOMPARE(event->type(), Qt3DCore::ComponentRemoved);
             QCOMPARE(event->subjectId(), entity->id());
             QCOMPARE(event->entityId(), entity->id());
             QCOMPARE(event->componentId(), comp->id());
             QCOMPARE(event->componentMetaObject(), comp->metaObject());
         }
+        {
+            const auto event = componentSpy.events.takeFirst().change().dynamicCast<Qt3DCore::QComponentRemovedChange>();
+            QCOMPARE(event->type(), Qt3DCore::ComponentRemoved);
+            QCOMPARE(event->subjectId(), comp->id());
+            QCOMPARE(event->entityId(), entity->id());
+            QCOMPARE(event->componentId(), comp->id());
+            QCOMPARE(event->componentMetaObject(), comp->metaObject());
+        }
     }
 }