From 3b60b3b0ecf10be4c3874143c22b4d70c0ad1c08 Mon Sep 17 00:00:00 2001
From: maelstrom <maelstrom@maelstrom.dev>
Date: Fri, 16 May 2025 09:43:15 +0200
Subject: [PATCH] feat(signal): added signal holder to automatically disconnect
 signals + instance AncestryChanged

---
 core/src/datatypes/signal.cpp      | 15 +++++++++++++++
 core/src/datatypes/signal.h        | 21 +++++++++++++++++++++
 core/src/objects/base/instance.cpp |  1 +
 core/src/objects/base/instance.h   |  4 ++++
 4 files changed, 41 insertions(+)

diff --git a/core/src/datatypes/signal.cpp b/core/src/datatypes/signal.cpp
index 4389b86..8fea91e 100644
--- a/core/src/datatypes/signal.cpp
+++ b/core/src/datatypes/signal.cpp
@@ -72,6 +72,19 @@ void CSignalConnection::Call(std::vector<Data::Variant> args) {
 
 //
 
+SignalConnectionHolder::SignalConnectionHolder() : heldConnection() {}
+SignalConnectionHolder::SignalConnectionHolder(std::shared_ptr<SignalConnection> connection) : heldConnection(connection) {}
+SignalConnectionHolder::SignalConnectionHolder(Data::SignalConnectionRef other) : heldConnection(other) {}
+
+SignalConnectionHolder::~SignalConnectionHolder() {
+    // printf("Prediscon!\n");
+    // if (!heldConnection.expired()) printf("Disconnected!\n");
+    if (!heldConnection.expired())
+        heldConnection.lock()->Disconnect();
+}
+
+//
+
 SignalConnectionRef Signal::Connect(std::function<void(std::vector<Data::Variant>)> callback) {
     auto conn = std::dynamic_pointer_cast<SignalConnection>(std::make_shared<CSignalConnection>(callback, weak_from_this()));
     connections.push_back(conn);
@@ -96,6 +109,8 @@ SignalConnectionRef Signal::Once(lua_State* state) {
     return SignalConnectionRef(conn);
 }
 
+//
+
 int __waitingThreads = 0;
 int Signal::Wait(lua_State* thread) {
     // If the table hasn't been constructed yet, make it
diff --git a/core/src/datatypes/signal.h b/core/src/datatypes/signal.h
index 831d39b..9d2e096 100644
--- a/core/src/datatypes/signal.h
+++ b/core/src/datatypes/signal.h
@@ -56,6 +56,27 @@ public:
     ~LuaSignalConnection();
 };
 
+// Holds a signal connection such that when the holder is deleted (either via its parent object being deleted, or being overwritten),
+// the connection is disconnected. Useful to prevent lingering connections that no longer contain valid objects
+class SignalConnectionHolder {
+    std::weak_ptr<SignalConnection> heldConnection;
+public:
+    SignalConnectionHolder();
+    SignalConnectionHolder(std::shared_ptr<SignalConnection>);
+    SignalConnectionHolder(Data::SignalConnectionRef other);
+    ~SignalConnectionHolder();
+
+    // Prevent SignalConnectionHolder being accidentally copied, making it useless
+    // https://stackoverflow.com/a/10473009/16255372
+    SignalConnectionHolder(const SignalConnectionHolder&) = delete;
+    SignalConnectionHolder& operator=(const SignalConnectionHolder&) = delete;
+    SignalConnectionHolder(SignalConnectionHolder&&) = default;
+    SignalConnectionHolder& operator=(SignalConnectionHolder&&) = default;
+
+    inline bool Connected() { return !heldConnection.expired() && heldConnection.lock()->Connected(); }
+    inline void Disconnect() { if (!heldConnection.expired()) heldConnection.lock()->Disconnect(); }
+};
+
 class Signal : public std::enable_shared_from_this<Signal> {
     std::vector<std::shared_ptr<SignalConnection>> connections;
     std::vector<std::shared_ptr<SignalConnection>> onceConnections;
diff --git a/core/src/objects/base/instance.cpp b/core/src/objects/base/instance.cpp
index e7607cd..871c583 100644
--- a/core/src/objects/base/instance.cpp
+++ b/core/src/objects/base/instance.cpp
@@ -96,6 +96,7 @@ void Instance::updateAncestry(std::optional<std::shared_ptr<Instance>> updatedCh
     }
 
     OnAncestryChanged(updatedChild, newParent);
+    AncestryChanged->Fire({updatedChild.has_value() ? Data::InstanceRef(updatedChild.value()) : Data::InstanceRef(), newParent.has_value() ? Data::InstanceRef(newParent.value()) : Data::InstanceRef()});
 
     // Old workspace used to exist, and workspaces differ
     if (!oldWorkspace.expired() && oldWorkspace != _workspace) {
diff --git a/core/src/objects/base/instance.h b/core/src/objects/base/instance.h
index 69d4c43..43f0c96 100644
--- a/core/src/objects/base/instance.h
+++ b/core/src/objects/base/instance.h
@@ -11,6 +11,7 @@
 #include <string>
 #include <vector>
 
+#include "datatypes/signal.h"
 #include "error/instance.h"
 #include "error/result.h"
 #include "member.h"
@@ -93,6 +94,9 @@ public:
     const static InstanceType TYPE;
     std::string name;
 
+    // Signals
+    SignalSource AncestryChanged;
+
     // Instance is abstract, so it should not implement GetClass directly
     virtual const InstanceType* GetClass() = 0;
     bool SetParent(std::optional<std::shared_ptr<Instance>> newParent);