-
Notifications
You must be signed in to change notification settings - Fork 87
Templated Builder: Options to allow profile to be disabled #1907
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 2 commits
453dc01
1342660
a429ae1
380b321
d3a3e3a
9d8d998
fcbbf49
b52cbf9
8fb9955
cfdc41f
bc10f94
b7451d9
8553813
f92bf17
74369ed
50fed12
6da9537
1e69afc
91a949c
e3b3ea4
bff9133
17a3c8f
ada3de8
73315d6
8264473
03bd696
e4e39b7
8bb2ff4
4f234e3
584ed01
ff7b4a1
3783033
f0443d9
e236dd2
f3eb885
97c8b61
82f2006
385d0bf
4ccf4d5
bcb9413
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -511,6 +511,8 @@ def build_template( | |||||
| level_limit=None, | ||||||
| keep_placeholders=None, | ||||||
| create_first_version=None, | ||||||
| apply_to_empty_scene=None, | ||||||
| apply_on_app_launch=None, | ||||||
| workfile_creation_enabled=False | ||||||
| ): | ||||||
|
moonyuet marked this conversation as resolved.
|
||||||
| """Main callback for building workfile from template path. | ||||||
|
|
@@ -532,6 +534,10 @@ def build_template( | |||||
| When set to True, this option initiates the saving of the | ||||||
| workfile for an initial version. It will skip saving if | ||||||
| a version already exists. | ||||||
| apply_to_empty_scene (bool): Whether to apply the template | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we rename this |
||||||
| to an empty scene(Act as 'new file' template). | ||||||
| apply_on_app_launch (bool): Whether to apply the template on | ||||||
| application launch if no workfile exists yet. | ||||||
| workfile_creation_enabled (bool): Whether the call is part of | ||||||
| creating a new workfile. | ||||||
| When True, we only build if the current file is not | ||||||
|
|
@@ -559,15 +565,22 @@ def build_template( | |||||
| keep_placeholders: bool = preset["keep_placeholder"] | ||||||
| if create_first_version is None: | ||||||
| create_first_version: bool = preset["create_first_version"] | ||||||
| if apply_to_empty_scene is None: | ||||||
| apply_to_empty_scene: bool = preset["apply_to_empty_scene"] | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| if apply_on_app_launch is None: | ||||||
| apply_on_app_launch: bool = preset["apply_on_app_launch"] | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| # Build the template if we are explicitly requesting it or if it's | ||||||
| # an unsaved "new file". | ||||||
| is_new_file = not self.host.get_current_workfile() | ||||||
| if is_new_file or explicit_build_requested: | ||||||
| if ( | ||||||
| apply_to_empty_scene | ||||||
| or explicit_build_requested | ||||||
| or (apply_on_app_launch and is_new_file) | ||||||
| ): | ||||||
| self.log.info(f"Building the workfile template: {template_path}") | ||||||
|
moonyuet marked this conversation as resolved.
|
||||||
| self.import_template(template_path) | ||||||
| self.populate_scene_placeholders( | ||||||
| level_limit, keep_placeholders) | ||||||
| self.populate_scene_placeholders(level_limit, keep_placeholders) | ||||||
|
|
||||||
| # Do not consider saving a first workfile version, if this is not set | ||||||
| # to be a "workfile creation" or `create_first_version` is disabled. | ||||||
|
|
@@ -863,6 +876,10 @@ def get_template_preset(self): | |||||
| filter_data, | ||||||
| logger=self.log | ||||||
| ) | ||||||
| if not profile.get("enabled", True): | ||||||
| self.log.info("Template profile is disabled for current context.") | ||||||
| return | ||||||
|
|
||||||
| if not profile: | ||||||
| raise TemplateProfileNotFound(( | ||||||
|
moonyuet marked this conversation as resolved.
Outdated
|
||||||
| "No matching profile found for task '{}' of type '{}' " | ||||||
|
|
@@ -890,6 +907,8 @@ def get_template_preset(self): | |||||
| # switch to remove placeholders after they are used | ||||||
| keep_placeholder = profile.get("keep_placeholder") | ||||||
| create_first_version = profile.get("create_first_version") | ||||||
|
moonyuet marked this conversation as resolved.
|
||||||
| apply_to_empty_scene = profile.get("apply_to_empty_scene", True) | ||||||
| apply_on_app_launch = profile.get("apply_on_app_launch", True) | ||||||
|
|
||||||
| # backward compatibility, since default is True | ||||||
| if keep_placeholder is None: | ||||||
|
|
@@ -898,7 +917,9 @@ def get_template_preset(self): | |||||
| return { | ||||||
| "path": resolved_path, | ||||||
|
moonyuet marked this conversation as resolved.
Outdated
|
||||||
| "keep_placeholder": keep_placeholder, | ||||||
| "create_first_version": create_first_version | ||||||
| "create_first_version": create_first_version, | ||||||
| "apply_to_empty_scene": apply_to_empty_scene, | ||||||
| "apply_on_app_launch": apply_on_app_launch, | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Idea: Rather store the
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
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have two questions with this:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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). |
||||||
| } | ||||||
|
|
||||||
|
moonyuet marked this conversation as resolved.
|
||||||
| def resolve_template_path(self, path, fill_data=None) -> str: | ||||||
|
|
||||||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
apply_on_app_launchis something this method can't resolve. It probably is duplicate ofworkfile_creation_enabled.