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

No disconnect when changing node type with less joints in visual shader editor #79417

Closed
timkrief opened this issue Jul 13, 2023 · 5 comments · Fixed by #89810
Closed

No disconnect when changing node type with less joints in visual shader editor #79417

timkrief opened this issue Jul 13, 2023 · 5 comments · Fixed by #89810

Comments

@timkrief
Copy link

Godot version

4.1.stable

System information

Ubuntu 22.04

Issue description

Visual shader editor should disconnect (at least visually) when we change the number of joints

before changing the type
image

after
image
image

this one break things
image

Steps to reproduce

You can see what to do to reproduce the bug in the screenshot

Minimal reproduction project

A visual shader, two nodes.

@EddieBreeg
Copy link
Contributor

I'd like to give this one a shot if no one is working on it

@EddieBreeg
Copy link
Contributor

EddieBreeg commented Jul 13, 2023

I found the source of the issue, however I really am not sure about how to fix it.
When you change a compose/decompose type, the set_op_type method of the relevant node class gets called.

void VisualShaderNodeVectorCompose::set_op_type(OpType p_op_type) {

void VisualShaderNodeVectorDecompose::set_op_type(OpType p_op_type) {

These methods just set the new vector type, and emit a signal to the editor. This signal gets handled by this method:

void VisualShaderGraphPlugin::update_node(VisualShader::Type p_type, int p_node_id) {
if (p_type != visual_shader->get_shader_type() || !links.has(p_node_id)) {
return;
}
remove_node(p_type, p_node_id, true);
add_node(p_type, p_node_id, true);
}

As far as I can see, the update method has no way of knowing the number of ports has changed, so it can't take care of removing the extra connections when needed.

The simplest solution I can think of would be to handle the signal with another method, similar to _expand_output_port.

@EddieBreeg
Copy link
Contributor

Another solution would be to compare the number of input/output ports between the start and the end of the update method.
If this number has changed we can then find the connections we should remove. It works, but I don't know how I feel about doing this in the update method.

@EddieBreeg
Copy link
Contributor

I sort of managed to fix the issue, but unfortunately it created an even weirder bug.

Here if I changed the vector compose node to vector2, it will correctly disconnected the ports.
image
image

But if I then switch the node back to 3D, and then try to recreate that EXACT same connection, it won't allow me: nothing happens. The connection doesn't register. If I now try to create a different connection to that port, it works as expected and everything goes back to normal.
image

@EddieBreeg
Copy link
Contributor

#79417 (comment)

This is now fixed, it was in fact caused by my original code not fully fixing the problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment