'connection_created' signal called too early when created by mouse action

robinechuca commented 1 year ago

Expected Behavior

I have connected the FlowScene connection_created signal to a function responsible of complete clear and redrawing the graph sometimes. The problem is that this function is called before the connection is drawn! It is a problem because when I delete all the items from this function, it is impossible for qtpynodeeditor to finish the task.

Current Behavior

I obtain the following traceback

Traceback (most recent call last):
  File "/home/robin/.pyenv/versions/3.11.4/lib/python3.11/site-packages/qtpynodeeditor/connection_graphics_object.py", line 201, in mouseReleaseEvent
    if node and interaction.try_connect():
                ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/robin/.pyenv/versions/3.11.4/lib/python3.11/site-packages/qtpynodeeditor/node_connection_interaction.py", line 175, in try_connect
    self._node.graphics_object.move_connections()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'move_connections'

Possible Solution

I've had a look at the source code and I think I've pinpointed the problem.

1) In mouseReleaseEvent method of connection_graphic_object.py, maybe call connection_created at the end of this function ? 2) In try_connect method of node_connection_interaction.py, at the line self._connection.connect_to(port) the signal is triggered. As explicited in the traceback, the graphics_object attribute of self._node can be indirectly reset because of the signal trigger. 3) In connect_to method of connection.py, the line self.connection_completed.emit(self) trigger the connection_completed signal. It is not directly the connection_created signal. 4) In create_connection method of flow_scene.py, the line connection.connection_completed.connect(self.connection_created.emit) has previously associated the 2 signals. Why this two signals are connected?

Maybe the solution could be to remove this signal alias in create_connection method and trigger directly connection_created at the end of try_connect or mouseReleaseEvent ? I'm not involved in the project, so maybe the solution I'm proposing will break something else ?

Thank you for this beautiful project, and if you guide me, I'm ready to try a pull request on this subject.

robinechuca commented 1 year ago

Solution

I have just try it and run the benchmark on my one computer: In flow_scene.py in create_connection, simplify the code with:

...
cgo = ConnectionGraphicsObject(self, connection)
# after self function connection points are set to node port
connection.graphics_object = cgo
self._connections.append(connection)

if port_a and port_b:
    in_port, out_port = connection.ports
    out_port.node.on_data_updated(out_port)
    self.connection_created.emit(connection)

return connection

And in connection_graphic_object.py in mouseReleaseEvent, append the signal trigger:

...
self.ungrabMouse()
event.accept()
node = self._scene.locate_node_at(event.scenePos(), self._scene.views()[0].transform())

interaction = NodeConnectionInteraction(node, self._connection, self._scene)
if node and interaction.try_connect():
    node.reset_reaction_to_connection()
    self._scene.connection_created.emit(self._connection)

if self._connection.requires_port:
    self._scene.delete_connection(self._connection)

klauer commented 1 year ago

Thanks for reporting that, @robinechuca. I think your suggestion seems reasonable for the most part - would you mind opening a PR? I'll give it a bit of testing on my end then.

It's possible we might need to still move around the connection_created signal emission line a bit, but I'll need to refresh my memory on this codebase a bit before I can say for certain.

robinechuca commented 1 year ago

Ha cool, thanks, it's nice to be supported ;) To be honest, I'm not very comfortable with pull request. If you want to do tests and all, as I'm only proposing 2 small changes, maybe you can make a pull request yourself ? It'd be a lot easier ! I don't care if I don't appear as a contributor. Anyway, if I really have to make the pull request myself, I will try, but it's going to be tricky.

klauer / qtpynodeeditor