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

Polish user-created subgraph nodes: imports in the Properties panel; reorder/delete/rename imports/exports #2105

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

Conversation

adamgerhant
Copy link
Collaborator

@adamgerhant adamgerhant commented Nov 10, 2024

Closes #2208 and continues to resolve #2043. Also resolves #2069.

Allows removing, renaming, and reordering imports/exports.
Input based properties for network nodes to allow useable reordering
Adds Tooltips for acceptable types of a node input
Annotations for both node and input based overrides added to the node macro

Reworks how the properties panel is generated. A full node level override for the entire panel is still possible, which is stored as a lambda in the definition. Modifying the imports/exports means that the reference is lost, and the properties are instead generated individually for each input. By default, this will use the data type/type annotation to generate an appropriate set of widgets. This is done in property_from_type. For custom input overrides, each input has the PropertiesRow struct, which can store a reference to a widget override (a lambda which will generate a custom set of widgets). This lambda is stored in a hashmap, since it would not be possible to serialize it for every node. This also makes it easily extendible, just like how the definitions are stored. This lambda has access to the entire network interface, which it can use to access any information it needs to generate the widget. In addition to the widget override reference, each input also stores a hashmap of serializable data, which can store any information necessary. For the most part this will just contain the input name, but the author of the node could store additional data as they need.

TODO:

  • Rename inputs in properties panel (deferred)
  • Automatic Into node insertion with drop down menu (deferred)
  • Protonode name at top of properties panel
  • Hiding input should automatically disconnect
  • Filtering out the primary inputs of autogenerated Properties panel node sections so only secondary inputs are shown (e.g. Ellipse and Merge nodes)
  • Valid input types for the identity node crashes
  • Upgrading previous documents

@Keavon Keavon force-pushed the modify-import-export branch from 73e1ae5 to 0fbbffd Compare November 12, 2024 22:32
@Keavon Keavon changed the title Continue adding support for user created node graphs Continue adding support for user created subgraphs Nov 12, 2024
@GraphiteEditor GraphiteEditor deleted a comment from github-actions bot Jan 13, 2025
@Keavon
Copy link
Member

Keavon commented Jan 16, 2025

Please implement upgrading logic for the Noise Pattern node when you get the chance. Thanks!

@Keavon Keavon changed the title Continue adding support for user created subgraphs Polish the experience around user created subgraph nodes Jan 20, 2025
@Keavon Keavon changed the title Polish the experience around user created subgraph nodes Polish user-created subgraph nodes: imports in the Properties panel; reorder/delete/rename imports/exports Jan 20, 2025
@Keavon
Copy link
Member

Keavon commented Jan 20, 2025

I'll discuss this with you on Discord but here are my notes of unresolved questions/potential concerns from my code review:

  • Weird capitalization in one of the first files that has all the spread out changes
  • Graphite save documents have possibly unintentional fields in the JSON
  • Can strings like #[widget(ParsedWidgetOverride::Custom = "assign_colors_repeat_every_row")] repeat_every: u32, be updated anytime without affecting saved documents? Some inconsistent naming like this example ending in _row which might mean we should do a full review on these before merging if that affects user documents.

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