Skip to content
This repository has been archived by the owner on Apr 9, 2024. It is now read-only.

Fix/incompatible port snap #62

Merged
merged 2 commits into from
Sep 15, 2022
Merged

Fix/incompatible port snap #62

merged 2 commits into from
Sep 15, 2022

Conversation

setzer22
Copy link
Owner

This takes care of the second half of #54, where ports snapped to incompatible colors even when the connection was not possible. As discussed there, once we allow custom rules for connections between different colors (this was being worked on as part of #30) we will need to adapt this logic from using == to using the custom comparison function.

I also took care of a bug where removing an existing connection and re-wiring it to the same port would not work. This is now fixed.

This takes care of the second half of #54, where ports snapped
to incompatible colors even when the connection was not possible.
There was a bug where removing an existing connection and re-wiring it
to the same port would not work. This is now fixed
Comment on lines +182 to +187
NodeData,
UserState,
DataType: DataTypeTrait<UserState>,
ValueType,
Key: slotmap::Key + Into<AnyParameterId>,
Value,
Copy link
Owner Author

Choose a reason for hiding this comment

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

Unfortunately, there's no type inference, even local private functions in Rust, so we need to carry over the ugly generics from the parent calling fn 😬

This cannot be made into a closure (which would support the generics) because it needs to be called with different types, and there's no monomorphization for closures.

Comment on lines -264 to +295
let other_node = self.graph.get_input(*input).node();
let other_node = self.graph.get_output(*output).node;
Copy link
Owner Author

Choose a reason for hiding this comment

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

This was a bug: When a disconnection event occurs, the node that still remains is the one attached to the output side, not the input.

@setzer22 setzer22 merged commit 0395039 into main Sep 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant