Skip to content

Update all ports when edge gets connected/disconnected #25

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

Merged
merged 2 commits into from
Sep 14, 2019

Conversation

RyouBakura
Copy link

First of all, thank you for your amazing work alelievr.

This PR fixes 1) dynamic ports and 2) your dynamic port example. Sadly your example never worked for me in the first place. :(

Anyway, now even an advanced node like below should update properly:

vHsaiMnjUX

@alelievr
Copy link
Owner

alelievr commented Sep 9, 2019

Hey, Thanks for you interest in the project :)

I just see that my custom ports node is indeed broken so i'll look into that.
For your PR i'm sorry but i can't merge it in it's current state, the way i wrote the MultiPorts node prevent a bunch of issue that happens when you play a lot with edges in dynamic ports (having a max number of ports that growth is way easier than a fully dynamic port count).

Some bug i encountered while playing around with the multi-port node
2019-09-09 18 12 42
2019-09-09 18 21 37

@RyouBakura
Copy link
Author

This is what it looks like for me (keep in mind the following is with my PR). When a port with an existing edge gets destroyed, no error is thrown and the edge just disappears.

oFHkoDjSE4
vbawZsa4t6

I'm only using the mouse to delete edges. If I use the delete hotkey, I get the same exception as you.

Edit: After some investigation I found that the difference between the two deleting methods is:

Delete with hotkey: BaseGraphView.Disconnect(edge) is invoked once.
Delete with mouse: BaseGraphView.Disconnect(edge) is invoked twice.

Simply duplicating the Disconnect call makes the hotkey work. ^__^

@alelievr
Copy link
Owner

I didn't noticed that they were different behaviors between delete by mouse or keyboard, thanks for noticing this.
I also found other issues related to graph deserialization with the current implementation of multi-ports that causes bugs with your work, so the plan is to merge your PR into a branch on my project so i can apply all the fixes i have and then hopefully land this to master :)

@alelievr alelievr changed the base branch from master to multiport-fixes September 14, 2019 16:58
@alelievr alelievr merged commit b256c6d into alelievr:multiport-fixes Sep 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants