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

Connections incorrectly assumed during LGraphNode.prototype.configure #481

Open
AdamantLife opened this issue Jun 2, 2024 · 1 comment

Comments

@AdamantLife
Copy link

AdamantLife commented Jun 2, 2024

In litegraph.js:

// Lines 2571 to 2573
var link_info = this.graph ? this.graph.links[input.link] : null;
if (this.onConnectionsChange)
	this.onConnectionsChange( LiteGraph.INPUT, i, true, link_info, input ); //link_info has been created now, so its updated

// Lines 2588 to 2590
var link_info = this.graph 	? this.graph.links[output.links[j]] : null;
if (this.onConnectionsChange)
	this.onConnectionsChange( LiteGraph.OUTPUT, i, true, link_info, output ); //link_info has been created now, so its updated

The first line of code acknowledges that link_info may be null, but onConnectionsChange() is always called and is also unconditionally called with the connected argument as true (the comment also suggests that link_info will never be null).

I did a quick sanity check to make sure that- in practice- link_info can be null by cloning a node with a connected input and confirmed that the link_info of the newly initialized node was null. This also contradicts the comment that asserts link_info had been created.

It seems like onConnectionsChange should either be called only when link_info is not null (or is truthy), or the connected argument should be false if link_info == null.

@atlasan
Copy link
Contributor

atlasan commented Jun 2, 2024

Having broken data is a situation in the-middle. Should we inform or not when we find a link with broken data?
The comment seems to have copied from another section or that was relevant to a global link_info debugging as noting on other portions.
The null link_info is enough at now to check in the custom implementation to ensure connection is actually valid.
Being a configure event it could be useful sometimes to check and condition on that, but this should not happen: that is a broken link reference. The true on the third parameter specify if connecting or disconnecting, so the meaning would be: trying to connect a broken link while configuring.
So, when registering a callback to onConnectionsChange let's always check that link_info received is good.

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

No branches or pull requests

2 participants