ltdrdata / ComfyUI-Workflow-Component

This is a side project to experiment with using workflows as components.
GNU General Public License v3.0
219 stars 9 forks source link

Optional values fix #21

Closed philhk closed 1 year ago

philhk commented 1 year ago

This PR fixes the issue where optional values don't actually work. Here's an explanation:

I replaced prompt with copy.deepcopy(prompt) in the doit function. The way you are unlinking optional prompts:

# Unlink unprovided option prompt
for key, value in prompt.items():
    new_value = {name: input_value for name, input_value in value['inputs'].items() if
                 not isinstance(input_value, list) or input_value[0] not in empty_option_prompts}
    prompt[key]['inputs'] = new_value

causes the old prompt values to be overwritten, thus causing links to be gone forever.

I also replaced:

empty_option_prompts = [key for key, value in prompt.items() if key in optional_inputs and key not in pe.outputs]

with prompt_ids because pe.outputs will always have the old values. This means if actual inputs are removed, it will still think they are there. I think it would be better to completely rely on the actual inputs instead of the outputs to detect changes.

The way I'm handling this bug may be suboptimal, possibly because I only started using Python today, and it took me about 6 hours to find this bug.

To replicate the bug, you only need to create a workflow node and add an optional input. When you start the queue and have an optional input connected, it will work. However, if you try to remove the input, it will still be registered as connected. The same issue occurs in the opposite scenario: if you don't connect it before the first queue, it will continue to believe it's not connected even when you do connect it.

ltdrdata commented 1 year ago

It my brief test it doesn't work. I don't have time right now, so I'll look into it later.

opttest.component.json.txt

ltdrdata commented 1 year ago

This is fixed on 9e8ccf5fa163cf2febabffff40989f77e8c51e62