Templated Builder: Options to allow profile to be disabled

[moonyuet]

Changelog Description

This PR is to add some more options to allow templated builder to be enabled in several scenarios:

1. Official Enabled
2. Template Always apply to empty scene (act as 'new file' template)
3. Apply on application launch if no workfile exists yet

Additional info

Resolve #1040
You can test with blender as starting point: ynput/ayon-blender#313

Testing notes:

1. Build workfile as followed in the host PR above

[BigRoy]

@iLLiCiTiT shall we discuss this code? Because I think you had bigger remarks to begin with?

[BigRoy]

Can we rename this apply_on_new_file?

[BigRoy]

This seems like an odd check for is_new_file? should this variable be is_app_launch_empty_file or whatever?

[iLLiCiTiT]

apply_to_empty_scene and apply_on_app_launch probably won't be available for old addons?

[iLLiCiTiT]

Suggested change

	apply_to_empty_scene: bool = preset["apply_to_empty_scene"]
	apply_to_empty_scene: bool = preset.get("apply_to_empty_scene", False)

[iLLiCiTiT]

Isn't this trying to resolve same thing as the other PR that did not raise an error if preset does not have file?

Right now we do expect that all values from get_template_preset are always returned and validated, at multiple places. As I said at the other PR, we have to refactor the logic to handle that correctly, we can't just return None at method that is expected to always return validated values without handling it at all.

When we'd do that I'd rather do it properly.

[iLLiCiTiT]

Suggested change

	apply_on_app_launch: bool = preset["apply_on_app_launch"]
	apply_on_app_launch: bool = preset.get("apply_on_app_launch", False)

[iLLiCiTiT]

I think apply_on_app_launch is something this method can't resolve. It probably is duplicate of workfile_creation_enabled.

[iLLiCiTiT]

Idea:
"apply_to_empty_scene" and "apply_on_app_launch" really don't make sense (from my point of view) to be resolved within the logic of workfile builder.

Rather store the profile to the output, and use correct arguments in build_template based on host logic:

• apply_to_empty_scene is duplicate of workfile_creation_enabled.
• apply_on_app_launch cannot be resolved in any way shape or form in workfile builder logic, it is used only for the import (again, duplicate of workfile_creation_enabled).

return {
    "path": resolved_path,
    "keep_placeholder": keep_placeholder,
    "create_first_version": create_first_version,
    "profile": profile,
}

Then you can do

# Somehow find out
just_lauched = ...

host = registered_host()
builder = TemplateBuilder(host)

preset = builder.get_template_preset()
profile = preset["profile"]
is_new_workfile = False
is_new_file = not self.host.get_current_workfile()
if just_lauched:
    is_new_workfile = profile["apply_to_empty_scene"]
elif is_new_file:
    is_new_workfile = profile["apply_on_app_launch"]
    
builder.build_template(
    template_path=preset["path"],
    keep_placeholder=preset["keep_placeholder"],
    create_first_version=preset["create_first_version"],
    workfile_creation_enabled=is_new_file,
)

I know this means the code has to be duplicated in each host that would want to do that, but this is the least affecting invasive code changes to avoid being even more blocked with future enhancements. Both values do affect the workfile template builder, but are not really related to the workfile builder. And yes we do have already the is_new_file logic, but why to add more to the pile.

[moonyuet]

I have two questions with this:

1. Isn't that we have discussed with @BigRoy about using dataclass for the return value? Or we dont need this now(Kinda offtrack though)
2. Aren't keep_placeholder and create_first_version already part of the profiles? So, I believe apply_to_empty_scene and apply_on_app_launch should be part of the profile settings?

[iLLiCiTiT]

So, I believe apply_to_empty_scene and apply_on_app_launch should be part of the profile settings?

They can be part of the profile, but it doesn't make sense to have them as part of the workfile build preset. The workfile builder has nothing to do with the values. It is additional values that describe context before the workfile builder is being used.

The point is that the workfile template builder really needs more refactoring and adding more stuff that is at the end not used by the builder will only complicate future possible enhancements.

Apply to empty scene and on application launch cannot be handled by the workfile builder, it even should not be handled by the builder, you as someone who explicitly do trigger the builder know it, and you should pass that information to the build method, for both options and cases there already is argument that does handle that as far as I understand. If the same method is triggered manually from action the values from the preset should have no value, because at that moment it is not triggered on launch, or on empty workfile, and it should not apply the same logic (you can still trigger the builder manually).

[LiborBatek]

The Create First Workfile still fails to save the workfile as the very first version but the rest of the options works fine...

[moonyuet]

The Create First Workfile still fails to save the workfile as the very first version but the rest of the options works fine...

Fixed in bcb9413

[LiborBatek]

The menu action works and saves the first workfile...

but the addons default option for first workfile creation fails still

[LiborBatek]

Works well together with the ayon-blender #313 and also been tested togehter with Web Actions from Ayon frontend succesfully!

LGTM