Merge branch 'main' into docs/1037-primers

Author: jslee02

dart/common/Composite.cpp

@@ -243,7 +243,7 @@ void Composite::matchAspects(const Composite* otherComposite)
   }
 
   for (auto& aspect : mAspectMap)
-    aspect.second = nullptr;
+    _replaceAspect(aspect.second, nullptr);
 
   duplicateAspects(otherComposite);
 }
@@ -262,22 +262,29 @@ void Composite::removeFromComposite(Aspect* aspect)
     aspect->loseComposite(this);
 }
 
+//==============================================================================
+void Composite::_replaceAspect(
+    std::unique_ptr<Aspect>& slot, std::unique_ptr<Aspect> replacement)
+{
+  removeFromComposite(slot.get());
+  slot = std::move(replacement);
+  addToComposite(slot.get());
+}
+
 //==============================================================================
 void Composite::_set(std::type_index type_idx, const Aspect* aspect)
 {
-  if (aspect) {
-    mAspectMap[type_idx] = aspect->cloneAspect();
-    addToComposite(mAspectMap[type_idx].get());
-  } else {
-    mAspectMap[type_idx] = nullptr;
-  }
+  auto insertion = mAspectMap.insert(AspectMap::value_type(type_idx, nullptr));
+  auto& slot = insertion.first->second;
+  _replaceAspect(slot, aspect ? aspect->cloneAspect() : nullptr);
 }
 
 //==============================================================================
 void Composite::_set(std::type_index type_idx, std::unique_ptr<Aspect> aspect)
 {
-  mAspectMap[type_idx] = std::move(aspect);
-  addToComposite(mAspectMap[type_idx].get());
+  auto insertion = mAspectMap.insert(AspectMap::value_type(type_idx, nullptr));
+  auto& slot = insertion.first->second;
+  _replaceAspect(slot, std::move(aspect));
 }
 
 } // namespace common

dart/common/Composite.hpp

@@ -173,6 +173,13 @@ class DART_API Composite
   /// to call the protected Aspect::loseComposite function.
   void removeFromComposite(Aspect* aspect);
 
+  /// Replace the Aspect stored in a given slot.
+  ///
+  /// This ensures that the old Aspect is notified that it is losing its
+  /// Composite and that the new Aspect is notified of its new Composite.
+  void _replaceAspect(
+      std::unique_ptr<Aspect>& slot, std::unique_ptr<Aspect> replacement);
+
   /// Non-templated version of set(const T*)
   void _set(std::type_index type_idx, const Aspect* aspect);
 

dart/common/detail/Composite.hpp

@@ -93,11 +93,14 @@ void Composite::set(std::unique_ptr<T>&& aspect)
 template <class T, typename... Args>
 T* Composite::createAspect(Args&&... args)
 {
-  T* aspect = new T(std::forward<Args>(args)...);
-  mAspectMap[typeid(T)] = std::unique_ptr<T>(aspect);
-  addToComposite(aspect);
+  auto insertion = mAspectMap.insert(AspectMap::value_type(typeid(T), nullptr));
+  auto& slot = insertion.first->second;
 
-  return aspect;
+  auto aspect = std::make_unique<T>(std::forward<Args>(args)...);
+  T* raw = aspect.get();
+  _replaceAspect(slot, std::move(aspect));
+
+  return raw;
 }
 
 //==============================================================================
@@ -106,10 +109,8 @@ void Composite::removeAspect()
 {
   AspectMap::iterator it = mAspectMap.find(typeid(T));
   DART_COMMON_CHECK_ILLEGAL_ASPECT_ERASE(removeAspect, T, DART_BLANK)
-  if (mAspectMap.end() != it) {
-    removeFromComposite(it->second.get());
-    it->second = nullptr;
-  }
+  if (mAspectMap.end() != it)
+    _replaceAspect(it->second, nullptr);
 }
 
 //==============================================================================

dart/common/detail/CompositeData.hpp

@@ -154,15 +154,14 @@ class CompositeData : public CloneableMap<MapType>
     using Data = typename GetData<AspectT>::Type;
     using AspectType = typename GetAspect<AspectT>::Type;
 
-    auto& it = this->mMap.insert(
-        std::make_pair<std::type_index, std::unique_ptr<Data>>(
-            typeid(AspectType), nullptr));
+    auto insertion = this->mMap.insert(
+        typename MapType::value_type(typeid(AspectType), nullptr));
+    auto& it = insertion.first;
 
-    const bool exists = !it.second;
-    if (!exists)
-      it.first = std::make_unique<Data>(std::forward<Args>(args)...);
+    if (insertion.second || !it->second)
+      it->second = std::make_unique<Data>(std::forward<Args>(args)...);
 
-    return static_cast<Data&>(*it.first);
+    return static_cast<Data&>(*it->second);
   }
 
   template <class AspectT>

dart/common/detail/SpecializedForAspect.hpp

@@ -206,12 +206,8 @@ void SpecializedForAspect<SpecAspect>::_set(
   usedSpecializedAspectAccess = true;
 #endif // DART_UNITTEST_SPECIALIZED_ASPECT_ACCESS
 
-  if (aspect) {
-    mSpecAspectIterator->second = aspect->cloneAspect();
-    addToComposite(mSpecAspectIterator->second.get());
-  } else {
-    mSpecAspectIterator->second = nullptr;
-  }
+  _replaceAspect(
+      mSpecAspectIterator->second, aspect ? aspect->cloneAspect() : nullptr);
 }
 
 //==============================================================================
@@ -232,8 +228,7 @@ void SpecializedForAspect<SpecAspect>::_set(
   usedSpecializedAspectAccess = true;
 #endif // DART_UNITTEST_SPECIALIZED_ASPECT_ACCESS
 
-  mSpecAspectIterator->second = std::move(aspect);
-  addToComposite(mSpecAspectIterator->second.get());
+  _replaceAspect(mSpecAspectIterator->second, std::move(aspect));
 }
 
 //==============================================================================
@@ -254,11 +249,11 @@ SpecAspect* SpecializedForAspect<SpecAspect>::_createAspect(
   usedSpecializedAspectAccess = true;
 #endif // DART_UNITTEST_SPECIALIZED_ASPECT_ACCESS
 
-  SpecAspect* aspect = new SpecAspect(std::forward<Args>(args)...);
-  mSpecAspectIterator->second = std::unique_ptr<SpecAspect>(aspect);
-  addToComposite(aspect);
+  auto aspect = std::make_unique<SpecAspect>(std::forward<Args>(args)...);
+  SpecAspect* raw = aspect.get();
+  _replaceAspect(mSpecAspectIterator->second, std::move(aspect));
 
-  return aspect;
+  return raw;
 }
 
 //==============================================================================
@@ -279,8 +274,7 @@ void SpecializedForAspect<SpecAspect>::_removeAspect(type<SpecAspect>)
 
   DART_COMMON_CHECK_ILLEGAL_ASPECT_ERASE(erase, SpecAspect, DART_BLANK);
 
-  removeFromComposite(mSpecAspectIterator->second.get());
-  mSpecAspectIterator->second = nullptr;
+  _replaceAspect(mSpecAspectIterator->second, nullptr);
 }
 
 //==============================================================================

docs/onboarding/aspect-system.md

@@ -120,7 +120,7 @@ This is used for copy/clone/serialization-style workflows:
 
 ### Typed bundles (`Composite::MakeState` / `Composite::MakeProperties`)
 
-When you *know at compile time* which Aspects you care about, you can use the typed bundling helpers:
+When you _know at compile time_ which Aspects you care about, you can use the typed bundling helpers:
 
 - `Composite::MakeState<Aspects...>`
 - `Composite::MakeProperties<Aspects...>`