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 list of acceptable types when hovering over node input #2069

Open
adamgerhant opened this issue Oct 22, 2024 · 8 comments · May be fixed by #2139
Open

Add list of acceptable types when hovering over node input #2069

adamgerhant opened this issue Oct 22, 2024 · 8 comments · May be fixed by #2139
Labels
Feature New feature or request Good First Issue Good for newcomers Paper Cut A small UX annoyance we should strive to improve

Comments

@adamgerhant
Copy link
Collaborator

adamgerhant commented Oct 22, 2024

As noted by Lerc, it would be helpful for the NodeInput tooltip to display all acceptable types that will not cause a node error. For protonodes, this would require accessing the list of implementations, which are defined in the node macro. The implementations would then be filtered based on the current type of all other inputs to return a list of all currently valid types for that input. Network nodes are a bit more complex, since the inputs will undergo automatic type casting (2042), and all usages need to be valid. This means the acceptable types will be the intersection of all types that implement From for the network input type, and all types that the import is connected to.

@adamgerhant adamgerhant added Feature New feature or request Good First Issue Good for newcomers Paper Cut A small UX annoyance we should strive to improve labels Oct 22, 2024
@github-project-automation github-project-automation bot moved this to Short-Term in Task Board Oct 22, 2024
@prikshitsingh24
Copy link

Hey @adamgerhant Could I take this issue?

@adamgerhant
Copy link
Collaborator Author

Sure! Dont worry about the automatic type casting yet. I would focus on just getting the proto node hovering tooltips to work, then work on the network nodes.

@prikshitsingh24
Copy link

prikshitsingh24 commented Dec 14, 2024

Hey @adamgerhant should it look something like this:
brave_W3tgUhZOr3

@adamgerhant
Copy link
Collaborator Author

Yes

@prikshitsingh24 prikshitsingh24 linked a pull request Dec 15, 2024 that will close this issue
@prikshitsingh24
Copy link

prikshitsingh24 commented Dec 15, 2024

i have implemented for protonodes please have a look in your free time and regarding network nodes I wanted to know like if i have created this node shouldn't this part automatically assign the input type of that network node to be for example in this Number
as the first node is divide similar to the output of the node or am i missing something:
brave_DNXOspG0tC

@adamgerhant
Copy link
Collaborator Author

No, that is currently the correct display. This node would give a type error when being executed since an input of TaggedValue::None is being supplied to the encapsulating network, which gets fed into the Divide node, which expects a number.

@prikshitsingh24
Copy link

Ok so how should I approach to display the type accepted by the network node in this case

@adamgerhant
Copy link
Collaborator Author

adamgerhant commented Dec 15, 2024

When hovering over the input for the encapsulating network node (which is not currently visible since its in the document network), it should use the outward connections from the import (currently visible) and return the supported type of that Divide node input.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request Good First Issue Good for newcomers Paper Cut A small UX annoyance we should strive to improve
Projects
Status: Short-Term
Development

Successfully merging a pull request may close this issue.

3 participants
@prikshitsingh24 @adamgerhant and others