search

agraef / pd-lua

Lua bindings for Pd, updated for Lua 5.3+
https://agraef.github.io/pd-lua/
GNU General Public License v2.0
51 stars 11 forks source link

expose Pd's canvas_realizedollar() and modify set_args() for dollar arguments #54

Closed ben-wes closed 1 month ago

ben-wes commented 1 month ago

creating this PR here mainly to summarize the result of https://github.com/agraef/pd-lua/issues/52, which escalated a bit. these changes certainly require review, and it's obviously also no problem if this PR is declined or needs further modifications.

the primary goal was:

changes:

  1. writing the necessary \$0-name string to the pd file isn't possible with set_args() as it also escapes the backslash to \\\$0-name. this is addressed with 48f00537b04ef2100e10c7e52aaec877b4b32d0d
  2. for the pdlua object to apply the given receiver name immediately, an API method to expand it is required. this functionality is implemented with 53f0342b1db016c5e1112e10b63532c4f5e7aefb

findings that may require further consideration:

  1. when a dollar argument can't be expanded, it remains a dollar argument (in lua, it will just be a "$1" string then, for example)
  2. sending \$01 from pd in that case will also result in "$1" on the lua side however

i've tested these changes in my setup without encountering issues so far. to facilitate testing elsewhere, i'm attaching these files (pdlua object and patch): test_dollar.zip

agraef commented 1 month ago

Looks good to me. You asked for comments from @timothyschoen in #52 and he didn't object, so I'd say that this can go in now.

agraef commented 1 month ago

Ben, maybe you can follow this up with some proper documentation of how set_args() works in doc/graphics.txt?

Or, better yet, add a more detailed example than gui-example to pdlua/examples? With some explanations of set_args() in the patch?

And add an example for pd.Class:canvas_realizedollar()?

agraef commented 1 month ago

Just tested it with your provided test patch in vanilla, works perfectly AFAICT. So I'll just go ahead and release this as 0.12.17.

ben-wes commented 1 month ago

Looks good to me. You asked for comments from @timothyschoen in #52 and he didn't object, so I'd say that this can go in now.

cool, thanks a lot for merging and all the comments in #52 and here! i assume that the principle functionality is as expected then - so even if there might be objections about implementation details at some point, it's hopefully not critical!

[...] maybe you can follow this up [...]

sure, i'm on it! will create a PR for that later today.

... and great to see all those updates to purr data btw!

agraef commented 1 month ago

If I had any objections about the implementation, I'd have voiced them here. No, you implemented everything exactly as I would have done it myself.

sure, i'm on it! will create a PR for that later today.

Great, looking forward to it!

... and great to see all those updates to purr data btw!

Oh boy, that was a lot of work this time, but I think that I fixed every single bug and usability issue that my students and I came across during the last 6 semesters. Almost 200 commits went into this release, and except for maybe a dozen commits from the GSoC 2024 contributions, I did all those myself since the end of June. I'm tired now. :)