Generic Loader OTLs #121

MustafaJafar · 2024-10-14T12:04:27Z

Changelog Description

Hopefully, resolve #118
This PR mixes our solutions for LOPLoadAssetLoader and FilePathLoader resulting in the new generic loader otls.

The Generic Load OTL provides the following parameter UI to select an AYON product.

The filepath loader is now using the generic loader instead of a null node.

The generic Load OTL can be access via TAB menu. it comes with default parameter values (no products are selected at this moment).

Users can also find a parameter action Load with AYON when right-click any parameter of type file on any non Rop nodes.

When using Load with AYON action, a parameter editor pane will pop up, where artists can easily modify the parameters.

The generic Load OTL can also show you the thumbnail of the loaded product

The Generic Load OTL features:

Users can load & manage the filepath of any product.
The OTL supports frame and UDIM tokens.
Users can create the nodes via the TAB menu in the support contexts.
Make it easy for artists to access/use the generic loader.
Parameter expressions instead of parameter callbacks

Support contexts:

Object
SOPs
LOPs
...

Additional Info

This PR extends our solution for LOPLoadAssetLoader and adds representation list so that artist can easily pick the representation to load.

Additional Info 2 - Why using an HDA

An alternative solution to the Generic Loader OTL is to create a null node and add the parameters dynamically.
But, that solution requires additional work to update nodes in any old scenes.
Unlike using OTLs, OTLs make it easier to update the nodes automatically when opening the scenes.

Additional Info 3 - Promoting the Generic Loader parameters

When building a custom HDA that uses the generic loader, the recommended way to promote the generic loader parameters is by creating the parameters from nodes.

But, you may hit an issue like this one if you promote a parameter with action, e.g. product parameter.

This issue is much similar when promoting some parameters (that have action callback) of the group sop node.

The issue is discussed in SOP subnet parameter promotion issue | SideFx Forums
and let me quote the easiest suggested solution:

The simplest is usually to promote the other parameters referenced by any such scripts as well,
making them invisible if they serve no other purpose on the promoted node. 
This is the easiest way to ensure the script is running similarly to how it does on the original node.

Here's a demo:
I've project and folder_path parameters hidden, but they need to exist because the action script on product expects them to be found on the node.

Additional Info 4 - Expressions are so powerful

After using expressions instead of callbacks, This unlocked some powerful features.

e.g. I've these 3 products with different numbers

I can load them in a for loop. and it works either or not I put my nodes inside a custom HDA.

Testing notes:

Launch Houdini via AYON Launcher
Try different load options:

Create the loader via tab menu.
right-click file parameter on file nodes to connect it to a generic loader.
use the loader tool to load using generic loader.

Try loading different products (usd, caches, image sequences, textures with/without udims)

BigRoy · 2024-10-14T12:32:07Z

Nice!

HDA maintenance

I really dislike the fact that we now need to maintain multiple separate HDA definitions for something that is (or should essentially be) a node that has no uniqueness in behavior across the different contexts. It's just a node that takes input parameters to define the result on another parameter.

It's quite cumbersome to update the HDA definitions if we need to do this per context - not impossible, but very annoying. This is just two contexts - but there may be way more.

Maybe someone else knows how to better manage this or make the one HDA be able to be used across the different contexts without needing to handle the parameter configuration for each one separately.

Let's 'figure that out' but for now hold off creating all the other ones, because it'll just mean more maintenance. Let's continue the work on these two - and then just keep 'checking' how much effort this really is to maintain.

UDIM path

Currently it'll return the resolved filepath - but there are some cases where we need it to resolve to <UDIM> token. See here. Not required for MVP for this - but definitely high priority after.

So just noting that it may be on the todo list.

Generic Loader in Loader UI

In your screenshot it's ordered at the top, let's move it down. It's the "last resort" type of loader, not the primary one to use. This can be configured with the order attribute on the Loader Plug-in.

Houdini native focus

Loading from the Loader UI is fine - but our main focus with this HDA is:

Creating and maintaning the nodes directly in Houdini via the tab menu. So I'd say the testing notes should include that: "Create the nodes via the TAB menu in the support contexts. It should be very comfortable to create the node and configure it. It should be trivial for the user using JUST the node to configure it to load whatever they want quickly and comfortably."
- This may spawn off into a separate issue and future PRs, but our goal is to make the loading experience VERY NICE with the parameters on this node and allowing anyone to have just as nice of an input for custom HDAs that then contain this HDA. So I want to make sure we keep evaluating it against that requirement.
Create the best building block for studios to "load anything" via AYON inside their own custom HDAs. The idea is that they embed this node INSIDE their HDAs - and they just expose the relevant parameters. I'm quite sure that it'll likely stop working when embedded like that inside a custom HDA. We need to identify if that's the case, and if so, define what's needed to fix that. E.g. currently we use parameter callbacks to update OTHER parameters, however - if these parameters are locked away INSIDE the HDA and are not editable... these callbacks will most likely fail because they are not allowed to set the parameters. I want to avoid that the custom HDA needs to keep these nodes 'editable' or whatever so we should look into ways into making this work, trivially without customization. Just embedding the HDA and exposing the parameters would be the goal. (We may need to replace the parameter callbacks with expressions, similar to Load LOPs: Support update via context options (replace parm callbacks with expressions) #112)
And potentially, make it very easy to load a filepath to any parameter. For example: What I'd want is for anyone clicking on a string or file parameter to have a context menu entry, like "Load path with AYON" - which then automatically generates the node (this can just be in AVALON_CONTAINERS and then directly adds a 'reference' to that node's filepath parameter on the attribute the user clicked on.) That way, you don't even need to create the node manually. You can just right click anywhere to set up a managed AYON path?

MustafaJafar · 2024-10-14T13:11:04Z

HDA maintenance

I really dislike the fact that we now need to maintain multiple separate HDA definitions for something that is (or should essentially be) a node that has no uniqueness in behavior across the different contexts. It's just a node that takes input parameters to define the result on another parameter.

It's quite cumbersome to update the HDA definitions if we need to do this per context - not impossible, but very annoying. This is just two contexts - but there may be way more.

Yeah, I think our case should be similar to the subnet nodes. since HDAs are built on top of subnets.
In asset manager, I can find multiple definitions of subnet, one for each Houdini context.
I hope there's a solution for this.

MustafaJafar · 2024-10-17T18:36:48Z

UDIM path

Currently it'll return the resolved filepath - but there are some cases where we need it to resolve to <UDIM> token. See here. Not required for MVP for this - but definitely high priority after.

So just noting that it may be on the todo list.

I've ported the logic from the filepath loader. we may clean it later/move the logic to lib so that both file loader and hda_utils can share it.

MustafaJafar · 2024-10-17T19:55:40Z

With the current state, I created an HDA with the loader node inside.
I copied the parameters from the generic loader and I needed to do the following:

remove callbacks.
~~change the menu script in version and representation parameters to import the functions instead of using hou.phm().~~ no need for that bccc305

For reference, loading objects from different projects leads to this result. which I think is not related to this PR.

MustafaJafar · 2024-10-18T00:29:41Z

I was able to use the generic loader in a for loop to load multiple products with the same name and different postfix number (although, I had to use a python node to change the product name and trigger the callback).
I don't know how to make it easy like #112 (comment)

BigRoy · 2024-10-18T06:56:51Z

Thanks - I see why this commit bccc305 would fix that. However, I'd love actually for somehow the menu script to stay attached to the 'inner' node in an HDA instead of also promoted.

Because, that way - one wouldn't need to promote ALL relevant attributes to the parent HDA but could even (for whatever reason) promote e.g. only "Version" or whatever. The question becomes - how do you do that?

In this case, how did you promote the attributes to the parent HDA? Did you copy/paste the parms? Or did you promote them upwards (which would then actually be referencing the inner parms I'd imagine?)

Because we're now calling it on the "outer node" but it'd be great if we can keep all the interactions to the inner node - keeping the logic on the Generic Loader node itself.

MustafaJafar · 2024-10-18T08:42:33Z

In this case, how did you promote the attributes to the parent HDA? Did you copy/paste the parms? Or did you promote them upwards (which would then actually be referencing the inner parms I'd imagine?)

Yes, I copied/pasted these parms to the parent node and then copied them by reference to the inner node.
Cool thing that the parameter actions work fine.

This commit bccc305 allows me to copy version and representation parms without errors.

e.g. I copied these parms to a null node and removed their callback actions.

BigRoy · 2024-10-18T09:18:11Z

This commit bccc305 allows me to copy version and representation parms without errors.

Correct - but it basically means the logic runs on that node. Which is what we'd like to avoid. We want it to run on the inner node preferably so that it's still contained all as logic there. Because now you need ALL those attributes exposed on the parent HDA. I'd prefer it it could also work if e.g. we ONLY expose e.g. the representations attribute.

The question is - how can we easily make it do that without needing to be super-specific when making HDAs to remap those callbacks.

MustafaJafar · 2024-10-18T22:17:29Z

Here's a demo for my last two commits. (I wish if I were able to avoid the noise in my street :" )

2024-10-19.01-06-55.mp4

MustafaJafar · 2024-10-18T22:19:44Z

Because now you need ALL those attributes exposed on the parent HDA. I'd prefer it it could also work if e.g. we ONLY expose e.g. the representations attribute.

Good catch.
my custom HDA still works eve if I only expose the representation.
But the menu button doesn't work anymore.

I made another test. my custom HDA has its own representation menu with hard corded values abc, usd and exr.
My custom HDA still works.
tbh, I'm not yet sure how to expose/mirror the generic loader parameters on the parent HDA while relying on the generic loader to run the logic. should it be the responsibility of the parent HDA creator? I'll keep digging around finding ideas to make it easier to expose these parameters.

Another test, only exposing representation. I modified the menu script where I passed the loader node.
The logic doesn't run in the loader node. but, it uses the actual node with the hda_utils function where I get the same results as if I'm clicking the menu button in the generic loader node.

from ayon_houdini.api.hda_utils import get_available_representations

node = hou.node(f'./generic_loader')
representations_names = get_available_representations(node)

result = []
for name in representations_names:
    result.append(name)
    result.append(name)
    
return result

BigRoy · 2024-10-20T21:49:13Z

I made another test. my custom HDA has its own representation menu with hard corded values abc, usd and exr.
My custom HDA still works.
tbh, I'm not yet sure how to expose/mirror the generic loader parameters on the parent HDA while relying on the generic loader to run the logic. should it be the responsibility of the parent HDA creator? I'll keep digging around finding ideas to make it easier to expose these parameters.

The issue here is with how you created the HDA and exposed the parameters the subnet.

This is how it works absolutely fine:

Create a subnet
Add the Generic Loader inside
Go to Edit Parameters of the subnet
Go top left to the "From nodes" tab
Drag 'n' drop the parms onto your parameter list.

So drag 'n' drop e.g. from here to root:

Then the menu script becomes:

Which just means - use the inner node's menu.

So the issue was solely due to you 'copy pasting' the parameters instead of how you should be exposing the inner node's parameters upstream.

So basically we shouldn't need to change anything, we can keep the phm() calls etc.

We'll just need to document or provide pointers for non-senior Houdini artists that this is how they should expose these.
In that case, you can e.g. just pick only the version field and it works absolutely fine.

I've played with this for a while - and it's really nice.

I think we just need:

Expression: We'll need Load LOPs: Support update via context options (replace parm callbacks with expressions) #112 first to replace the callbacks to expressions and have that working.
Remove or rethink thumbnails: I think we're best off removing the thumbnail logic from this generic loader - or hopefully fix it in Load LOPs: Support update via context options (replace parm callbacks with expressions) #112 so it also makes sense there, likely making it non-live and requiring a "refresh" click. We may then need an additional parameter thumbnail_holder_node which would define the node that would get the thumbnail (the parent hda) instead of the node itself. Or we even make a generic "thumbnail node" hda to separate it out from the minimal base loader functionality. Anyway, stuff to think about - but no need for focus now.

…through `hou.phm()`

MustafaJafar · 2024-10-21T10:09:43Z

So basically we shouldn't need to change anything, we can keep the phm() calls etc.

I've reverted the changes in f7d719c

MustafaJafar · 2024-10-21T10:15:26Z

This commit baa2082 replaces the list of operator paths with operator list.

list of operators: where I used an invisible parameter to control/modify the list of operator paths.
operator list.

Although, I like the operator list less but it complies with mark tucker's comment on the side fx forums:

You should really not be "doing things" as a result of evaluating expressions.

I also tried making an expression for the operator path parameter but it will be something like

references = hou.parm('./file').parmsReferencingThis()
num = int(hou.expandString("$CH").split("_")[-1])
parm = references[num-1]
parm.node().path()

BigRoy · 2024-10-30T00:22:02Z

Based on our last discussion, it'd be better to open the parameters panel for the created generic loader. Also, it'd be cool to use better names for the generic loader, so we came up with {filenode_node}_{fileparm_name}_loader I borrowed some code from #140 and here's how it looks like:

@MustafaJafar I've made some commits to your branch - in particular it now 'replaces' the "Load filepath to node" loader.

Aside of that I can't seem to get it to work that it shows me the popup panel?

MustafaJafar · 2024-10-30T10:19:31Z

in particular it now 'replaces' the "Load filepath to node" loader.

Thank you.

Aside of that I can't seem to get it to work that it shows me the popup panel?

we only need to add:

show_generic_loader_parmpanel(node)

I've already add it here 5ed1357 ^^

MustafaJafar · 2024-10-30T12:05:29Z

Promoting the Generic Loader parameters

When building a custom HDA that uses the generic loader, the recommended way to promote the generic loader parameters is by creating the parameters from nodes.

But, you may hit an issue like this one if you promote a parameter with action, e.g. product parameter.

This issue is much similar when promoting some parameters (that have action callback) of the group sop node.

The issue is discussed in SOP subnet parameter promotion issue | SideFx Forums
and let me quote the easiest suggested solution:

The simplest is usually to promote the other parameters referenced by any such scripts as well,
making them invisible if they serve no other purpose on the promoted node. 
This is the easiest way to ensure the script is running similarly to how it does on the original node.

Here's a demo:
I've project and folder_path parameters hidden, but they need to exist because the action script on product expects them to be found on the node.
This demo is from #112 that implements expressions. Without implementing expressions we either set the generic loader node as editable node inside the HDA or we will get the following permission error

permission error

Traceback (most recent call last):
  File "ayon::Sop/generic_loader::1.0/product_name", line 2, in <module>
  File "E:\Ynput\ayon-houdini\client\ayon_houdini\api\hda_utils.py", line 395, in on_representation_parms_changed
    node.parm("representation").set(representation_id)
  File "C:\PROGRA~1/SIDEEF~1/HOUDIN~1.805/houdini/python3.9libs\houpythonportion\Parm.py", line 91, in set
    self._set(value)
  File "C:\PROGRA~1/SIDEEF~1/HOUDIN~1.805/houdini/python3.9libs\hou.py", line 65711, in _set
    return _hou.Parm__set(self, *args)
hou.PermissionError: Failed to modify node or parameter because of a permission error.  Possible causes include locked assets, takes, product permissions or user specified permissions

…put/ayon-houdini into enhancement/generic_load_test

MustafaJafar · 2024-10-30T21:47:17Z

Show the parameter editor, set it to the node path without selecting the node.
By default the hscript commands create the pane on the lower left of the window and we may fix it using -P ‹x› ‹y› flag, refernce.

client/ayon_houdini/plugins/load/load_filepath.py

client/ayon_houdini/api/lib.py

client/ayon_houdini/plugins/load/load_filepath.py

client/ayon_houdini/api/lib.py

…put/ayon-houdini into enhancement/generic_load_test

BigRoy

Looks good to me. Nice work, I'd say - ready to merge!

…enhancement/generic_load_test # Conflicts: # client/ayon_houdini/api/hda_utils.py

MustafaJafar · 2024-10-31T14:48:29Z

After using expressions instead of callbacks, This unlocked some powerful features.

e.g. I've these 3 products with different numbers

I can load them in a for loop. and it works either or not I put my nodes inside a custom HDA.

Also,
here's the a demo for updating the version, the representation.

here's the a demo for updating the thumbnail

MustafaJafar added 4 commits October 14, 2024 11:19

add generic loader HDAs

02d3e41

Implement GenericLoader

6dcc9cf

Add logic to generic loader otls and extend hda_utils functions

c910b8e

make find_active_network more generic

3e32e96

MustafaJafar added the type: enhancement Improvement of existing functionality or minor addition label Oct 14, 2024

MustafaJafar requested review from BigRoy and moonyuet October 14, 2024 12:04

MustafaJafar self-assigned this Oct 14, 2024

MustafaJafar added 2 commits October 17, 2024 16:59

Merge branch 'develop' into enhancement/generic_load_test

0450291

support frames and UDIM token

4dc818c

adjust the order of generic loader

60e0a43

import the functions instead of access them through hou.phm()

bccc305

MustafaJafar added 2 commits October 19, 2024 00:56

add ayon_load button on create to file and alembic nodes

b75c9bb

Show nodes referencing the file parm in generic loader nodes

f853386

Merge branch 'develop' into enhancement/generic_load_test

ce9b4df

MustafaJafar added 2 commits October 21, 2024 12:01

revert bccc305d952993e55e69176d613a50cb28540181 - access functions …

f7d719c

…through `hou.phm()`

replace list of operator paths with operator list

baa2082

Fix typo

3aa4dc4

show parm panel when using FilePathLoader

5ed1357

BigRoy and others added 3 commits October 30, 2024 13:51

Merge branch 'enhancement/generic_load_test' of https://github.com/yn…

9cda781

…put/ayon-houdini into enhancement/generic_load_test

refactor show_generic_loader_parmpanel to show_node_parmeditor

e11c12e

set the parameter editor to the node path without selecting the node

314e04a

BigRoy reviewed Oct 30, 2024

View reviewed changes

BigRoy added 6 commits October 31, 2024 00:15

Apply suggestions from code review

53eff90

Merge branch 'enhancement/generic_load_test' of https://github.com/yn…

552116c

…put/ayon-houdini into enhancement/generic_load_test

Tweak readability

e2479c4

Revert changes

699c97b

Remove unused import

6088dca

Cosmetics

08540c0

BigRoy approved these changes Oct 30, 2024

View reviewed changes

BigRoy self-requested a review October 31, 2024 13:49

BigRoy added 6 commits October 31, 2024 14:56

Merge branch 'develop' of https://github.com/ynput/ayon-houdini into …

02f26a2

…enhancement/generic_load_test # Conflicts: # client/ayon_houdini/api/hda_utils.py

Update HDAs after refactoring

5dcbc56

Fix missing imports

941f775

Remove callback

e95a9c7

Fix loader plugin

c31d90e

Fix bug on thumbnails for ObjNode

1b06f3d

BigRoy merged commit 952ded4 into develop Oct 31, 2024
1 check passed

MustafaJafar deleted the enhancement/generic_load_test branch November 5, 2024 08:36

This was referenced Dec 17, 2024

Introduce Houdini Generic Loader HDAs with examples for users ynput/ayon-documentation#320

Open

Use Generic Loader as internal building block for Load Asset/Shot LOP #199

Open

Houdini: Load products in active panel instead of a container group #13

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generic Loader OTLs #121

Generic Loader OTLs #121

MustafaJafar commented Oct 14, 2024 •

edited

Loading

BigRoy commented Oct 14, 2024 •

edited

Loading

MustafaJafar commented Oct 14, 2024

HDA maintenance

MustafaJafar commented Oct 17, 2024 •

edited

Loading

UDIM path

MustafaJafar commented Oct 17, 2024 •

edited

Loading

MustafaJafar commented Oct 18, 2024

BigRoy commented Oct 18, 2024 •

edited

Loading

MustafaJafar commented Oct 18, 2024 •

edited

Loading

BigRoy commented Oct 18, 2024

MustafaJafar commented Oct 18, 2024

MustafaJafar commented Oct 18, 2024 •

edited

Loading

BigRoy commented Oct 20, 2024 •

edited

Loading

MustafaJafar commented Oct 21, 2024

MustafaJafar commented Oct 21, 2024 •

edited

Loading

BigRoy commented Oct 30, 2024

MustafaJafar commented Oct 30, 2024 •

edited

Loading

MustafaJafar commented Oct 30, 2024 •

edited

Loading

MustafaJafar commented Oct 30, 2024 •

edited

Loading

BigRoy left a comment

MustafaJafar commented Oct 31, 2024

Generic Loader OTLs #121

Generic Loader OTLs #121

Conversation

MustafaJafar commented Oct 14, 2024 • edited Loading

Changelog Description

Additional Info

Additional Info 2 - Why using an HDA

Additional Info 3 - Promoting the Generic Loader parameters

Additional Info 4 - Expressions are so powerful

Testing notes:

BigRoy commented Oct 14, 2024 • edited Loading

HDA maintenance

UDIM path

Generic Loader in Loader UI

Houdini native focus

MustafaJafar commented Oct 14, 2024

HDA maintenance

MustafaJafar commented Oct 17, 2024 • edited Loading

UDIM path

MustafaJafar commented Oct 17, 2024 • edited Loading

MustafaJafar commented Oct 18, 2024

BigRoy commented Oct 18, 2024 • edited Loading

MustafaJafar commented Oct 18, 2024 • edited Loading

BigRoy commented Oct 18, 2024

MustafaJafar commented Oct 18, 2024

MustafaJafar commented Oct 18, 2024 • edited Loading

BigRoy commented Oct 20, 2024 • edited Loading

MustafaJafar commented Oct 21, 2024

MustafaJafar commented Oct 21, 2024 • edited Loading

BigRoy commented Oct 30, 2024

MustafaJafar commented Oct 30, 2024 • edited Loading

MustafaJafar commented Oct 30, 2024 • edited Loading

Promoting the Generic Loader parameters

MustafaJafar commented Oct 30, 2024 • edited Loading

BigRoy left a comment

Choose a reason for hiding this comment

MustafaJafar commented Oct 31, 2024

MustafaJafar commented Oct 14, 2024 •

edited

Loading

BigRoy commented Oct 14, 2024 •

edited

Loading

MustafaJafar commented Oct 17, 2024 •

edited

Loading

MustafaJafar commented Oct 17, 2024 •

edited

Loading

BigRoy commented Oct 18, 2024 •

edited

Loading

MustafaJafar commented Oct 18, 2024 •

edited

Loading

MustafaJafar commented Oct 18, 2024 •

edited

Loading

BigRoy commented Oct 20, 2024 •

edited

Loading

MustafaJafar commented Oct 21, 2024 •

edited

Loading

MustafaJafar commented Oct 30, 2024 •

edited

Loading

MustafaJafar commented Oct 30, 2024 •

edited

Loading

MustafaJafar commented Oct 30, 2024 •

edited

Loading