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

AY-6654 Look: Fix None values in collecting and applying attributes #89

Merged

Conversation

BigRoy
Copy link
Contributor

@BigRoy BigRoy commented Sep 3, 2024

Changelog Description

This fixes a case where looks failed to apply due to None values being present in the collected attributes.
These will now be ignored in collected. There's an edge case where Maya returns None for string attributes that have no values set - those are captured now explicitly to just "" to still collect and apply them later.

Existing looks will now also apply correctly with None value in their look attributes, but the attributes with None values will be ignored with a warning.

Additional info

Testing notes:

  1. Load a model reference
  2. Create a string attribute, e.g. test
  3. Do not set any value for it.
    • Maya will return None for this attribute, e.g. select the object and run:
from maya import cmds
print(cmds.getAttr(".test"))

It will print None.
4. Publish the look for this mesh
5. The published .json file should have attribute value for test set to ""

To test the 'backwards compatible fix':

  • Publish the same look with develop branch. The value will be null in the .json file.`
  • OR, just open the last published .json file, and edit the "" value to null

Then, confirm that applying the look succeeds without errors but with a warning regarding the None value when applying with this PR. For example in my test run it logged:

// Warning: ayon_maya.api.lib : Skipping setting |_GRP|cube_GEO.test with value 'None'

@BigRoy BigRoy added type: bug Something isn't working sponsored This is directly sponsored by a client or community member labels Sep 3, 2024
@BigRoy BigRoy requested a review from LiborBatek September 3, 2024 10:38
@BigRoy BigRoy self-assigned this Sep 3, 2024
@BigRoy BigRoy linked an issue Sep 3, 2024 that may be closed by this pull request
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.

It all works speaking of the new way of treating the occasion when no any entry in the custom attribute present...so all good on this front!

I have issue when trying the "backward compatible" steps as it throws an error when None value present in the JSON so I dont get any warning only instead.

aka it seems that it still fails... (even tho my asset got the look assigned anyway)

// Warning: ayon_maya.api.lib : Skipping setting |screwD_modelMain_01_:_GRP|screwD_modelMain_01_:model_GRP|screwD_modelMain_01_:Body_GEO.custom with value 'None'
# Traceback (most recent call last):
#   File "c:\Work\REPO\ayon-maya\client\ayon_maya\tools\mayalookassigner\app.py", line 286, in on_process_selected
#     assign_look_by_version(
#   File "c:\Work\REPO\ayon-maya\client\ayon_maya\api\lib.py", line 1889, in assign_look_by_version
#     apply_shaders(relationships, shader_nodes, nodes)
#   File "c:\Work\REPO\ayon-maya\client\ayon_maya\api\lib.py", line 2027, in apply_shaders
#     apply_attributes(attributes, nodes_by_id)
#   File "c:\Work\REPO\ayon-maya\client\ayon_maya\api\lib.py", line 1738, in apply_attributes
#     set_attribute(attr, value, node)
#   File "c:\Work\REPO\ayon-maya\client\ayon_maya\api\lib.py", line 1694, in set_attribute
#     kwargs = ATTRIBUTE_DICT[value_type]
# KeyError: 'NoneType'

@BigRoy
Copy link
Contributor Author

BigRoy commented Sep 4, 2024

It all works speaking of the new way of treating the occasion when no any entry in the custom attribute present...so all good on this front!

I have issue when trying the "backward compatible" steps as it throws an error when None value present in the JSON so I dont get any warning only instead.

aka it seems that it still fails... (even tho my asset got the look assigned anyway)

// Warning: ayon_maya.api.lib : Skipping setting |screwD_modelMain_01_:_GRP|screwD_modelMain_01_:model_GRP|screwD_modelMain_01_:Body_GEO.custom with value 'None'
# Traceback (most recent call last):
#   File "c:\Work\REPO\ayon-maya\client\ayon_maya\tools\mayalookassigner\app.py", line 286, in on_process_selected
#     assign_look_by_version(
#   File "c:\Work\REPO\ayon-maya\client\ayon_maya\api\lib.py", line 1889, in assign_look_by_version
#     apply_shaders(relationships, shader_nodes, nodes)
#   File "c:\Work\REPO\ayon-maya\client\ayon_maya\api\lib.py", line 2027, in apply_shaders
#     apply_attributes(attributes, nodes_by_id)
#   File "c:\Work\REPO\ayon-maya\client\ayon_maya\api\lib.py", line 1738, in apply_attributes
#     set_attribute(attr, value, node)
#   File "c:\Work\REPO\ayon-maya\client\ayon_maya\api\lib.py", line 1694, in set_attribute
#     kwargs = ATTRIBUTE_DICT[value_type]
# KeyError: 'NoneType'

That's exactly the error this PR should solve. It applies the shaders yes, but fails to apply all attributes.
Did you test that "applying" with the null in the JSON file with this PR enabled?
(You will need to launch Maya while in this branch! So restart if you hadn't)

…kip_none' into bugfix/collect_look_attributes_skip_none
@BigRoy
Copy link
Contributor Author

BigRoy commented Sep 4, 2024

Sorry, somehow my previous push of the commit failed. It was missing this: fc52bb1

Try again.

@BigRoy BigRoy requested a review from LiborBatek September 4, 2024 10:52
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.

Now all good and no errors thrown...

Just that Warning message as mentioned in testing steps.

Good to go!

@LiborBatek LiborBatek merged commit 3947462 into ynput:develop Sep 4, 2024
1 check passed
@BigRoy BigRoy deleted the bugfix/collect_look_attributes_skip_none branch September 4, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sponsored This is directly sponsored by a client or community member type: bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AY-6654_Maya: assign lookMain to multiple assets
2 participants