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

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

Merged
merged 2 commits into from
Sep 21, 2024

Conversation

ben-wes
Copy link
Contributor

@ben-wes ben-wes commented Sep 18, 2024

creating this PR here mainly to summarize the result of #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:

  • enable pdlua externals to have their senders and receivers set through messages, similar to iemguis and other externals.

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 48f0053
  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 53f0342

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
Copy link
Owner

agraef commented Sep 21, 2024

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 agraef merged commit 2a6dda2 into agraef:master Sep 21, 2024
10 checks passed
@agraef
Copy link
Owner

agraef commented Sep 21, 2024

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
Copy link
Owner

agraef commented Sep 21, 2024

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.

@agraef agraef mentioned this pull request Sep 21, 2024
@ben-wes
Copy link
Contributor Author

ben-wes commented Sep 21, 2024

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
Copy link
Owner

agraef commented Sep 21, 2024

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. :)

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

Successfully merging this pull request may close these issues.

2 participants