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

can timing of postinitialize be postponed? #59

Open
ben-wes opened this issue Oct 3, 2024 · 6 comments
Open

can timing of postinitialize be postponed? #59

ben-wes opened this issue Oct 3, 2024 · 6 comments
Assignees

Comments

@ben-wes
Copy link
Contributor

ben-wes commented Oct 3, 2024

there was a short discussion in #57 about the timing of postinitialize(), painting and get_args():

... obviously, we can immediately close this issue here again if it doesn't make sense or makes things too complicated - but i'll repeat the question once more since it was off-topic in the PR:

i'm wondering if it might be an option to delay postinitialize() [...]. would this possibly allow to read the arguments via get_args() then?

@agraef
Copy link
Owner

agraef commented Oct 3, 2024

This is not a bad idea at all, as it would arguably make some kinds of initialization much easier. The problem is that postinitialize needs to run before any other message passing takes place, and that can only be done reliably during object instantiation when the object hasn't been fully created on the Pd side yet. So we now run this callback pretty much at the latest possible time, when the object is already instantiated on the Lua side (which is the sole reason for postinitialize to exist, AFAICT), but not on the Pd side.

That's my understanding, anyway. :) @claudeha may know better. Claude?

@agraef agraef self-assigned this Oct 3, 2024
@agraef
Copy link
Owner

agraef commented Oct 3, 2024

i'm wondering if it might be an option to delay postinitialize() [...]. would this possibly allow to read the arguments via get_args() then?

This specific problem might be somewhat easier to tackle. We might be able to provide a kind of "shadow binbuf" with parsed arguments during the initialization phase when the object's proper binbuf doesn't exist yet.

@claudeha
Copy link
Contributor

claudeha commented Oct 3, 2024

I have no memory of this (long time ago) but I found this in my repository history, seems before postinitialize was added objects had to manage their own postinitialization:

$ git show 42139ebd6e98f965dbde5ceec274f94d17afafcd
commit 42139ebd6e98f965dbde5ceec274f94d17afafcd
Author: Claude Heiland-Allen <claudiusmaximus@goto10.org>
Date:   Sun Dec 2 23:38:41 2007 +0000

    pdlua examples simplified thanks to postinitialize()
    
    git-svn-id: https://code.goto10.org/svn/maximus/pdlua@488 3b2f0ab7-5e1d-0410-9a92-d32b2e5d6f3a

diff --git a/examples/ldelay.lua b/examples/ldelay.lua
index b3a9507..4e310e3 100644
--- a/examples/ldelay.lua
+++ b/examples/ldelay.lua
@@ -13,11 +13,12 @@ function LDelay:initialize(name, atoms)
   return true
 end
 
+function LDelay:postinitialize()
+  self.clock = pd.Clock:new():register(self, "trigger")
+end
+
 function LDelay:finalize()
-  if nil ~= self.clock then
-    self.clock:destruct()
-    self.clock = nil
-  end
+  self.clock:destruct()
 end
 
 function LDelay:in_2_float(f)
@@ -30,16 +31,11 @@ function LDelay:in_1_float(f)
 end
 
 function LDelay:in_1_bang()
-  if nil == self.clock then
-    self.clock = pd.Clock:new():register(self, "trigger")
-  end
   self.clock:delay(self.delay)
 end
 
 function LDelay:in_1_stop()
-  if nil ~= self.clock then
-    self.clock:unset()
-  end
+  self.clock:unset()
 end  
 
 function LDelay:trigger()
diff --git a/examples/ldelay2.lua b/examples/ldelay2.lua
index cd69633..d6d681f 100644
--- a/examples/ldelay2.lua
+++ b/examples/ldelay2.lua
@@ -13,15 +13,14 @@ function LDelay:initialize(name, atoms)
   return true
 end
 
+function LDelay:postinitialize()
+  self.clock1 = pd.Clock:new():register(self, "trigger1")
+  self.clock2 = pd.Clock:new():register(self, "trigger2")
+end
+
 function LDelay:finalize()
-  if nil ~= self.clock1 then
-    self.clock1:destruct()
-    self.clock1 = nil
-  end
-  if nil ~= self.clock2 then
-    self.clock2:destruct()
-    self.clock2 = nil
-  end
+  self.clock1:destruct()
+  self.clock2:destruct()
 end
 
 function LDelay:in_2_float(f)
@@ -34,23 +33,13 @@ function LDelay:in_1_float(f)
 end
 
 function LDelay:in_1_bang()
-  if nil == self.clock1 then
-    self.clock1 = pd.Clock:new():register(self, "trigger1")
-  end
-  if nil == self.clock2 then
-    self.clock2 = pd.Clock:new():register(self, "trigger2")
-  end
   self.clock1:delay(self.delay)
   self.clock2:delay(self.delay * 2)
 end
 
 function LDelay:in_1_stop()
-  if nil ~= self.clock1 then
-    self.clock1:unset()
-  end
-  if nil ~= self.clock2 then
-    self.clock2:unset()
-  end
+  self.clock1:unset()
+  self.clock2:unset()
 end  
 
 function LDelay:trigger1()

git blame tells me I originally added it here

commit 2170da3e0f495e17c0b621ba05eba0b936f3bf41
Author: Claude Heiland-Allen <claudiusmaximus@goto10.org>
Date:   Sun Dec 2 19:10:09 2007 +0000

    pdlua receive support
    
    git-svn-id: https://code.goto10.org/svn/maximus/pdlua@485 3b2f0ab7-5e1d-0410-9a92-d32b2e5d6f3a

diff --git a/TODO b/TODO
index 2f197a9..26eba65 100644
--- a/TODO
+++ b/TODO
@@ -1,9 +1,9 @@
 todo
 - send support
-- receive support (preferably multiple receives)
-- add hook for object (post creation)
 
 done
+- add hook for object (post creation)
+- receive support (preferably multiple receives)
 - clock support
 - audit possible bugs relating to self.f = f(self, ...) misuse
 - write docs on foo.lua vs foo.luax
diff --git a/doc/internal.txt b/doc/internal.txt
index 64e62f5..9702a0c 100644
--- a/doc/internal.txt
+++ b/doc/internal.txt
@@ -58,7 +58,5 @@ way Pd uses them.
 Architecture Issues
 -------------------
 
-:initialize() is called before the object is created.  There needs to
-be a hook for "after object creation", too, or maybe rethink the way
-pdlua works.
-
+:initialize() is called before the object is created.
+:postinitialize() is called after the object is created.

(plus some long code changes I don't include here that are not so interesting)

so it seems to me now that the rationale for postinitialize is to do pd-dependent things (like binding to receive names, create clocks, etc) at creation time - after the pd object is created.

I don't know what the current problem is, I haven't used Pd or PdLua for quite a while...

@agraef
Copy link
Owner

agraef commented Oct 3, 2024

Hi Claude, thanks for chiming in!

so it seems to me now that the rationale for postinitialize is to do pd-dependent things (like binding to receive names, create clocks, etc) at creation time - after the pd object is created.

Just to make sure that I'm not talking nonsense, I've just looked at the git blames, and postinitialize has indeed been in its present position for a long time. Our git history only goes back as far as Jul 21, 2014 which is when IOhannes imported your 0.6.0 version, but I'd imagine that it's still where you added it all the way back in 2007. That's a testament on how good your original design was -- it's still basically the same today, although we added a few interesting new bits and bobs here and there. :)

It's true that at this point the t_pdlua object has already been created via pd_new(), this happens in pdlua_object_new() on the C side which gets called from the Lua object constructor pd.Class:construct() which then goes on to call initialize() and then, if that returned true, postinitialize(). So the C object pointer already exists and thus many things will work which require that object pointer, like setting up timers and receive symbols. But the Lua constructor in turn gets called by the real Pd object constructor which is pdlua_new() on the C side. This then goes on to check the return value of pd.Class:construct() -- which is the created Lua object if initialize() returned true, and the Lua nil value otherwise. It has to be that way, since the Pd object has to know whether the Lua object creation succeeded before it returns the t_pdlua object to Pd to tell it that the object was created successfully -- or NULL if it wasn't.

(I've simplified this a little, because pdlua_new() actually goes through an extra dispatch level which then calls pd.Class:construct(), but that's just a minor technical detail which has no impact on the problem at hand.)

Now the catch here is that the object's binbuf is only initialized after the object was instantiated with pdlua_new() on the C side. I think that Pd itself does this after instantiating the object in order to record the object's class name and creation arguments. That's why operations relying on the existence of the binbuf, specifically the new get_args and set_args methods do not work in postinitialize. And there's simply no way that we could call postinitialize at a later time, but before message processing starts because message processing starts immediately after pdlua_new() returns.

That's what the talk about setting up a shadow binbuf is about. This would be added just to make those two methods work in the postinitialize method.

Note to self: While looking at pdlua_new() again, I just realized that we may be leaking memory on the object pointer created by pdlua_object_new() if initialize() and consequently pd.Class:construct() fails. At least I don't see that we free that pointer again in this case.

@claudeha
Copy link
Contributor

claudeha commented Oct 5, 2024

thanks for the clear explanation! if the new get_args/set_args are part of the Pd API (I haven't had time to check), then presumably they can't be used at construction time in C externals either - so maybe it'd be better to fix this in Pd if possible, so externals in all languages can benefit?

@agraef
Copy link
Owner

agraef commented Oct 5, 2024

if the new get_args/set_args are part of the Pd API [...]

They aren't. They are just part of the Pd-Lua API. (Tim introduced this with his Pd-Lua graphics API so that he could store state in the creation arguments -- mainly for graphics objects, but this also works with ordinary Lua objects.)

I doubt that anyone at the vanilla project would be interested in adopting this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants