thedyze / save-image-extended-comfyui

Extended Save node for ComfyUI
82 stars 6 forks source link

Older saves not compatible #6

Closed AlUlkesh closed 11 months ago

AlUlkesh commented 11 months ago

When I load an image, that was saved before this commit: https://github.com/thedyze/save-image-extended-comfyui/commit/d712114afa75dd61c9dde6c9cb896e8590212f6b

the fields are not properly aligned to their values.

For example, if I have an image which had this setting before that commit:

image

and load it with the current commit I get this:

image

aegis72 commented 11 months ago

Thanks! I'll be sure not to update before it's resolved. Thanks for pointing this one out; took a second to see it but yeah, the new 'job custom text" field (a cool addition) seems to be the culprit, pushing fields/values around

thedyze commented 11 months ago

I think this might have to do with how comfy loads nodes. If you remove the node and re add it, does it resolve your issue?

AlUlkesh commented 11 months ago

Well, yes, but then everything is set to default and the saved info is lost.

I think it has something to do with this:

https://github.com/thedyze/save-image-extended-comfyui/blob/d712114afa75dd61c9dde6c9cb896e8590212f6b/save_image_extended.py#L260-L278

Since most of these are positional parameters, adding new ones in the middle messes up the handling of older files.

thedyze commented 11 months ago

That might be it yeah. Going forward I don't see myself adding much more parameters, but I'll keep that in mind.

Though if new params are added, they should preferably be grouped with other params belonging in to same category. I do think the lost settings of the parameters is really not that hard to replace by hand?

AlUlkesh commented 11 months ago

I do think the lost settings of the parameters is really not that hard to replace by hand?

Well, the issue is not tragic, but certainly annoying.

I looked a bit more into the ComfyUI code and it's actually this part that determines the load order:

https://github.com/thedyze/save-image-extended-comfyui/blob/d712114afa75dd61c9dde6c9cb896e8590212f6b/save_image_extended.py#L24-L46

So, yes, since this is a ComfyUI limitation, I don't think we can do much about it.

Perhaps I'll write a converter for the metadata inside the older images, when I get around to it.