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

add an inlet to [receive] #604

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

add an inlet to [receive] #604

wants to merge 4 commits into from

Conversation

porres
Copy link
Contributor

@porres porres commented Apr 30, 2019

this makes [receive] behave just like [send] - when you load it without arguments, it creates an inlet that you can use to set a receive name by giving it a symbol message.

closes #603

requires #1760 to be safe.

this makes it behave similarly to [send], when you load it without arguments, it creates an inlet that you can use to set a receive name by giving it a symbol.

closes #603
@porres porres changed the title adds an inlet to [receive] add an inlet to [receive] Apr 30, 2019
@porres
Copy link
Contributor Author

porres commented Apr 30, 2019

this is particularly useful to create GUI abstractions that behave like other GUIs, where you can give it a 'receive' name via an input message from the inlet

Copy link
Contributor

@Spacechild1 Spacechild1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this has been on my list for ages, so I would really like to see this happening! I've suggested a couple of minor changes to make the code cleaner.

src/x_connective.c Outdated Show resolved Hide resolved
src/x_connective.c Outdated Show resolved Hide resolved
src/x_connective.c Outdated Show resolved Hide resolved
src/x_connective.c Outdated Show resolved Hide resolved
src/x_connective.c Show resolved Hide resolved
src/x_connective.c Outdated Show resolved Hide resolved
doc/5.reference/receive-help.pd Outdated Show resolved Hide resolved
@porres
Copy link
Contributor Author

porres commented Apr 30, 2019

hi, can you just send me the PR? I don't know how this works, if you have proposed the changes already, or if I have to do them myself...

I assumed my code wouldn't be great, as I was actually copying from some other object in cyclone. I don't really know how to do a proxy object, this is my first one ever, so I just copied. I can't answer why I used one thing or another, I'm just glad I tested it and it works. I'm also happy that I know I had to do a proxy for this to work :)

Anyway, I was assuming it could be clumsy and that people could revise and make it cleaner/better, thanks for that

@porres
Copy link
Contributor Author

porres commented Apr 30, 2019

this has been on my list for ages, so I would really like to see this happening!

Awesome, I assume many will be happy with this one :) - glad I can help

@Spacechild1
Copy link
Contributor

Spacechild1 commented Apr 30, 2019

I don't know how this works, if you have proposed the changes already, or if I have to do them myself...

usually you would apply the suggested changes (if you agree with them). the review is really just comments, no PR.

@umlaeute
Copy link
Contributor

iirc there were arguments by @millerpuckette why this is not such a good idea.
mostly because you could change the receiver-label while receiving, and get a segmentation fault.

this should be in the list-archives somewhere

@Spacechild1
Copy link
Contributor

Spacechild1 commented Apr 30, 2019

thanks for the hint! I had a look and discovered that this is also true for the right inlet of [value]. the PR above (#607) attempts to solve this issue.

@Spacechild1
Copy link
Contributor

Spacechild1 commented Apr 30, 2019

@porres porres#1

Code has been cleaned up with the help of @SpaceChild and the help file has been fixed
@porres
Copy link
Contributor Author

porres commented Apr 30, 2019

@porres porres#1

I cherry picked and made a new commit to cleanup the code 0a2a5d4 - this commit also fixes the documentation

@umlaeute umlaeute added feature suggestion for an enhancement and removed feature suggestion for an enhancement labels Jun 18, 2019
@millerpuckette
Copy link
Contributor

This necessitates a complicated change to symbol binding/unbinding that I fear will never be completely debugged.

An alternative idea might be to make a multiple-symbol receive with an inlet to select among a (pre-determined) list of symbols by name or perhaps index.

@porres
Copy link
Contributor Author

porres commented Dec 7, 2019

An alternative idea might be to make a multiple-symbol receive with an inlet to select among a (pre-determined) list of symbols by name or perhaps index.

Not sure I get, and my actual use case here is the need to set a [receive] object with a name that is not known or pre-determined.

@porres
Copy link
Contributor Author

porres commented Dec 7, 2019

my actual use case here is the need to set a [receive] object with a name that is not known or pre-determined.

Let me give more details of this particular use case. Say you have a GUI abstraction and you want the user to be able to give it "send" and "receive" symbol names, like you can do with sliders and stuff. For [send] this is trivial, but the issue is if you want to set a receive name. And this name is not pre-determined.

For illustration, I have here a two dimensional slider from my ELSE library that's doing this kind of thing. It's an abstraction based on Data Structures.
Screen Shot 2019-12-07 at 17 30 53

@millerpuckette
Copy link
Contributor

my actual use case here is the need to set a [receive] object with a name that is not known or pre-determined.

Let me give more details of this particular use case. Say you have a GUI abstraction and you want the user to be able to give it "send" and "receive" symbol names, like you can do with sliders and stuff. For [send] this is trivial, but the issue is if you want to set a receive name. And this name is not pre-determined.

[...]

That's a reasonble use case. THe best way to do this given Pd's current state is to make the patch self-modify to make a subpatch with the desired receive and a local send in it. Even though that's ugly it's less ugly than the code needed to allow dynamic symbol unbinding.

@Spacechild1
Copy link
Contributor

but the problem is still that you can't change the receive name because you can't (safely) unbind...

btw, my personal use case is to dynamically select event sources, similar to how you use [receive~] to dynamically select audio sources.

@porres
Copy link
Contributor Author

porres commented Dec 11, 2019

Something like dynamic patching to delete and create a new [receive] object with dynamic patching into a subpatch, right?

that's actually a solution and an external abstraction, purely vanilla, from 'mmb', I believe it's in https://puredata.info/downloads/mmb

I made an external called [else/receiver], which seems to work fine for my abstractions. It's a compiled object and basically the same as this PR and if there were any issues, I thought #614 would take care of it.

But yeah, if a dynamic patching abstraction is what we need, I can think of working on something like that for my library instead, and it would also be a vanilla abstraction and something any one could use for dynamically setting a receive name.

UNLESS......... @Spacechild1 still tries to convice he's got the perfect simple and best solution for this in #614 ...

@porres
Copy link
Contributor Author

porres commented Dec 11, 2019

Hey, once again I did reply before I could see @Spacechild1 had replied before me.

Well, I'm out of this debate as I clearly have no idea what the issues involved are... so I'll let the grown ups do the talking

cheers

@Spacechild1
Copy link
Contributor

Spacechild1 commented Dec 11, 2019

still tries to convice he's got the perfect simple and best solution for this

it certainly isn't perfect, and I'm not even sure it's the best, but it's a proof of concept that it's possible to solve the issue with dynamic unbinding in the core. it is something we should keep investigating.

@porres
Copy link
Contributor Author

porres commented Apr 16, 2020

it certainly isn't perfect, and I'm not even sure it's the best, but it's a proof of concept that it's possible to solve the issue with dynamic unbinding in the core. it is something we should keep investigating.

seems #849 is better?

@porres
Copy link
Contributor Author

porres commented May 14, 2020

this is particularly useful to create GUI abstractions that behave like other GUIs, where you can give it a 'receive' name via an input message from the inlet

Well, it occurs to me now that since IEMGUIs can set a "receive", whatever issues we have with this may already present itself in those cases, right?

@Spacechild1
Copy link
Contributor

yes

@porres
Copy link
Contributor Author

porres commented May 14, 2020

well, as I understand it then, this makes #849 necessary even if no inlet is added to [receive]

@porres
Copy link
Contributor Author

porres commented Apr 9, 2023

Hi, this has issues to be solved but I can't see them or do anything about it. Are you supposed to do something instead @Spacechild1 ?

@Spacechild1
Copy link
Contributor

Seems like I forgot to accept the changes.

You could mention in the PR description that it requires #1760 to be safe.

@porres
Copy link
Contributor Author

porres commented Apr 9, 2023

You could mention in the PR description that it requires #1760 to be safe.

done

@umlaeute
Copy link
Contributor

I think this PR is a no go if applied without #1760.
Therefore i suggest to merge #1760 into this PR (rather than just mentioning it as a prerequisite).

(The only reason for not merging is, that there might be an alternative implementation of the issues fixed by #1760... But I don't see any such alternative implementation)

@porres
Copy link
Contributor Author

porres commented Apr 11, 2023

Therefore i suggest to merge #1760 into this PR (rather than just mentioning it as a prerequisite).

Or the other way around? Get the work here on top of the other? What do you say @Spacechild1 ?

@umlaeute
Copy link
Contributor

Either way.

(Though conceptually it makes more sense to have the PR that depends on another PR to include its dependencies. If you want to keep history logical (you first need the commits of #1760, then the commits of this PR), consider rebasing your PR on top of #1760 and then force push. (I really think you should learn how to do this))

@Spacechild1
Copy link
Contributor

Spacechild1 commented Apr 11, 2023

Therefore i suggest to merge #1760 into this PR (rather than just mentioning it as a prerequisite).

I don't think that's necessary. After all, someone might come up with a better implementation for the bind list problem (which I rather doubt), or I need to do further modifications. IMO it's enough to state #1760 as a prerequisite (which @porres has already done), but I don't really care too much :-)

(I really think you should learn how to do this))

+1 for learning how to rebase! If you have a feature branch where you are the only contributor, it is much better to rebase than to merge the target branch; it avoids needless merge commits and leads to a cleaner history.

Also, take the chance and learn about interactive rebasing which allows to fix individual commits, combine multiple commits, drop individual commits, change commit messages, etc. In particular, this helps to avoid awkward "fix up commits" such as "Oops, fix foo", "Forgot bar", "Try to fix baz again", etc.

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

Successfully merging this pull request may close these issues.

request: inlet for [receive] to set receive symbol
4 participants