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

Substance Painter: Add support for sync workfile version #590

Merged

Conversation

moonyuet
Copy link
Member

@moonyuet moonyuet commented Jun 4, 2024

Changelog Description

This PR is to add sync workfile version option into substance painter host.

Additional info

Build the latest version of core addon and substance painter addon

Testing notes:

  1. Enable Follow workfile version in either ayon+settings://core/publish/CollectAnatomyInstanceData/follow_workfile_version
  2. Publish
  3. TextureSet and Workfile version should be aligned

@ynbot
Copy link
Contributor

ynbot commented Jun 4, 2024

@ynbot ynbot added size/XS type: enhancement Improvement of existing functionality or minor addition host: Substance Painter labels Jun 4, 2024
package.py Outdated Show resolved Hide resolved
Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

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

I did test both occasions when Follow workfile version been set in Core addon as seen here

Screenshot 2024-06-04 181806

when published already present texture set publish instance it works and publish the version following the workfile version.

Screenshot 2024-06-04 181311

It also works when the Core addon settings being disabled, but the user sets it ON when creating new Publish instance also works fine!

Screenshot 2024-06-04 175718

Small note: there is no hint message for this option as with others...we should also incorporate it for this settings too.

Overall works nicely in both occasions...

@LiborBatek
Copy link
Member

One small comment:

@moonyuet would it be possible to keep visible that option Follow workfile version on the Publish Instance so user could potentially switch the behaviour?? not sure if possible and also if good to have it exposed for ever...

@BigRoy
Copy link
Collaborator

BigRoy commented Jun 4, 2024

@moonyuet would it be possible to keep visible that option Follow workfile version on the Publish Instance so user could potentially switch the behaviour?? not sure if possible and also if good to have it exposed for ever...

I wouldn't actually make that an artist's choice - that's a studio choice usually. Can you think of a use case where an artist really wants to take that control?

Actually - I also wouldn't expose it on the Create tab. Maya, Houdini and others have similar functionality but I've never seen that toggle in any of the integrations on any Creators? What am I missing? :)

@LiborBatek
Copy link
Member

@BigRoy I get your point, makes sense

@moonyuet But now we can control it only globally within Ayon Core Addon with this setting ?

Screenshot 2024-06-04 181806

I think we need to allow user to set it within Substance Painter addon imho....kind of local override so it doesnt follow global settings only imho?

@BigRoy opinion on this?

@moonyuet
Copy link
Member Author

moonyuet commented Jun 5, 2024

@moonyuet But now we can control it only globally within Ayon Core Addon with this setting ?

Yes, it is

I think we need to allow user to set it within Substance Painter addon imho....kind of local override so it doesnt follow global settings only imho?

@BigRoy I think we can do this too by using instance.data["followWorkfileVersion"]

@BigRoy
Copy link
Collaborator

BigRoy commented Jun 5, 2024

But now we can control it only globally within Ayon Core Addon with this setting ?
Screenshot 2024-06-04 181806

I think we need to allow user to set it within Substance Painter addon imho....kind of local override so it doesnt follow global settings only imho?

Currently this is global to all I believe, and yes - that's how we use it. If we want to make that "profile" based so that we can go more complex there I'd make that a separate issue and separate PR but it's a valid request I suppose.

I think we can do this too by using instance.data["followWorkfileVersion"]

I think profiles filtering logic can be used for this - but I'd not merge that into this PR.

@BigRoy
Copy link
Collaborator

BigRoy commented Jun 5, 2024

But now we can control it only globally within Ayon Core Addon with this setting ?
Screenshot 2024-06-04 181806

Looking at the logic now it's still in the creator - and not "globally" like in other hosts. Is it correct that you still need to implement it so that it matches e.g. the maya/houdini, etc. logic?

@moonyuet
Copy link
Member Author

moonyuet commented Jun 5, 2024

But now we can control it only globally within Ayon Core Addon with this setting ?
Screenshot 2024-06-04 181806
I think we need to allow user to set it within Substance Painter addon imho....kind of local override so it doesnt follow global settings only imho?

Currently this is global to all I believe, and yes - that's how we use it. If we want to make that "profile" based so that we can go more complex there I'd make that a separate issue and separate PR but it's a valid request I suppose.

I think we can do this too by using instance.data["followWorkfileVersion"]

I think profiles filtering logic can be used for this - but I'd not merge that into this PR.

I actually implement the logic here for temporary use. And once the profile setting has been finished we can make changes on
that.

@BigRoy
Copy link
Collaborator

BigRoy commented Jun 5, 2024

I actually implement the logic here for temporary use. And once the profile setting has been finished we can make changes on
that.

I'd make the logic here match the other integrations - so it's aligned and once we move things over it works for all. Best not to deviate here, especially if it's "temporary".

@moonyuet
Copy link
Member Author

moonyuet commented Jun 5, 2024

I actually implement the logic here for temporary use. And once the profile setting has been finished we can make changes on
that.

I'd make the logic here match the other integrations - so it's aligned and once we move things over it works for all. Best not to deviate here, especially if it's "temporary".

I already restored the creator logic and the sync workfile version can only sync through the global settings and we can integrate profile setting later.

Copy link
Member

@LiborBatek LiborBatek left a comment

Choose a reason for hiding this comment

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

All works as supposed...when Follow workfile version being Enabled

here I have produced version 021 from the workfile v021 and all matches.

Screenshot 2024-06-07 102138

There is also correct Validate when trying to produce invalid / existing or lower version than already present in the DB.

Screenshot 2024-06-07 102254

LGTM!

Co-authored-by: Jakub Trllo <43494761+iLLiCiTiT@users.noreply.github.com>
@moonyuet moonyuet requested a review from iLLiCiTiT June 7, 2024 09:00
@moonyuet moonyuet merged commit a6709f2 into develop Jun 11, 2024
1 check passed
@moonyuet moonyuet deleted the enhancement/AY-5648_Substance-work--publish-version-sync branch June 11, 2024 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS type: enhancement Improvement of existing functionality or minor addition
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants