adieyal / comfyui-dynamicprompts

ComfyUI custom nodes for Dynamic Prompts
MIT License
187 stars 20 forks source link

Random seed support #29

Open romeobuilderotti opened 8 months ago

romeobuilderotti commented 8 months ago

Finally we get reproducible workflows!

Changes:

Caveats:

As an offtopic, I see "view prompt" item in your to do list. There is a working implementation in Custom Scripts plugin but it involves some JS hacks

Robzz commented 8 months ago

I stumbled on your PR while looking for a way to get reproducible workflows using this extension and gave it a try. It does seem reproducible, but it also seems to break wildcards in the random prompts node (tested by dumping the generated prompt to the console using a node from the WAS node suite). Anyway, thank you for making this PR, this would be very nice to have :heart:

romeobuilderotti commented 8 months ago

@Robzz oh this is because of the change of DPRandomGenerator to inherit from DPGeneratorNode instead of DPAbstractSamplerNode as it doesn't initialize the wildcard_manager. TBH I don't exactly know why only random and combinatorial nodes had support for wildcards and not others, maybe wildcard support should be moved to a base class so that all nodes support it?

By the way, I'm currently working on wildcard pipes in a separate branch of my repo, so maybe common code for loading wildcards both from files and the pipe can be moved there. image Not sure whether other people think think that this approach to wildcards is useful. I use it for smaller item lists to split my large prompts into pieces while keeping everything inside the workflow.

adieyal commented 7 months ago

Hi @romeobuilderotti

Thanks for the PR. I'm looking at it now, but I don't understand why you removed the WildcardManager from the Random Prompt generator.

romeobuilderotti commented 7 months ago

@adieyal I made DPRandomGenerator inherit from DPGeneratorNode which is a better fit for random generation rather than DPAbstractSamplerNode which deals with deterministic generation. At least this is how I understood the need for 2 different base classes.

Probably in addition to that WildcardManager should be extracted to a separate file so that both DPAbstractSamplerNode and DPGeneratorNode support it. What is the reason why jinja, feeling_luckly and magicprompt didn't support wildcards? I didn't use file based WildcardManager so I lack experience with it