search

getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
218 stars 49 forks source link

Workfiles: Multiple hosts support #365

Closed BigRoy closed 5 years ago

BigRoy commented 5 years ago

Issue

Supporting multiple hosts in the Workfiles app was originally laid out in the code, but relatively spread across the app.py file. Making it:

Now I've done some ground work on support Houdini + Fusion, see this commit and wanted to discuss on the implementation of the Host class I set up solely for Workfiles tool.

The issue I'm seeing is that it mimics (it being called Host) the avalon.maya, avalon.fusion, avalon.houdini the basic host integrations that Avalon already has - yet they don't require the save, open, etc. API to be implemented for a Host integration to work with Avalon - just the Workfiles app wouldn't work.

Discussion

We can go about this in two ways:

The methods currently required for Workfiles can be found here and comes down to: save, open, current_file and root. Their docstrings should elaborate somewhat on what they intend to do.

Referenced on Gitter: https://gitter.im/getavalon/Lobby?at=5c403bcdcb47ec3000586afe

davidlatwe commented 5 years ago

I would vote for option 2 : Move these API methods to the Avalon globally supported hosts and have these methods available in their api.

Like, for example, having a sub-module workio.py in every host module.

Since the workfile save/load and other definitions are part of the hosts' behavior/features, and possible other tools or plugins would use these methods.

mottosso commented 5 years ago

Ah, I see what's going on. :)

That's sneaky!

Yes, any references to any host should reside within the host's own sub-package, like Maya and friends do. Is that what you're suggesting here, @BigRoy?

BigRoy commented 5 years ago

Is that what you're suggesting here, @BigRoy?

Yes, the second way I proposed indeed means removing this Host class that I now implemented with this commit and move those methods to avalon.maya, avalon.houdini, etc. (In that commit I already separated it purely away from avalon.tools.workfiles.app into its own file.

BigRoy commented 5 years ago

Refactor to avalon registered host

Pushed a new commit that refactors it to use the registered avalon host. For now I moved it into a workio.py for each host as @davidlatwe mentioned.

@mottosso, what do you think? Better?

Work Files Root

@tokejepsen, I felt a bit weird about always needing to pass root to the Work Files too where I'd expect the host to be able to figure it out. So I created work_root. However, I'm not too sure about it. It feels odd that the workio.py now needs to define it. Anyone have any other ideas? It should almost be as simple as the environment variable AVALON_WORKDIR but usually the actual scene files are stored in a scenes folder or alike. In Maya dependent on the workspace.mel but I'd want to look into something universal across Avalon.

Also added some slight documentation into the avalon.tools.workfiles README.md file. :)

tokejepsen commented 5 years ago

Very nice work on this @BigRoy!

Anyone have any other ideas?

I think the sole reason for not using AVALON_WORKDIR was Maya's workspace workflow.

Could also tackle it the other way around, where you force Maya "scene" directory to be AVALON_WORKDIR (or whatever it'll end up being). But that might create confusing behaviour for developers when they have a custom "scene" directory in the workspace.mel.

mkolar commented 5 years ago

But that might create confusing behaviour for developers when they have a custom "scene" directory in the workspace.mel.

I'd actually feel quite strongly that using avalon, we should bypass all software specific folder management tools like workspace.mel in maya. it causes just headaches and brings extra variable that are hard to control fully.

BigRoy commented 5 years ago

I'd actually feel quite strongly that using avalon, we should bypass all software specific folder management tools like workspace.mel in maya. it causes just headaches and brings extra variable that are hard to control fully.

Great, then the next step is - how do we customize this beyond AVALON_WORKDIR? Since it'd need to incorporate the scenes folder, or whatever the artist might need to work in (customized to a specific studio setting)?

Would the .toml need an extra definition for work_scene_folder or alike? Any ideas? It already has application_dir maybe additionally scenes_dir?

mottosso commented 5 years ago

I think the Host class-based approach is right, and (almost) aligns with how we've got plug-ins for creation, management, loading and publishing already. But how about we make it a plug-in too, so it's 100% aligned? That way, the API will remain familiar.

tokejepsen commented 5 years ago

Great, then the next step is - how do we customize this beyond AVALON_WORKDIR? Since it'd need to incorporate the scenes folder, or whatever the artist might need to work in (customized to a specific studio setting)?

@BigRoy I think the scope of this conversation is getting too big for the issue. Maybe we could just work towards getting multiple hosts supported in the workfiles app with using nothing by Avalon requirements. Think setting AVALON_WORKDIR would be enough for the workfiles app for now.

On that note it would be good to see a PR for this, that we all can work on to get merged?

But how about we make it a plug-in too, so it's 100% aligned?

@mottosso could you elaborate on this?

BigRoy commented 5 years ago

This has been implemented with #387