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 unrealuasset as the publish template #328

Merged

Conversation

moonyuet
Copy link
Member

@moonyuet moonyuet commented Aug 29, 2024

PR Checklist

Description of changes

Added the published template setting for unreal uasset/umap publishing
Related to this PR: ynput/ayon-core#859

Technical details

The current uasset/umap publisher is using default template name as the published path, but the default one would change the filename of the uasset/umap, which would corrupt the uasset/umap, and u sers cannot load the uasset/umap. There is some custom publish template needs to be added to make sure the uasset is published with original filename so that the file itself would not be corrupted.

Additional context

The related ayon-core PR: ynput/ayon-core#859
The related issues: ynput/ayon-unreal#64 and ynput/ayon-unreal#66

@LiborBatek
Copy link
Member

@antirotor @iLLiCiTiT guys, who is the best for review of this one?

@MustafaJafar
Copy link

@antirotor @iLLiCiTiT guys, who is the best for review of this one?

Let me share my 2 cents..
Martin can push this to experimental ayond dev. and then guys who can test unreal PRs can use the experimental ayon and test ynput/ayon-core#859 all together.

@MustafaJafar MustafaJafar added the type: enhancement Improvement of existing functionality or minor addition label Sep 9, 2024
@MustafaJafar
Copy link

MustafaJafar commented Sep 9, 2024

I also wonder why anatomy settings live in ayon-backend repo.
shouldn't it live in a designated addon e.g. ayon-core ?

Tagging @dee-ynput

@dee-ynput
Copy link

That's actually a very legit question for which I would love to have the answer :)

@martastain
Copy link
Member

I don't know what do you mean. That core addon would somehow define the anatomy model? Or its part? Please elaborate how it would work.

Keep in mind that across the system we heavily depend on certain parts of the anatomy, such as statuses, folder/task types and roots.

@MustafaJafar
Copy link

Please elaborate how it would work.

tbh, It's hard for me to make a suggestion in this area.

But, I didn't expect that a PR to ayon-backend is needed just to add some default settings.
Maybe it's a grey area.

Personally, I thought that ayon-backend includes the base classes definitions so that they are used somewhere else. e.g. BaseServerAddon.

@martastain
Copy link
Member

But, I didn't expect that a PR to ayon-backend is needed just to add some default settings.
Maybe it's a grey area.

This PR just adds a one item to defaults of the built-in anatomy preset. It might as well be an example. As soon a studio creates their own anatomy preset, they may delete this one. Maybe we could replace the default_factory method of publish templates with something else (pulling from an addon?). but is it worthed? the entire idea behind anatomy presets is that studio can create one or more custom preset they will use.

@moonyuet
Copy link
Member Author

But, I didn't expect that a PR to ayon-backend is needed just to add some default settings.
Maybe it's a grey area.

This PR just adds a one item to defaults of the built-in anatomy preset. It might as well be an example. As soon a studio creates their own anatomy preset, they may delete this one. Maybe we could replace the default_factory method of publish templates with something else (pulling from an addon?). but is it worthed? the entire idea behind anatomy presets is that studio can create one or more custom preset they will use.

We can mention in the documentation about this anatomy preset and how they can use it for the uasset publish. If the users get any question on the uasset publish, we can just guide them to the docs and ask them to add the anatomy preset and its relevant setting in the core addon.

@MustafaJafar
Copy link

We can mention in the documentation about this anatomy preset and how they can use it for the uasset publish. If the users get any question on the uasset publish, we can just guide them to the docs and ask them to add the anatomy preset and its relevant setting in the core addon.

So, instead of this PR, we will add something like the following in Docs:

  1. For uasset publish, you'd need a custom anatomy template.
  2. Add the following template
    image

I think this could work. But, I'm not sure what is better product wise.


Maybe we could replace the default_factory method of publish templates with something else (pulling from an addon?). but is it worthed?

maybe, I can't tell.

--

tbh, I just didn't know that anatomy definitions are part of the backend itself. which sounds strange to me as I thought that is living in the core addon. which also not entirely correct idea because AYON server can have no addons and then we should have the anatomy definitions to be able to create any projects.

anyways, I should leave it for some who is smarter and have wider overview than me.

@dee-ynput
Copy link

The anatomy is a core concept of AYON, naming convention (template) is more or less a core concept, but DCC specific templates should definitely not be declared at server core.

That being said,:

  • We have this kind of domain leakage in other places (application addon for example)
  • We already broke the flow of domains with maya2unreal, simpleUnrealTextureHero, ...
  • We have an Epic going on for the Settings UX which may help fix this soon.

So I would validate this PR and fix that concern (+more) later with a dedicated Epic 👍

@mkolar mkolar marked this pull request as ready for review September 30, 2024 14:51
@moonyuet moonyuet merged commit 7c11cce into develop Oct 1, 2024
@moonyuet moonyuet deleted the enhancement/add_puiblsh_template_path_for_uasset_publish branch October 1, 2024 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement Improvement of existing functionality or minor addition
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants