asagi4 / comfyui-prompt-control

ComfyUI nodes for prompt editing and LoRA control
GNU General Public License v3.0
200 stars 16 forks source link

[BUG] ComfyUI's random choice prompt syntax isn't supported #58

Closed Retusthese closed 2 months ago

Retusthese commented 2 months ago

Before you start Verify the following:

Describe the bug ComfyUI's built-in prompts allow randomization in the form of {choice1|choice2|etc}, but Prompt Control's prompts don't select a random choice, instead seemingly using the text as-is. (I'm assuming this is a bug, as Prompt Control is intended to not deviate from the behavior of equivalent built-in nodes.)

To Reproduce Here's an importable workflow screenshot demonstrating the behavior of built-in prompts alongside Prompt Control's: workflow

Expected behavior Prompt Control's prompts should handle randomized choices as the built-in ones do.

asagi4 commented 2 months ago

This is not a bug with prompt control; the text inputs don't set a dynamicPrompts to true and I have no intention of doing so, since I think ComfyUI's dynamic prompts being dictated by the backend is a bug in ComfyUI. It should be a UI toggle and the backend should not be involved at all.

I think you can get the dynamic behaviour if you make the widget into an input and connect a text primitive or some custom node that enables dynamic prompts and outputs strings into the PromptToSchedule node.

It's easy enough to work around; I'm not going to change this in prompt control.

Retusthese commented 2 months ago

Ah, that's unfortunate. Looks like DynamicPrompts is tied to widgets, and the only widgets that enable it are on the built-in prompt nodes. A text primitive fed into a prompt node (even built-in) doesn't do it, and none of the third-party nodes I have enable it. Even your MUJinjaRender node doesn't, despite avoiding {} as if it did. I assume these nodes simply predate the DynamicPrompts extension.

Your opinion of dynamic prompts confuses me, though. It looks like the backend enables the extension for appropriate widgets (such as prompt inputs), and the frontend JS actually handles it, so there could be a UI toggle for it, but nobody ever implemented one. Sounds more like a feature request than a bug.

the backend should not be involved at all.

How would the frontend even know what widgets are prompts unless the backend flags them as such?

Anyway, this doesn't seem quite so easy to work around, as I've neither yet seen, nor know enough to make, a custom text output node with DynamicPrompts. But thank you for pointing out what I'd need to jury-rig! If anyone else stumbles on this problem, dynamic prompts can be added to nodes by modifying their .py file, finding the multiline input(s) in their INPUT_TYPES that look something like this: "text": ("STRING", {"multiline": True}), and adding , "dynamicPrompts": True, like so: "text": ("STRING", {"multiline": True, "dynamicPrompts": True}),

asagi4 commented 1 month ago

How would the frontend even know what widgets are prompts unless the backend flags them as such?

The frontend could take a hint from the backend, but it should still be a toggle in the frontend. What goes in the text field doesn't matter. The frontend also already has properties that are stored frontend-only, so really even the hint from the backend is unnecessary.

You can easily make a custom node like so:

class DynamicString:
  @classmethod
  def INPUT_TYPES(s): 
    return {'required': {"text": ("STRING", {"dynamicPrompts": True})}}

  RETURN_TYPES = ("STRING",)
  FUNCTION = "do"
  def do(self, text):
    return (text,)

And yeah, I consider it a bug because the feature is badly designed.