Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patch: optimization, no need to re-create Callabels on disconnect. #607

Open
migueldeicaza opened this issue Nov 14, 2024 · 0 comments
Open

Comments

@migueldeicaza
Copy link
Owner

Currently, we recreate the callable on disconnect, which come to think of it, seems like it could go wrong, so this patch makes it so we keep it around and use it to disconnect without recreating the object.

diff --git a/Generator/Generator/ClassGen.swift b/Generator/Generator/ClassGen.swift
index 46bab03..e0e83cf 100644
--- a/Generator/Generator/ClassGen.swift
+++ b/Generator/Generator/ClassGen.swift
@@ -567,12 +567,14 @@ func generateSignalType (_ p: Printer, _ cdef: JGodotExtensionAPIClass, _ signal
         }
         p ("public func connect (flags: Object.ConnectFlags = [], _ callback: @escaping (\(args)) -> ()) -> Object") {
             p ("let signalProxy = SignalProxy()")
+            p ("guard let callable = signalProxy.callable else") {
+                p ("fatalError(\"SignalProxy should have set callable\")")
+            }
             p ("signalProxy.proxy = ") {
                 p ("args in")
                 p (argUnwrap)
                 p ("callback (\(callArgs))")
             }
-            p ("let callable = Callable(object: signalProxy, method: SignalProxy.proxyName)")
             p ("let r = target.connect(signal: signalName, callable: callable, flags: UInt32 (flags.rawValue))")
             p ("if r != .ok { print (\"Warning, error connecting to signal, code: \\(r)\") }")
             p ("return signalProxy")
@@ -580,7 +582,12 @@ func generateSignalType (_ p: Printer, _ cdef: JGodotExtensionAPIClass, _ signal
 
         doc (p, cdef, "Disconnects a signal that was previously connected, the return value from calling ``connect(flags:_:)``")
         p ("public func disconnect (_ token: Object)") {
-            p ("target.disconnect(signal: signalName, callable: Callable (object: token, method: SignalProxy.proxyName))")
+            p ("guard let signalProxy = token as? SignalProxy else { return }")
+            p ("let callable = signalProxy.callable")
+            p ("signalProxy.proxy = nil")
+            p ("if let callable") {
+                p ("target.disconnect(signal: signalName, callable: callable)")
+            }
         }
         doc (p, cdef, "You can await this property to wait for the signal to be emitted once")
         p ("public var emitted: Void "){
diff --git a/Sources/SwiftGodot/Core/SignalSupport.swift b/Sources/SwiftGodot/Core/SignalSupport.swift
index f5a0c77..b06792b 100644
--- a/Sources/SwiftGodot/Core/SignalSupport.swift
+++ b/Sources/SwiftGodot/Core/SignalSupport.swift
@@ -31,10 +31,13 @@ public class SignalProxy: Object {
     /// The code invoked when Godot invokes the `proxy` method on this object.
     public typealias Proxy = (borrowing Arguments) -> ()
     public var proxy: Proxy?
-    
+    var callable: Callable? = nil
+
     public required init () {
         let _ = SignalProxy.initClass
+        callable = nil
         super.init()
+        callable = Callable(object: self, method: SignalProxy.proxyName)
     }
     
     public required init (nativeHandle: UnsafeRawPointer) {
@@ -84,7 +87,7 @@ public class SimpleSignal {
         self.target = target
         self.signalName = signalName
     }
-    
+
     /// Connects the signal to the specified callback.
     ///
     /// To disconnect, call the disconnect method, with the returned token on success
@@ -103,6 +106,9 @@ public class SimpleSignal {
     @discardableResult
     public func connect (flags: Object.ConnectFlags = [], _ callback: @escaping () -> ()) -> Object {
         let signalProxy = SignalProxy()
+        guard let callable = signalProxy.callable else {
+            fatalError("SignalProxy should have set callable")
+        }
         if flags.contains(.oneShot) {
             signalProxy.proxy = { [weak signalProxy] args in
                 callback ()
@@ -115,21 +121,22 @@ public class SimpleSignal {
                 callback ()
             }
         }
-        
-        let callable = Callable(object: signalProxy, method: SignalProxy.proxyName)
         let r = target.connect(signal: signalName, callable: callable, flags: UInt32 (flags.rawValue))
         if r != .ok {
             print ("Warning, error connecting to signal \(signalName.description): \(r)")
         }
         return signalProxy
     }
-    
+
     /// Disconnects a signal that was previously connected, the return value from calling ``connect(flags:_:)``
     public func disconnect (_ token: Object) {
         guard let signalProxy = token as? SignalProxy else { return }
+        let callable = signalProxy.callable
         signalProxy.proxy = nil
         _ = signalProxy.callDeferred(method: "free")
-        target.disconnect(signal: signalName, callable: Callable (object: token, method: SignalProxy.proxyName))
+        if let callable {
+            target.disconnect(signal: signalName, callable: callable)
+        }
     }
     
     /// You can await this property to wait for the signal to be emitted once
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant