comfyanonymous / ComfyUI

The most powerful and modular stable diffusion GUI, api and backend with a graph/nodes interface.
GNU General Public License v3.0
40.19k stars 4.28k forks source link

[Enhancement suggestion] API, avoid ambiguity #522

Open bmad4ever opened 1 year ago

bmad4ever commented 1 year ago

It is somewhat cumbersome to identify the nodes to set up the data to send in the prompt when there are multiple nodes of the same type (not all nodes are text / load image, and the user may also want to use string-encoded images).

At the moment I see 2 possible solutions:

  1. add the title of the node in the workflow (maybe only if changed?) to the respective node in the prompt json;
  2. allow to use of the workflow json as a prompt, as it already contains the names of the nodes.
comfyanonymous commented 1 year ago

Yeah maybe there should be a semi hidden developer menu with an option to export the current workflow in api json format.

You can look at the PNG info of the images that ComfyUI generates, the information in the prompt section is in the api json format.

bmad4ever commented 1 year ago

Thank you for the reply. Although having the option to export the workflow in API JSON format would be a nice QoL feature to have, it would not solve the mentioned inconvenience. To clear potential misunderstandings I will elaborate a bit more:

After having the API JSON, if the user wants to replace some of the original data with some other data, they need to check the nodes in some text editor. If there are multiple nodes of the same type with similar, or hard-to-identify, data, this can be somewhat cumbersome. That I am aware (please correct me if there is some other way), ignoring the node data, the only way to find a specific node is to check its ID in the workflow JSON (because it also contains the node name) and then search for it in the API JSON; and, since the API JSON is not "deserialization friendly", you have to do it by string matching "id": to avoid matches with other numbers.

Some way to avoid this inconvenience would be appreciated.

Additionally, from an opinionated standpoint, I would prefer the API JSON to be easy to deserialize, in the same fashion the workflow JSON is, but I understand that this may not be possible, or desirable.

Kind Regards.

comfyanonymous commented 1 year ago

Would having an option to show the api names (with the node id) as the node names instead of the friendly names solve that?

bmad4ever commented 1 year ago

Being able to check a node's ID in the UI would help, yes. Although not frictionless, as it is easier to keep track of names than IDs when "interfacing", it would eliminate the need to find the node in the workflow json to then find its match on the api json.

morphles commented 1 year ago

So export api workflow would be super welcome, was suprised that saved one can't be run via API. I think nicest would be, to have "global vars" node, or multi wars, and just wire it to all the stuff you need to have variations on. Then you just need to locate that "global vars" node in JSON (ultra bonus points, if it is made sure such node is exporeted the first in output). Then handling any variation you might want would become quite simple.

bmad4ever commented 1 year ago

@morphles a global vars node sounds doable. Potentially cumbersome to add new node types, but for most use cases should suffice. Might try to implement something like that... if no one beats me to it, I will share it later. It may leave some feeling a bit dirty, but if it gets the job done I have no qualms with it.

bmad4ever commented 1 year ago

I was able to implement a bare-bones, dirty, implementation of a "global vars" node.

Was not yet able to set different data types, only strings. Can make workaround with nodes to parse the data, but is there no other way ? ? ?

working with strings

It seems, RETURN_TYPES in the class must be defined with the correct return types... From the javascript side, setting the type of the new output or calling setOutputDataType does not change expected output types, and I get a: Failed to validate prompt ... error. So, I set a high number of STRING outputs in RETURN_TYPES, and remove them on the javascript side.

I will try to revisit it later this week, and if I find no other solution, I will clean the implementation before sharing.

WASasquatch commented 1 year ago

Having a Right Click -> "Copy Node ID" context would be amazing.

bmad4ever commented 1 year ago

Done, added the node to my repo. The relevant node RequestInputs, is implemented in api_nodes.py and RequestInputs.js.

The API json can be printed/written using the auxiliary node GetPrompt ( this node will not appear in the json ).

So far I have not written any documentation, I hope it is easy enough to use.

Kind Regards.

WASasquatch commented 1 year ago

Done, added the node to my repo. The relevant node RequestInputs, is implemented in api_nodes.py and RequestInputs.js.

The API json can be printed/written using the auxiliary node GetPrompt ( this node will not appear in the json ).

So far I have not written any documentation, I hope it is easy enough to use.

Kind Regards.

Great work!

bmad4ever commented 1 year ago

My bad for the previous notification (it wasn't working as expected). I just fixed RequestInputs and GetPrompt nodes, they should work as expected now.

Other nodes in the repo, that use hidden inputs, should not work via api, so despite the file name, until they are fixed, they are useless. I was previously not aware that some hidden inputs could not be used when using api. They seem to require workflow data that is missing from the generated api request.