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

[Proposal] overload definitions in the patch explorer #41

Open
sebllll opened this issue Jun 5, 2021 · 2 comments
Open

[Proposal] overload definitions in the patch explorer #41

sebllll opened this issue Jun 5, 2021 · 2 comments
Labels

Comments

@sebllll
Copy link

sebllll commented Jun 5, 2021

Ok, i want to have overloads #40 .
I'm assuming it's not that much a technical issues as overloads from imported libs are working and so hopefully patched ones can go the same route.

The proposal is to have some kind of standardized* annotation , that can define overloads in the patch explorers Operations section.

as a (bad) example, let's assume, the annotation syntax is an underscore followed by some number, the Operations panel could look like this:
image
The add_1 and add_2 Operations would then be treated as overloads: there's one single add entry in the node-browser and their 2 signatures could be chosen the usual way.

The cool thing is, that this could also work with the Create operation, where i miss this feature most.

  • the ideal annotation syntax should be chosen wisely :)
@gregsn
Copy link
Member

gregsn commented Jun 22, 2021

Regarding the Create overloads:

I can imagine that this gets tricky soon. Create is special in different regards

  • Implicitly all Create fragments of all process nodes end up getting called at this special moment
  • The way the This-Node works together with those process node initializations (their Create fragments)
    probably way more.

Regarding the first bullet point:

  • Imagine you have two Create moments/operations.
  • Imagine some of the process nodes have actual Create fragments with white pins that you actually want to feed with data.

We now would need to have an exception that allows connecting this pin from different Create operations.
Currently, I think we get graph sharing warnings, which are just warnings that tell the user that nodes or node fragments end up getting executed at different moments. So we might be halfway through. But this graph-sharing idea/feature got hidden behind a warning, because it is hard to read such a patch. And yes, still we don't allow connecting a pin twice. So we'd need to allow this and come up with new moment inference ideas...

I can imagine that there are more issues with the idea, since in our compiler Create, Update and Dispose are just special in order to get some ease of use or some magically right behavior here and there. Getting rid of the idea that Create is a single special moment, but can be many special moments sounds like something for the long-term wish list. We'd need to collect all the reasons why and how Create is special before allowing more than one constructor.

Up to that point please tell us if proposal #43 works for you and if not why not.

Regarding overloads for other not-so-special cases: We should have a discussion that compares

  • nodes that have the same name, but a different signature
  • nodes that have different versions

When starting to design VL, we were copying the idea of nodes having versions from vvvv beta as we were happy with that kind of distinguishment. Version-names allow the node designer to describe the purpose of that overload with a snappy version-name. Support for real overloads was actually only added in order to reference external nodes from any .Net assembly. We allowed to use the pin-choice overload-selecting node references also for normal VL nodes, but we're not yet super happy with this kind of system. So up to the point where we have a solution that all of the core team are happy with, I guess, we are still more in favor of versions or completely different node names for different operations. I understand that this is not your real issue. Your real issue is that you want to have a second constructor and the naming currently is the only way of expressing what a constructor is. But again this might get trickier than expected. So we are eager to hear if there are workarounds that allow you to proceed with whatever you actually wanted to do.

@sebllll
Copy link
Author

sebllll commented Jun 23, 2021

i agree to all of that and yes, #43 is a good workaround

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

No branches or pull requests

2 participants