[feat] Some improvements to dynamic inputs (and proposal for contribution)

[pterrien]

Describe the problem

Currently the dynamic inputs work well, but could be slightly improved with the following features:

• custom separator, instead of default _ (even empty '')
• custom start_index, instead of default 1 (I personally prefer 0, but I understand most people don't)
• allow using non-dynamic / static inputs in addition to dynamic ones

Describe the solution you'd like

I have implemented some light modifications to your code (setupDynamicConnections() and dynamic_connection() in web/comfy_shared.js) to handle the cases above:

• modify the options to add separator and start_index
• allow static inputs to coexist with dynamic ones, with the following drawbacks:
  • input order is not managed, meaning that static ones do not always appear at the top of inputs
    • but I guess dealing with input order would be far more complicated (recreate links?)
  • possible undesired behavior if a static input starts with the same prefix as dynamic ones and there's no separator (eg argument vs arg)
    • because I use a simple startsWith() to filter dynamic ones
    • this could be improved by using a regexp, with a possible (minor) impact on performance

Alternatives considered

No response

Additional context

Salut !

First, a big thank for your work!
Although rather new to Comfy, I got interested with dynamic inputs for my own / personal nodes, so I first discovered the cozy-comfyui repo (very nice introduction), then came across yours which proposes a more advanced solution.
And I must say I'm impressed, except a very minor issue on manual disconnection (totally acceptable), it works perfectly!

I implemented the tiny QoL improvements above for my own use: the first one (separator / start_index) is pretty straightforward, the second one a bit more debatable, as not dealing with input order may confuse users a bit.

No revolution here, they are very simple, but if you think they could be interesting, just tell me and I'll be happy to contribute.
Either by describing the changes in this issue, or even proposing a PR if you want.

Thanks again for sharing your work, cheers!

[pterrien]

I checked setting labels, and could bring another very minor improvement (they are currently discarded).
I may also need some explanation about the nameArray option, as I do not know what it is and want to properly test that, too.

[melMass]

Hi,

Thanks for the write up!
Those are great improvements, mixing static and dynamic is something I once solved but it stopped working at some point, I'd love to bring that back! I'm completely open for a PR!

• nameArray? This is a setting I use in some unpublished mtb nodes to define the dynamic names sequentially.... I use it in my CosyVoice fork too: dynamicAlphabet, where it uses the alphabet (A,B,C,D...) as dynamic input names. Another use case is multi axis stuff, so you could pass [x, y, z, w]: when connecting x, y pops up, when connecting y, z pops up etc etc...

370914061-25630131-2f4a-45bc-80e8-9b5acaf2f1c9.mp4

[melMass]

Oh and FYI I have a lot of unpublished code (mostly js/frontend stuff) that I plan to organize better: use a bundler, probably switch to typescript....
The org just released the types that I've requested ever since the new UI, this will improve DX by 10 for me: https://www.npmjs.com/package/@comfyorg/comfyui-frontend-types

[pterrien]

Great! So I will work on a PR (my first one...), you can expect something by the end of this week the latest.
So to summarize, I will implement all features above (include labels). Regarding nameArray, my modifications should have not impact on it, but I will do my best to ensure this.

And BTW: I'm more comfortable with TS than JS (and I surely prefer good typings), so in the future if you need some help, don't hesitate to ping me. That would be a good exercise for me to understand Comfy better.

[pterrien]

Hi @melMass,
All my sincere apologies, I had some personal issues and couldn't match the deadline I promised.
But that should be better in the following days, in any case the base code is ready, just need to merge it and properly test it with yours.
BTW: how do you want me to provide a test base? Should I add some other Debug nodes (with static inputs for example), and if so as a separate branch not included in the PR? Or should I just ignore that part, and let you test the changes on your own?

[melMass]

No pressure at all!
Thanks.
I think for this nothing beats manual testing unless we have a good "framework" to do so, I'm starting to add tests for Python progressively, but canvas tests are quite involving. The comfy org uses a very slick test pipeline I want to take some inspiration from at some point: https://ci.comfy.org/

[melMass]

(That said it can be also tested with a workflow showing various cases we can try to break)

[pterrien]

Hi @melMass ,

Once again, all my sincere apologies for the time I took to push this PR, but here it is at last! -> #222

The 3 improvements described above have been implemented. You may want to check the JSDoc syntax (not really used to), for the rest it should be good I think.

Note 1: sorry, my prettier config may have slightly messed up with yours, so you may have to re-save the file I'm afraid...

Note 2: my code currently relies on startsWith to identify the dynamic inputs. In some rare specific cases (e.g. if dynamic inputs are prefixed with arg and there is one static input named argument), this may cause issues.

• we could improve that by using a regexp (which won't be perfect but better)
• but this is not very efficient performance-wise, as we'll have to recompile the regexp at each function call

We talked about testing, and I could propose a node you may find interesting. It's a simple string formatter (using Python string.format()), taking any number of inputs, with a single string output:

• connect your inputs (can be string, int, float...)
• setup your template using placeholders; either by input name ({arg0}), or by index ({0})

Example with the template string: I saw a {arg0}, wearing a {1} coat, in a {arg2}:

(do not focus on the inputs, these are custom and not relevant here)

I hope you'll find this PR interesting. Do not hesitate if you want me to update this PR with the regexp, or to create a new one with the string formatter.

Cheers!

[melMass]

Nice thanks for the write up!
And really no worries about the timing.

For point 2 I think it's not a huge constraint to have, you just have to be aware of it!

Regarding testing I will already test all the dynamic nodes of mtb, in case they broke, there is quite a few so it should be helpful. But yes maybe it's worth making nodes using the new options you proposed, I'll prepare that tomorrow.

And btw, it's poorly named but Apply Text Template is just that:

comfy_mtb/nodes/graph_utils.py

Lines 87 to 116 in e5482ae

	# class MTB_ApplyTextTemplate:
	class MTB_ApplyTextTemplate:
	"""
	Experimental node to interpolate strings from inputs.
	Interpolation just requires {}, for instance:
	Some string {var_1} and {var_2}
	"""
	@classmethod
	def INPUT_TYPES(cls):
	return {
	"required": {
	"template": ("STRING", {"default": "", "multiline": True}),
	},
	}
	RETURN_TYPES = ("STRING",)
	RETURN_NAMES = ("string",)
	CATEGORY = "mtb/utils"
	FUNCTION = "execute"
	def execute(self, *, template: str, **kwargs):
	res = f"{template}"
	for k, v in kwargs.items():
	res = res.replace(f"{{{k}}}", f"{v}")
	return (res,)

[pterrien]

Maybe easier than a PR, here is the code for the string formatter node, as you can see it's dead simple:

class FormatString:
    def __init__(self):
        pass

    @classmethod
    def INPUT_TYPES(s):
        return {
            'required': {
                'format': (
                    'STRING',
                    {
                        'default': 'my first arg: {arg0}, second by index: {1}',
                        'multiline': False,
                    },
                ),
            }
        }

    @classmethod
    def VALIDATE_INPUTS(s, input_types):
        for t in input_types.values():
            if t not in ['STRING', 'INT', 'FLOAT', 'BOOLEAN']:
                return False
        return True

    CATEGORY = 'ChangeMe'

    RETURN_TYPES = ('STRING',)
    RETURN_NAMES = ('formatted',)

    FUNCTION = 'run'

    def run(self, format: str, **kw):
        # handle both index and name references
        output = format.format(*kw.values(), **kw)
        return (output,)

With the following JS code, in beforeRegisterNodeDef():

setupDynamicConnections(nodeType, "arg", "*", { separator: "", start_index: 0,});

[melMass]

Ah yours looks better! nice