jchanvfx / NodeGraphQt

Node graph framework that can be re-implemented into applications that supports PySide2
http://chantonic.com/NodeGraphQt/
MIT License
1.26k stars 256 forks source link

Disabling/enabling nodes may not work as expected #291

Closed aloytag closed 1 year ago

aloytag commented 1 year ago

When using the disable_nodes() method of an instance of the NodeGraph class, I expect the selected nodes to change their state between disabled and enabled. It works fine when all the selected nodes are enabled, or when all are disabled. Otherwise, disable_nodes() only checks the state of the first node. Thus, It doesn't matter if other selected nodes are enabled or disabled. If the first node is enabled, all the selection will be disabled (and viceversa). And it's not clear which is the first node either.

To get the selected nodes to change their corresponding state, I suggest implementing the disable_nodes() method like this....

def disable_nodes(self, nodes, mode=None):
    if not nodes:
        return
    if mode is None:
        mode = [not node.disabled() for node in nodes]
    else:
        mode = [not nodes[0].disabled()] * len(nodes)

    text = 'disable/enable ({}) nodes'.format(len(nodes))
    self._undo_stack.beginMacro(text)
    for node, m in zip(nodes, mode):
        node.set_disabled(m)
    self._undo_stack.endMacro()

In this implementation, every selected node initially enabled will be disabled. And every selected node initially disabled will be enabled. It woks well even when some are enabled and others are disabled. The mode argument is meant for "legacy" support. It could be required by other components of the library.

NodeGraphQt version: 0.5.1 Qt.py version: 1.3.7 PySide2 version: 5.15.2.1 Python version: 3.9.2 OS: Debian GNU/Linux 11 (bullseye)

Regards.

jchanvfx commented 1 year ago

I've updated the enable/disable logic to now toggle between the node states.