Skip to content
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

Add support to run the workfile template builder on startup and simplify resolve houdini path #74

Merged
merged 8 commits into from
Sep 10, 2024

Conversation

fabiaserra
Copy link

@fabiaserra fabiaserra commented Aug 21, 2024

Changelog Description

The workfile template builder wasn't running automatically when starting up Houdini, you had to go click the button Build Workfile from template in order to trigger it instead. This PR adds the callback on startup so it automatically creates a first version of a workfile with the templated scene.

This also refactors the usage of hou.text.expandString to expand Houdini variables in the template path. It requires ynput/ayon-core#875 but will work backwards compatible without that (but it will then just not resolve).

It also fixes the bug where hou.text.expandString would remove the double backslashes from a UNC path by escaping backslashes.

Additional info

n/a

Testing notes:

  1. Create a profile on the Templated workfile build setting on ayon-houdini settings with Create first version enabled and a path to a template scene file
  2. Start Houdini on a new context where no workfiles have been created yet
  3. A new workfile scene should be created based on the templated scene

@BigRoy BigRoy added the type: enhancement Improvement of existing functionality or minor addition label Aug 27, 2024
@BigRoy BigRoy assigned fabiaserra and unassigned MustafaJafar Sep 2, 2024
Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, I gave this a test from user perspective.
Here I tested some cases:

  • Launch Houdini as usual. I've few workfiles. (Build skipped)
    image
  • Launch Houdini while skipping opening last workfile. (Build skipped)
    image
  • Launch Houdini (without having any last files) and create first version is enabled. (Build Done)
    image
  • Launch Houdini (without having any last files) and create first version is disabled. (didn't build.) (Build didn't continue)
    image

@BigRoy
Copy link
Contributor

BigRoy commented Sep 4, 2024

  1. Launch Houdini as usual. I've few workfiles. (Build skipped)
  2. Launch Houdini while skipping opening last workfile. (Build skipped)
  3. Launch Houdini (without having any last files) and create first version is enabled. (Build Done)
  4. Launch Houdini (without having any last files) and create first version is disabled. (didn't build.) (Build didn't continue)

@fabiaserra could you point out - from that list above what your expected behavior would be for each?

I'd assume that you'd expect 2) to also still "build", yes?

@fabiaserra
Copy link
Author

fabiaserra commented Sep 4, 2024

  1. Launch Houdini as usual. I've few workfiles. (Build skipped)
  2. Launch Houdini while skipping opening last workfile. (Build skipped)
  3. Launch Houdini (without having any last files) and create first version is enabled. (Build Done)
  4. Launch Houdini (without having any last files) and create first version is disabled. (didn't build.) (Build didn't continue)

@fabiaserra could you point out - from that list above what your expected behavior would be for each?

I'd assume that you'd expect 2) to also still "build", yes?

Yeah otherwise an artist could be starting a new workfile (even if there's existing ones) without being based on the template scene and we want to reduce the chances of that happening... Although the phrasing of your test cases is a bit confusing as the first two is not clear whether Create first version is enabled or disabled?

This phrasing is more clear:

  1. Create First Version: True and Open last workfile: True
    a. Launch with NO existing workfiles
    b. Launch with existing workfiles

  2. Create First Version: True and Open last workfile: False
    a. Launch with NO existing workfiles
    b. Launch with existing workfiles

  3. Create First Version: False and Open last workfile: True
    a. Launch with NO existing workfiles
    b. Launch with existing workfiles

  4. Create First Version: False and Open last workfile: False
    a. Launch with NO existing workfiles
    b. Launch with existing workfiles

Having that said, I would expect the following:

  1. Create First Version: True and Open last workfile: True
    a. Launch with NO existing workfiles -> NO workfile template build
    b. Launch with existing workfiles -> NO workfile template build

  2. Create First Version: True and Open last workfile: False
    a. Launch with NO existing workfiles -> Workfile template build
    b. Launch with existing workfiles -> Workfile template build

  3. Create First Version: False and Open last workfile: True
    a. Launch with NO existing workfiles -> NO workfile template build
    b. Launch with existing workfiles -> NO workfile template build

  4. Create First Version: False and Open last workfile: False
    a. Launch with NO existing workfiles -> NO workfile template build
    b. Launch with existing workfiles -> NO workfile template build

@BigRoy
Copy link
Contributor

BigRoy commented Sep 4, 2024

  1. Create First Version: True and Open last workfile: True
    a. Launch with NO existing workfiles -> NO workfile template build

This should launch WITH workfile template build right? Because it would initialize to empty scene otherwise?

@fabiaserra
Copy link
Author

This should launch WITH workfile template build right? Because it would initialize to empty scene otherwise?

You are right yeah, let me update it

Create First Version: True and Open last workfile: True
a. Launch with NO existing workfiles -> Workfile template build
b. Launch with existing workfiles -> NO workfile template build

Create First Version: True and Open last workfile: False
a. Launch with NO existing workfiles -> Workfile template build
b. Launch with existing workfiles -> Workfile template build

Create First Version: False and Open last workfile: True
a. Launch with NO existing workfiles -> NO workfile template build
b. Launch with existing workfiles -> NO workfile template build

Create First Version: False and Open last workfile: False
a. Launch with NO existing workfiles -> NO workfile template build
b. Launch with existing workfiles -> NO workfile template build

@BigRoy
Copy link
Contributor

BigRoy commented Sep 4, 2024

Great, so in essence:

Create First Version: True

Basically, whenever an "empty scene" would initialize in Houdini - then use the template

  1. Open last workfile: True
    a. Launch with NO existing workfiles -> Workfile template build ✅
    b. Launch with existing workfiles -> NO workfile template build, because it opens the existing workfile. 🔴

  2. Open last workfile: False
    a. Launch with NO existing workfiles -> Workfile template build ✅
    b. Launch with existing workfiles -> Workfile template build ✅

When doing "New scene" (e.g. CTRL+N), run the Workfile Template Build ✅

Create First Version: False
Never build workfile template

@MustafaJafar MustafaJafar added the community Issues and PRs coming from the community members label Sep 5, 2024
@MustafaJafar
Copy link
Contributor

MustafaJafar commented Sep 5, 2024

Hey,
I'm back with more test runs... I've checked the code that @fabiaserra mentioned in the PR description the code logic here on ayon-core.

As well as the test cases summarized by @BigRoy here #74 (comment)


anyways, I did further testing and was able to reproduce the the provided cases by using this PR and (this core PR ynput/ayon-core#871)
Here's my summary:

cases:
I'm here exploring the values of different variables in side the method. to find out the proper combination to achieve the provided cases.

  1. Launch with existing workfiles:
    a. Skip Opening last workfile: False (default)
    b. Skip Opening last workfile: True
    both cases:
    create_first_version: True
    workfile_creation_enabled: True
    created_version_workfile: False

  2. Launch with NO existing workfiles:
    a. Skip Opening last workfile: False (default)
    b. Skip Opening last workfile: True
    both cases:
    create_first_version: True
    workfile_creation_enabled: True
    created_version_workfile: \storage\work\ayon_projects\Robo\Assets\Character\robococo\work\cfx\robo_robococo_cfx_v001.hip

Test Runs:
1.a
create_first_version: True
workfile_creation_enabled: True
created_version_workfile: False
current_workfile: //storage/work/ayon_projects/Robo/Assets/Character/robopopo/work/cfx/robo_robopopo_cfx_v001.hip

1.b
create_first_version: True
workfile_creation_enabled: True
created_version_workfile: False
current_workfile: None

2.a and 2.b
create_first_version: True
workfile_creation_enabled: True
created_version_workfile: \storage\work\ayon_projects\Robo\Assets\Character\robococo\work\cfx\robo_robococo_cfx_v001.hip
current_workfile: //storage/work/ayon_projects/Robo/Assets/Character/robococo/work/cfx/robo_robococo_cfx_v001.hip

  1. Pressing Ctrl+ntriggers the build but it always boils down to the same cases above:
    a. scene untitled and there's existing workfiles -> 1.b
    b. scene is NOT untitled and there's existing workfiles -> 1.b
    c. scene untitled and there's No existing workfile -> 2.a and 2.b

@fabiaserra
Copy link
Author

Hey, I'm back with more test runs... I've checked the code that @fabiaserra mentioned in the PR description the code logic here on ayon-core.

My questions regarding that code of ayon-core where a bit more in terms of the flow of the function making it a bit hard to read, something like this makes more sense to me:

def build_template(
    self,
    template_path=None,
    level_limit=None,
    keep_placeholders=None,
    create_first_version=None,
    workfile_creation_enabled=False
):
    # Get default values if not provided
    if template_path is None or keep_placeholders is None or create_first_version is None:
        preset = self.get_template_preset()
        template_path = template_path or preset["path"]
        keep_placeholders = keep_placeholders if keep_placeholders is not None else preset["keep_placeholder"]
        create_first_version = create_first_version if create_first_version is not None else preset["create_first_version"]

    created_version_workfile = self.create_first_workfile_version() if create_first_version else False

    # Handle workfile creation if enabled and a new version was created
    if workfile_creation_enabled and created_version_workfile:
        self.import_template(template_path)
        self.populate_scene_placeholders(level_limit, keep_placeholders)
        self.save_workfile(created_version_workfile)
        return

    # If workfile creation is not enabled or no new version was created,
    # proceed with template import and population
    if not workfile_creation_enabled:
        self.import_template(template_path)
        self.populate_scene_placeholders(level_limit, keep_placeholders)

@MustafaJafar
Copy link
Contributor

My questions regarding that code of ayon-core where a bit more in terms of the flow of the function making it a bit hard to read, something like this makes more sense to me:

tbh, I'm not sure how to fix that in my PR ynput/ayon-core#871
also, keep in mind that I tried different combinations till I found something that fulfills the cases mentioned by BigRoy.

@fabiaserra
Copy link
Author

tbh, I'm not sure how to fix that in my PR ynput/ayon-core#871 also, keep in mind that I tried different combinations till I found something that fulfills the cases mentioned by BigRoy.

What results are you getting with the code that I shared?

To be honest, in my opinion it even makes sense to separate the Create first version toggle and the Workfile build template being enabled (although that could be implicit if a profile is set or an actual template is found). On that case... this condition would no longer be true:

Create First Version: False
Never build workfile template

And so even if a workfile version isn't created and saved... the template scene is populated, which I think aligns with the code that I shared

@BigRoy
Copy link
Contributor

BigRoy commented Sep 5, 2024

Create First Version: False Never build workfile template

And so even if a workfile version isn't created and saved... the template scene is populated, which I think aligns with the code that I shared

Hmm yes - I think I may have misunderstood the system or whatever. To me, dumbed down it may be best described as:

  1. Open last workfile: When enabled, open the last workfile if it exists.
  2. New scene workfile templates (workfile_creation_enabled?): When enabled, if a new scene is opened or host starts without opening an existing workfile then load/build a template as starting point
  3. Create first version (create_first_version): If there are no existing workfiles, initialize my workfile as v001 and save it directly. This may include the workfile template if that was enabled and built. But it may also just be an empty scene?

Tagging @dee-ynput @antirotor for their opinions.

Also, Pressing New Scene (CTRL+N) would never 'save'. The create first version is only upon host application launch. (Which should technically never happen anyway, because if Create first version (as I described it here) is enabled it would've already generated it at launch).

If create_first_version also allows to save a first version WITHOUT having a template enabled then the code would just be:

def build_template(
    self,
    template_path=None,
    level_limit=None,
    keep_placeholders=None,
    create_first_version=None,
    workfile_creation_enabled=False
):
    # NOTE: This function should NOT run when a last workfile was opened. It should only trigger
    #   on initializing a host to a new scene or on pressing CTRL+N (new file). Pressing CTRL+N
    #   should enforce `create_first_version` to False
    # Get default values if not provided
    if template_path is None or keep_placeholders is None or create_first_version is None:
        preset = self.get_template_preset()
        template_path = template_path or preset["path"]
        keep_placeholders = keep_placeholders if keep_placeholders is not None else preset["keep_placeholder"]
        create_first_version = create_first_version if create_first_version is not None else preset["create_first_version"]

    # Handle workfile creation if enabled, proceed with template import and population
    if workfile_creation_enabled:
        self.import_template(template_path)
        self.populate_scene_placeholders(level_limit, keep_placeholders)

    # Save first workfile version
    if create_first_version:
        self.create_first_workfile_version()

@antirotor
Copy link
Member

Thanks @BigRoy for the summary - I think it is exactly like that. Might be worth taking it and put it to the documentation

Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't go deep through the code.
But, it works on my side.

For reference, I was using this PR all the time while testing ynput/ayon-core#871
since this PR allowed running the template builder by default when opening a new scene.
However, we need the core PR to make it work with empty scenes or with the first workfile created by enabling create_first_version.
since it creates the new file before the building step.


What I like the most about this PR and the core PR. is that they contain good info for documenting the tool.

@MustafaJafar MustafaJafar linked an issue Sep 6, 2024 that may be closed by this pull request
2 tasks
Copy link
Contributor

@BigRoy BigRoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mustafa approved me, live... watching me.. it was scary!

@BigRoy BigRoy changed the title Add support to run the workfile template builder on startup Add support to run the workfile template builder on startup and simplify resolve houdini path Sep 6, 2024
Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made few test runs.
The PR still works as expected.

@BigRoy BigRoy requested a review from MustafaJafar September 9, 2024 14:25
@BigRoy
Copy link
Contributor

BigRoy commented Sep 9, 2024

Updated the path template resolving to match an update in the core PR. Would be good to test again @MustafaJafar

@iLLiCiTiT
Copy link
Member

If it is dependent on ynput/ayon-core#875 then it should bump required ayon-core to >0.4.4 in package.py .

@BigRoy
Copy link
Contributor

BigRoy commented Sep 10, 2024

If it is dependent on ynput/ayon-core#875 then it should bump required ayon-core to >0.4.4 in package.py .

It is not entirely dependent - because without core 0.4.4+ it would just not trigger the resolve template. It wouldn't crash necessarily, just the feature would do nothing... which may be unwanted side effect since there would be no warning for that either.

So, shall I pin it to >0.4.4 @MustafaJafar @iLLiCiTiT ?

@MustafaJafar
Copy link
Contributor

MustafaJafar commented Sep 10, 2024

It is not entirely dependent - because without core 0.4.4+ it would just not trigger the resolve template. It wouldn't crash necessarily, just the feature would do nothing... which may be unwanted side effect since there would be no warning for that either.

So, shall I pin it to >0.4.4 @MustafaJafar @iLLiCiTiT ?

imo. it's not a necessary. because it'll work perfectly fine with regular paths.
However, supporting Houdini environment in the paths will be 0.4.4+

Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works on my side as expected.
Tested along with ynput/ayon-core#871

@BigRoy BigRoy merged commit 4b1c0ef into ynput:develop Sep 10, 2024
1 check passed
@BigRoy
Copy link
Contributor

BigRoy commented Sep 11, 2024

Just wanted to link this other PR ynput/OpenPype#6286 here which uses some function naming that I think is much nicer. It's all in Maya - which isn't preferred - at best we can make that DCC-agnostic and trigger it somehow in core without having to duplicate the idea everywhere. But, as said.. just wanted to link it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Issues and PRs coming from the community members type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UNC paths don't work on Houdini template paths
5 participants