Chore: Make sure udim in representation is not a list

[moonyuet]

Changelog Description

Udim in representation context is not list of integers but only single integer.

Additional info

This PR is to fix the bug where the scene inventory cannot update the filepath with udim extension, which uncovered that key udim in representation context data is stored as list of integers. That was changed to store first used udim value. For old representations was added auto-fix in get representation path functions to automatically use first item from udim list.

Testing notes:

Build and install the addon with this branch.

1. Build addon with this branch
2. Launch Substance Painter
3. Publish anything with udim
4. Launch Maya
5. load image texture
6. update/set version with scene inventory

[ynbot]

Task linked: AY-5975 Maya: Manage - "Set Version/Update to latest" action doesnt work for Image Product using

[iLLiCiTiT]

What is difference from default template? And why we have default setting for it if we don't have that template in default anatomy?

[moonyuet]

What is difference from default template? And why we have default setting for it if we don't have that template in default anatomy?

very small difference:<_{udim}> and <.{udim}>, any way to add the template into default anatomy?

[BigRoy]

very small difference:<_{udim}> and <.{udim}>, any way to add the template into default anatomy?

Aren't we better of just updating the default then?
And also, why does Maya fail with the first one - isn't that technically valid as well? (And hence, may just need fixing om some loading part in maya instead?)

Either:

• file_1001.exr is a valid UDIM pattern, if so - we should fix the maya load.
• or it's an invalid UDIM pattern and we should update the default template
  ?

And why we have default setting for it if we don't have that template in default anatomy?

Also, your screenshot shows a default template path for substanceImage but your PR does not contain it. That's what @iLLiCiTiT is trying to say.

[moonyuet]

Aren't we better of just updating the default then?
And also, why does Maya fail with the first one - isn't that technically valid as well? (And hence, may just need fixing om some loading part in maya instead?)

It doesn't fail in loading but updating assets, and it is not failing in maya loading but the scene inventory.

Also, your screenshot shows a default template path for substanceImage but your PR does not contain it. That's what @iLLiCiTiT is trying to say.

I am trying to find the json data which stores the data in template.json in OP. But I can't

[BigRoy]

I am trying to find the json data which stores the data in template.json in OP. But I can't

Ah, good point - those are anatomy settings I believe and are actually in the backend as presets. I noticed that recently when doing this draft PR: ynput/ayon-backend#252

As such they should be somewhere here: https://github.com/ynput/ayon-backend/blob/develop/ayon_server/settings/anatomy/templates.py

It doesn't fail in loading but updating assets, and it is not failing in maya loading but the scene inventory.

Again, seems like we should fix that instead. This sounds like a Maya loader issue - not a template issue.

[iLLiCiTiT]

NOTE: The bug that happened on update IS NOT related to template. If this is meant as bugfix of that bug then this won't help.

It is not bug of maya either, it is bug of our load api and scene inventory tool.

[BigRoy]

NOTE: The bug that happened on update IS NOT related to template. If this is meant as bugfix of that bug then this won't help.

I had to figure out what the issue was - I reported it for now as this: #765

[iLLiCiTiT]

This change breaks backwards compatibility, does not resolve existing representations and not sure it is correct fix for the issue.

• Do we know why "udim" was stored as list of integers? Is it wrong? (@antirotor you might know why we do that)
• Do we know if the validation of existing path in scene inventory is reasonable? Shouldn't we focus on that?

[iLLiCiTiT]

Modified PR. Adding explicitly "udim" to representation context is skipped as it is there automatically within used_values on filled template. Added auto-fix for old representations to use first udim during get representation path.

[BigRoy]

Changes look good to me. Would be great if @moonyuet and/or @LiborBatek can confirm this works with existing substance painter products and whether new products also work AFTER publishing with this update in core when publishing textures with UDIMs from substance painter.

That publish from substance painter will need to be 'investigated manually' because the "backwards compatibility" fix will hide it if currently the new published data is still wrong.

[iLLiCiTiT]

That publish from substance painter will need to be 'investigated manually' because the "backwards compatibility" fix will hide it if currently the new published data is still wrong.

What's wrong with substance painter? Did I miss something?

[LiborBatek]

That publish from substance painter will need to be 'investigated manually' because the "backwards compatibility" fix will hide it if currently the new published data is still wrong.

What's wrong with substance painter? Did I miss something?

Hmm I think nothing? ;) ...lets give it a try and see if any issue arises...testing round 3

[LiborBatek]

It works well now!

It even does work when Manage > Update to latest version and each version of image product have different publishing template used both with _{udim} and .{udim} which is even more impressive to me!

I guess can be merged then...I have also test to produce new image product in Substance Painter host to be sure nothing is broken by this PR. All good and working...

@iLLiCiTiT small note tho, probably for other PR?? when performing Manage > Set Version it works just on single image product not for multiple selected image products (even tho other actions like the aformentioned update to latest works for multiselection of images)

here is the traceback for you:

# Traceback (most recent call last):
#   File "C:\Work\REPO\ayon-core\client\ayon_core\tools\sceneinventory\view.py", line 366, in <lambda>
#     lambda: self._show_version_dialog(item_ids, active_repre_id)
#   File "C:\Work\REPO\ayon-core\client\ayon_core\tools\sceneinventory\view.py", line 820, in _show_version_dialog
#     version_option = SelectVersionDialog.ask_for_version(
#   File "C:\Work\REPO\ayon-core\client\ayon_core\tools\sceneinventory\select_version_dialog.py", line 255, in ask_for_version
#     dialog.set_versions(version_options)
#   File "C:\Work\REPO\ayon-core\client\ayon_core\tools\sceneinventory\select_version_dialog.py", line 242, in set_versions
#     self._versions_combobox.set_versions(version_options)
#   File "C:\Work\REPO\ayon-core\client\ayon_core\tools\sceneinventory\select_version_dialog.py", line 175, in set_versions
#     icon = get_qt_icon({
#   File "C:\Work\REPO\ayon-core\client\ayon_core\tools\utils\lib.py", line 578, in get_qt_icon
#     return _IconsCache.get_icon(icon_def)
#   File "C:\Work\REPO\ayon-core\client\ayon_core\tools\utils\lib.py", line 496, in get_icon
#     cache_key = cls._get_cache_key(icon_def)
#   File "C:\Work\REPO\ayon-core\client\ayon_core\tools\utils\lib.py", line 489, in _get_cache_key
#     return "|".join(parts)
# TypeError: sequence item 1: expected str instance, NoneType found

I think this one was fixed recently but maybe with UDIMs its not fixing it...

[LiborBatek]

@iLLiCiTiT for better info here is the working occasion vs mutliselection when trying Manage > Set Version

Again, this could be for other PR maybe??

[iLLiCiTiT]

small note tho, probably for other PR?? when performing Manage > Set Version it works just on single image product not for multiple selected image products (even tho other actions like the aformentioned update to latest works for multiselection of images)

Looking into it. Merging this one tho.

[BigRoy]

What's wrong with substance painter? Did I miss something?

Sorry @iLLiCiTiT what I meant was - we should double check whether the new UDIM publish now actually does integrate with the value to NOT be a list anymore in the database and that it results in a value we expected in representation_entity["context"]

[iLLiCiTiT]

@LiborBatek did you try to publish new texture with udim? Could you quickly try with develop if not?

[iLLiCiTiT]

Libor confirmed he tried to publish and update new textures.

[BigRoy]

Libor confirmed he tried to publish and update new textures.

Haha, sorry for going back to this again. But my point is that the updating itself would work regardless, even if the new publish data would still be storing it as a list for whatever reason because we have the conversion logic there now as well.

To be entirely sure the data is now fixed we may need to investigate the actual representation entity's data to ensure representation_entity["context"] is now a str like 1001 instead of list like ["1001"].

Looking at the code I suspect it should be fixed - but the code is also complex in other areas and we might be missing another subtle bug.

[iLLiCiTiT]

To be entirely sure the data is now fixed we may need to investigate the actual representation entity's data to ensure representation_entity["context"] is now a str like 1001 instead of list like ["1001"].

Looking at the code I suspect it should be fixed - but the code is also complex in other areas and we might be missing another subtle bug.

Aaah, now I get it. Will validate with Libor tomorrow.