Animatect / Prism2_PluginFusion

Fusion plugin for the Prism Pipeline Framework
1 stars 1 forks source link

Fix issue with Media Identifier naming #15

Closed AltaArts closed 2 weeks ago

AltaArts commented 4 weeks ago

Since Fusion has strict naming conventions, we need to ensure that a user supplied Identifier name is handled.

An example is that if the user has a dash in the Identifier name, then the saver will be auto-renamed to an underscore by Fusion. This then breaks the binding of the Render State and the Saver node.

Maybe add code to change "illegal" characters to a placeholder that can then be replaced.

Animatect commented 4 weeks ago

Do you want to take care of this one?, if not just let me know and I'll try to tackle it after figuring out what is making update masters fail

AltaArts commented 3 weeks ago

Either way. I think I am going to port the thumbnail next.

So I think whoever gets to it and puts it over to "In progress".

AltaArts commented 3 weeks ago

Ok, so added name checking for the Media Identifier name. It takes the entered text str from the dialogue and checks if it is "legal", and will return out if it is not.

By "legal" it has to:

So a question is this too limiting for workflows? For example others may need a MediaID to begin with a number.

And if the name is legal and has a hyphen for example (which is illegal in Fusion naming), it will put a placeholder "xHx" in there. It is ugly, but the placeholder has to be Fusion legal as well which is limiting. This all works, but the question would be the UI/UX.

Right now, the StateManager RenderNode name display (green/red box) shows the exact same name as the Saver node that is added - that makes sense and is technically super correct. But then if a MediaID is something like "TEST-Rec709-24" it will be converted to "TEST_xHx_Rec709_xHx_24" and shown in both the SM RenderNode name and the Saver. Is that good UX?

I think there are a couple of options I can think of:

What ya think?

Animatect commented 3 weeks ago

Ok, so a lot to unpack here :) I think the most important thing here is that what we do with this name theoretically wont be traced back at any point to a file name or something that could break, assuming that I think the UX is good enough as you are propossing.

I thought about using an UUID at some point and this seems actually to be a good idea we can implement it using node.GetData("UUID", ) and mirroring that ID in a class variable for the state. The only thing that gave me pause was that retrieving the data was just a tiny bit mode cumbersome, but I think it would be the right call.

Do you agree?

Animatect commented 3 weeks ago

@AltaArts I'm going to merge this but maybe we should make a ticket to discuss if it would be convenient in the future to use a UUID, for the time being it works.

AltaArts commented 3 weeks ago

I cannot remember off hand, but this may have broken the importImages versioning. I mean the part where if you want to update the version of the imported images it will not work. This is because I changed the naming scheme of the saved images and I did not realize the versioning relays on the naming convention.

But we can chat about it to brainstorm.

Animatect commented 3 weeks ago

ok, I'm going to check again before merging.