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

Workfiles: Multiple hosts support #365

Closed
BigRoy opened this issue Jan 17, 2019 · 10 comments
Closed

Workfiles: Multiple hosts support #365

BigRoy opened this issue Jan 17, 2019 · 10 comments
Assignees

Comments

@BigRoy
Copy link
Collaborator

BigRoy commented Jan 17, 2019

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:

  • hard to find the exact implementation details needed for a host.
  • and continuously harder as the amount of hosts would grow.

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:

  • Continue down the route of this commit and keep save/open/current-file and similar functions there solely for Workfiles for simplicity even though it might introduce some confusion for newcomers.
  • Move these API methods to the Avalon globally supported hosts and have these methods available in their api.

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

@BigRoy BigRoy self-assigned this Jan 17, 2019
@davidlatwe
Copy link
Collaborator

davidlatwe commented Jan 17, 2019

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
Copy link
Contributor

mottosso commented Jan 17, 2019

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
Copy link
Collaborator Author

BigRoy commented Jan 17, 2019

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
Copy link
Collaborator Author

BigRoy commented Jan 17, 2019

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
Copy link
Collaborator

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
Copy link
Member

mkolar commented Jan 18, 2019

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
Copy link
Collaborator Author

BigRoy commented Jan 18, 2019

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
Copy link
Contributor

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
Copy link
Collaborator

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
Copy link
Collaborator Author

BigRoy commented Jul 25, 2019

This has been implemented with #387

@BigRoy BigRoy closed this as completed Jul 25, 2019
davidlatwe pushed a commit to davidlatwe/core that referenced this issue Sep 23, 2021
…asses_for_automatic_testing

Removed orphaned import of mayalookassigner
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants