search

WASasquatch / was-node-suite-comfyui

An extensive node suite for ComfyUI with over 210 new nodes
MIT License
1.22k stars 177 forks source link

[BUG] WAS_Text_List_Concatenate doesn't work at all #303

Closed Arcitec closed 11 months ago

Arcitec commented 11 months ago

Hi, thanks a lot for this node pack. I'm working my way through the text nodes to try to make a dynamic prompt workflow. So far I'm using a bunch of Text Multiline nodes into a Text List node, which allows me to Bypass any Text Multiline boxes to rapidly toggle certain parts of the prompt.

All I need now is to convert that list of ["a", "b", "c"] strings into a single comma-delimited String for use in prompting.

So I noticed that the Text List node's output is only compatible with the WAS_Text_List_Concatenate node. And I see that it has a "delimiter" input. So I assume that it's meant to turn the provided lists into a concatenated string.

If so, there are many bugs in the source code of the WAS_Text_List_Concatenate node:

  1. The delimiter parameter is not used whatsoever. It's literally not used in the code at all.
  2. The delimiter is marked as a REQUIRED parameter. But if we provide it, the node bugs with WAS_Text_List_Concatenate.text_concatenate_list() got an unexpected keyword argument 'delimiter because the node isn't coded properly to take that parameter, hehe.
  3. The list_b is marked as a REQUIRED parameter too. If the purpose is to concatenate 1+ lists into a text string, it should only require the list_a. The rest should be optional.
  4. The code forgot to actually convert the list to a STRING/TEXT. Because I assume that the entire purpose of the node is to take a bunch of lists and convert them to a normal string with a delimiter? Instead, it's just returning a concatenated list of all the given lists. I can't find any other nodes for turning the lists into a string.
  5. It would be more convenient if delimiter was an internal text field (which can be converted to an input if needed), so we don't need to connect a separate Text node just to provide a delimiter.

PS: Adding a new, separate node that is able to concatenate lists would also be a good idea to complete the suite, basically to allow merging many separate Text List into one big list before concatenating it all into a text string with a final delimiter.

WASasquatch commented 11 months ago

If you have fixes, or well rewrites probably in a lot of cases please make a PR. Most of this needs a rewrite. I basically had been learning Python with this. Some nodes were just never finished and pushed along with other nodes, too. Found some of those a few months ago.

I don't have the time to work on this project right now (or many others).

Arcitec commented 11 months ago

Ah I see. I can thank you for inspiring me to learn how to write ComfyUI nodes. Since this was broken, I spent a few hours researching and developing some nodes, and that's mostly thanks to you. ^^

There's a trick that can be done to merge all these incoming texts via **kwargs. I'll see if I have time to make a rewrite and pull request for this node tomorrow!

WASasquatch commented 11 months ago

Ah I see. I can thank you for inspiring me to learn how to write ComfyUI nodes. Since this was broken, I spent a few hours researching and developing some nodes, and that's mostly thanks to you. ^^

There's a trick that can be done to merge all these incoming texts via **kwargs. I'll see if I have time to make a rewrite and pull request for this node tomorrow!

I knew I should have used kwargs from the beginning, cause it's actually not fun adding all the params across a custom node lol