Changed KSamplerAdvanced input types

[axymeus]

#9847

[blepping]

Have you considered how this will affect existing workflows?

[axymeus]

I've opened this PR as a point of reference. How it should be handled in regard to existing overflow is up to you.

– Best solution would be to make a check during deserialization of a workflow. Fully transparent to the user. Not sure if comfy already has version control serialization in place. In any case, if you can point me in the right direction I should be able to take care of it.
– Could deprecate KSamplerAdvanced in favor of a new KSamplerAdvancedBoolean with the correction.
– Accept to break backward compatibility, accepting it should have been like that in the first place and that it opens new possibilities the change may be welcomed. It requires a simple toggle to fix back.

[axymeus]

I ended up implementing a KSamplerBoolean instead. Now the best but the easiest solution at least. 100% backward compatible.

[blepping]

How it should be handled in regard to existing overflow is up to you.

Just to be clear, I'm a random person with no authority sharing my opinion so nothing is up to me.

Best solution would be to make a check during deserialization of a workflow. Fully transparent to the user. Not sure if comfy already has version control serialization in place. In any case, if you can point me in the right direction I should be able to take care of it.

Unfortunately, I don't think anything like that currently exists. I'm actually not even sure where deserialization happens.

I ended up implementing a KSamplerBoolean instead. Now the best but the easiest solution at least. 100% backward compatible.

I don't like the enable/disable combo approach either but I don't feel like it's worth adding a whole new node for a minor cosmetic issue like that. Like I mentioned though, that's just my personal opinion. I definitely don't think the approach that made the node incompatible with existing workflows would work (or be accepted) and ComfyUI doesn't have a history of making workflow breaking changes. Not sure I can even think of one in recent history, and definitely not to solve a minor/cosmetic issue.

[Kosinkadink]

I could see a change like this being made when a quality-of-life node review is done, but it would require some testing to see what breaks + a better deprecation system. A new node that is identical to another except for one input difference probably won't get merged until that deprecation system is in place.

[axymeus]

I don't like the enable/disable combo approach either but I don't feel like it's worth adding a whole new node for a minor cosmetic issue like that.

It's not only cosmetic however. Custom data types cannot be remotely controlled. Booleans can. For instance, Wan2.2 has a popular 3 samplers workflow. With boolean values I can automatically control the add_noise parameter of the 2nd sampler based on whether the first sampler is enabled or not. My workflow depends on being able to do that.

Boolean fields can interact with GetNode / SetNode from popular KJNodes pack for instance, while custom data types aren't.

[blepping]

Custom data types cannot be remotely controlled.

Not saying it's a good solution or anything but it's not impossible to do this (at the moment, anyway). For example, the BlehCast node in my node pack here will let you connect anything you want to any input: https://github.com/blepping/ComfyUI-bleh/blob/bb3c5981c72d349f655df27eb65de450aa09ae6b/py/nodes/misc.py#L134

So you can do something like this:

Note: It doesn't do any conversion or checking that the types are compatible so if you connect something that has an actual value that the input doesn't expect then stuff will explode.