From 7b1495dd450b0691f47c5074f996e23a6755d2b9 Mon Sep 17 00:00:00 2001
From: Michael Reed <mike@reedtribe.org>
Date: Tue, 3 May 2022 16:30:13 -0400
Subject: [PATCH] Make isInstance virtual, remove variable

---
 include/rive/artboard.hpp | 18 ++++++++++++-----
 src/artboard.cpp          | 42 ++++++++++++++++++++++++---------------
 2 files changed, 39 insertions(+), 21 deletions(-)

diff --git a/include/rive/artboard.hpp b/include/rive/artboard.hpp
index 3dcf69cb..04aea9b1 100644
--- a/include/rive/artboard.hpp
+++ b/include/rive/artboard.hpp
@@ -31,10 +31,16 @@ namespace rive {
         friend class ArtboardImporter;
         friend class Component;
 
-    private:
-        std::vector<Core*> m_Objects;
+    protected:
+        void deleteObjects();   // call from destructors
+    
+        // these are owned if we are not an instance, but are not owned
+        // if we are an instance.
         std::vector<LinearAnimation*> m_Animations;
         std::vector<StateMachine*> m_StateMachines;
+
+    private:
+        std::vector<Core*> m_Objects;
         std::vector<Component*> m_DependencyOrder;
         std::vector<Drawable*> m_Drawables;
         std::vector<DrawTarget*> m_DrawTargets;
@@ -45,7 +51,6 @@ namespace rive {
         std::unique_ptr<CommandPath> m_ClipPath;
         Factory* m_Factory = nullptr;
         Drawable* m_FirstDrawable = nullptr;
-        bool m_IsInstance = false;
         bool m_FrameOrigin = true;
 
         std::queue<Message> m_MessageQueue;
@@ -68,7 +73,7 @@ namespace rive {
 
     public:
         Artboard() {}
-        ~Artboard();
+        ~Artboard() override;
         StatusCode initialize();
 
         Core* resolve(uint32_t id) const override;
@@ -151,7 +156,7 @@ namespace rive {
         std::unique_ptr<ArtboardInstance> instance() const;
 
         /// Returns true if the artboard is an instance of another
-        bool isInstance() const { return m_IsInstance; }
+        virtual bool isInstance() const { return false; }
 
         /// Returns true when the artboard will shift the origin from the top
         /// left to the relative width/height of the artboard itself. This is
@@ -171,6 +176,9 @@ namespace rive {
     class ArtboardInstance : public Artboard {
     public:
         ArtboardInstance() {}
+        ~ArtboardInstance() override;
+
+        bool isInstance() const override { return true; }
 
         std::unique_ptr<LinearAnimationInstance> animationAt(size_t index);
         std::unique_ptr<LinearAnimationInstance> animationNamed(std::string name);
diff --git a/src/artboard.cpp b/src/artboard.cpp
index 91437b0a..7e64acbc 100644
--- a/src/artboard.cpp
+++ b/src/artboard.cpp
@@ -22,6 +22,18 @@
 using namespace rive;
 
 Artboard::~Artboard() {
+    this->deleteObjects();
+
+    // If we were an instance, these arrays will already have been cleared.
+    for (auto object : m_Animations) {
+        delete object;
+    }
+    for (auto object : m_StateMachines) {
+        delete object;
+    }
+}
+
+void Artboard::deleteObjects() {
     for (auto object : m_Objects) {
         // First object is artboard
         if (object == this) {
@@ -29,19 +41,7 @@ Artboard::~Artboard() {
         }
         delete object;
     }
-
-    // Instances reference back to the original artboard's animations and state
-    // machines, so don't delete them here, they'll get cleaned up when the
-    // source is deleted.
-    // TODO: move this logic into ArtboardInstance destructor???
-    if (!m_IsInstance) {
-        for (auto object : m_Animations) {
-            delete object;
-        }
-        for (auto object : m_StateMachines) {
-            delete object;
-        }
-    }
+    m_Objects.clear();
 }
 
 static bool canContinue(StatusCode code) {
@@ -536,6 +536,8 @@ std::unique_ptr<ArtboardInstance> Artboard::instance() const {
         }
     }
 
+    // note: we just push bare pointers here. The instance is not an owner
+    // of these objeccts (we are).
     for (auto animation : m_Animations) {
         artboardClone->m_Animations.push_back(animation);
     }
@@ -545,11 +547,8 @@ std::unique_ptr<ArtboardInstance> Artboard::instance() const {
 
     if (artboardClone->initialize() != StatusCode::Ok) {
         artboardClone = nullptr;
-    } else {
-        artboardClone->m_IsInstance = true;
     }
 
-    assert(artboardClone->isInstance());
     return artboardClone;
 }
 
@@ -595,6 +594,17 @@ bool Artboard::nextMessage(Message* msg) {
 #include "rive/animation/linear_animation_instance.hpp"
 #include "rive/animation/state_machine_instance.hpp"
 
+ArtboardInstance::~ArtboardInstance() {
+    this->deleteObjects();
+
+    // Instances reference back to the original artboard's animations and state
+    // machines, so don't delete them here, they'll get cleaned up when the
+    // source is deleted. Thus, so that our baseclass Artboard doesn't delete
+    // them, we clear the arrays now (w/o deleting the elements)
+    m_Animations.clear();
+    m_StateMachines.clear();
+}
+
 std::unique_ptr<LinearAnimationInstance> ArtboardInstance::animationAt(size_t index) {
     auto la = this->animation(index);
     return la ? std::make_unique<LinearAnimationInstance>(la, this) : nullptr;