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

Better validation report when publishing from unsaved workfile #786

Merged

Conversation

BigRoy
Copy link
Collaborator

@BigRoy BigRoy commented Jul 18, 2024

Changelog Description

The user now gets presented with a better validation report if a workfile is not saved.

Additional info

Previously the user would get to a "This is not your fault" error on Collect, but often newcomers failed to identify why it failed.

After this PR that's more informative:
Maya
image

Fusion
image

Testing notes:

  1. Publish from host without saved workfile/filepath
  2. Validator should catch it.

@BigRoy BigRoy added the type: enhancement Improvement of existing functionality or minor addition label Jul 18, 2024
@ynbot ynbot added the size/XS label Jul 18, 2024
Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

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

I gave it a shot in Houdini, where I opened a scene and did some changes and triggered validation. and nothing happened.
I added some log in validate_file_saved and it was cool 😅

image

I gave it another shot with an untitled scene. but the publisher errored telling me it's not my fault.
image
image

So, how does it work ?

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jul 24, 2024

So, how does it work ?

VERY GOOD! :) I hadn't tested specifically Houdini, but Houdini is an interesting outlier here. When a scene is unsaved, the scene is actually untitled.hip instead of None or "". So in a way Houdini DOES report as if it has a saved filename, and that filename indeed fails to collect scene version from.

So in a way - it's a different bug. Probably one we can best tackle on ayon-houdini to report a better file if 'unsaved' like that.

@moonyuet
Copy link
Member

moonyuet commented Jul 24, 2024

Max
image
image

SP(Guess you need to have some debug messages for workfile representation)
image

@BigRoy
Copy link
Collaborator Author

BigRoy commented Jul 24, 2024

Seems Max has its own "validator" which we should indeed bring to ayon-core instead so it automatically has the nice description and easy access to the fix actions, etc.

  1. Add max to families here
  2. Remove validator from max

Substance Painter would need some additional fixes indeed - but then again. The error itself isn't WORSE than what it was since it'd crash publisher UI in a similar manner. We can set up a PR to fix Substance Painter specifically and improve that later!

  1. Add substancepainter to families here
  2. Fix other Collectors in substancepainter integration

@BigRoy BigRoy self-assigned this Jul 24, 2024
@BigRoy
Copy link
Collaborator Author

BigRoy commented Aug 28, 2024

@MustafaJafar I've made a Houdini PR to improve Houdini's validation: ynput/ayon-houdini#81

@moonyuet I've made a Substance Painter PR ynput/ayon-substance-painter#7 to improve there. For Max we may want to follow up with removing Max's own one, but for now it may display two invalidations with this PR (the nice one from this PR, and the legacy one existing in Max).

Please double check the PRs in combination and approve if this works! 👍

@iLLiCiTiT
Copy link
Member

iLLiCiTiT commented Sep 2, 2024

Question? Are we moving the error to validation to show the message to artist "nicer"?

@BigRoy
Copy link
Collaborator Author

BigRoy commented Sep 2, 2024

Question? Are we moving the error to validation to show the message to artist "nicer"?

Yes.

@iLLiCiTiT
Copy link
Member

Then I might have a better solution, new PR in ayon-core (comming soon).

@BigRoy
Copy link
Collaborator Author

BigRoy commented Sep 2, 2024

Then I might have a better solution, new PR in ayon-core (comming soon).

As we discussed in DM your 'coming up' solution may not allow to run actions to allow the user with easy to access actions if an invalidation occurs during Collect. Hence, this PR is still the recommended approach for the time being.

I guess getting approval for this feature would still be the 'next step' and then just merge this.

@moonyuet @MustafaJafar if you can test and approve - then lovely!

@iLLiCiTiT
Copy link
Member

As we discussed in DM your 'coming up' solution may not allow to run actions to allow the user with easy to access actions if an invalidation occurs during Collect. Hence, this PR is still the recommended approach for the time being.

Just confirming that PR #861 would only show the message, but would not allow to trigger actions.

Copy link
Contributor

@MustafaJafar MustafaJafar left a comment

Choose a reason for hiding this comment

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

Maya
image
Houdini using ynput/ayon-houdini#81
image

@BigRoy BigRoy merged commit 9c6c0e3 into ynput:develop Sep 4, 2024
1 check passed
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
None yet
Development

Successfully merging this pull request may close these issues.

5 participants