fix: allow empty prefix and unique name - #40
Conversation
|
Conceptually @Sasbom why was there a need for this prefix / suffix juggling to begin with? Isn't that exactly what |
The whole reason why I added this is to make sure there is as little ambiguity when naming a product. The secondary prefix is optional, but when I saw artists use this tool, they were exporting lots of passes to disk, which is why I thought that it would be best if i enforce a unique name, to prevent collisions and make sure that everything is clearly labelled, as the names of the nodes are used to look them up during collection and other processes, if I recall correctly. The unique identifiers Comfy gives them have been shaky for me, i personally haven't found them to be as reliable to match with. I suppose that'd be a bit overboard if you are only generating one image though, but I feel like in production that's often not what you end up doing, so allowing the artist the freedom to publish data that has little by way of labeling attached to it namewise, to semantically distinguish it from others, would cause a lot of chaos. That's just my 2 cents on the matter, based on what I have seen. I'm sure it can be structured better, i'm curious to see if we can restructure this a bit better. |
Sasbom
left a comment
There was a problem hiding this comment.
As stated in my earlier comment,
The original intent was to enforce distinct naming for products, since they often can't be derived from the filename / task alone,
e.g. it seems common to generate multiple products and to export these.
Also, distinct naming helps with collisions, since if I create a node in the scene for publishing, I can only keep track of it if it has a unique name.
It can happen that due to some bugginess, an existing ayon save node of the same name might exist in the graph, and because the logic in JS matches against names and node types, creating a new node will indeed create a new node and product, but then not properly update the one with the correct name, instead making one with <nodename>1 (since node name overlap isn't allowed). Then you end up with 2 problems:
- The context set on the new node is correct but it expects to have a different name
- The old lingering node has semi-reentered the context but without the proper matching imprint info
Did you find this to get in the way? How do you use ComfyUI? I myself am only a very light user, really only for the development of this plugin, so I'm very interested in how you use it.
|
I will have to check back with the artist who was testing this, but from what I understand, we're trying to match the product names and overall user experience as close as possible to what users are already familiar with from other applications. our product names (in most cases) follow the template: Our initial idea here was to make Since you've mentioned the parallel with |
I see, thanks for explaining! @BigRoy Things seem fine on my end! |
Sasbom
left a comment
There was a problem hiding this comment.
After explanation, I understand the motivation behind this.
Having the 2 prefixes be optional would have things be as flexible as possible for more workflows.
|
just chiming in to let u know that i'm running this since yesterday morning and so far had no complaints :) |
Changelog Description
Allows
prefixandunique_nameto be left emptyAdditional review information
When
prefixand/orunique_namewas left empty it would create leading underscores in the filename.This PR adds a shared
get_full_product_namemethod to concatenate the product_name.It also updates the logic on the
publish_nodesside. If aprefixand/orunique_nameis provided it will still be used as the filename. If not, theproduct_namewill be used instead.in the following screenshot:
imageMain/imageMain_0001.pngayon_ayon_imageMain/ayon_ayon_0001.pngTesting notes: