studio/blender-studio-tools
12
31
Fork 19
Code Issues 20 Pull Requests 4 Projects Releases 1 Activity

Proposal: Create 'Convention Check' System #84

New Issue
Open
opened 2023-06-16 20:37:01 +02:00 by Nick Alberelli · 10 comments
Nick Alberelli commented 2023-06-16 20:37:01 +02:00
Member
Copy Link

Production Files created by Blender Studio users can be saved without checking if the file's contents matches the Blender Studio Pipeline's conventions. This proposal outlines the issues created by the lack of convention enforcement and a proposed solution.

Issues

Solution

Convention Check on Save

Convention Checks are verifications that the file's contents match the Blender Studio Pipeline's conventions. Convention Checks will be run before users can save their work. This ensures all files in the production are aligned with the conventions we set. Therefor, as issues arise for specific tasks, we can created additional checks to accommodate those issues. Some examples of things that can be Convention Checked are: https://studio.blender.org/pipeline/archive/pipeline-proposal-2019/task-companion-add-on Once all information about files can be saved in Kitsu, then we can confidently run convention check(s) on any production files.

  • Determine what convention checks need to be run based on Kitsu task type
  • Allow users to optionally skip Convention Checks
  • Report to Kitsu if File is saved with/without convention check
  • (Optional) Only use convention check when saving a Published version in Pull pipeline studio/blender-studio-pipeline#85

Steps to Success

  1. Make an operator that checks if the files meets conventions based on current Kitsu Task, report to Kitsu if Convention Checks have passed/failed.
  2. Include existing Convention Enforcement Operators (e.g. Check Action Names or Check Output Collection) in Convention Checks
Production Files created by Blender Studio users can be saved without checking if the file's contents matches the Blender Studio Pipeline's conventions. This proposal outlines the issues created by the lack of convention enforcement and a proposed solution. # Issues - Production Files are built with a combination of some automation using `Blender Kitsu>Shot Builder` and some manual tools. This means production files do not have reliable conventions set at the beginning of the production. (e.g. [Error when Building Shot](https://projects.blender.org/TinyNick/blender-studio-tools/issues/23)) [Errors showing up across many files](https://projects.blender.org/studio/blender-studio-pipeline/issues/41)) - **Production Files can be saved in an unknown state causing surprises for users down the pipeline** - Conventions are enforced manually by user (e.g. [`Check Action Names`](https://projects.blender.org/studio/blender-studio-pipeline/src/commit/32b7fc0c4925be75a48c9f99c951c16286b08ef0/scripts-blender/addons/blender_kitsu/anim/ops.py#L102-L278) or [`Check Output Collection`](https://projects.blender.org/studio/blender-studio-pipeline/src/commit/32b7fc0c4925be75a48c9f99c951c16286b08ef0/scripts-blender/addons/blender_kitsu/anim/ops.py#L36-L99)) # Solution ## Convention Check on Save Convention Checks are verifications that the file's contents match the Blender Studio Pipeline's conventions. Convention Checks will be run before users can save their work. This ensures all files in the production are aligned with the conventions we set. Therefor, as issues arise for specific tasks, we can created additional checks to accommodate those issues. Some examples of things that can be Convention Checked are: https://studio.blender.org/pipeline/archive/pipeline-proposal-2019/task-companion-add-on Once all information about files can be saved in Kitsu, then we can confidently run convention check(s) on any production files. - Determine what convention checks need to be run based on Kitsu task type - Allow users to optionally skip Convention Checks - Report to Kitsu if File is saved with/without convention check - (Optional) Only use convention check when saving a Published version in Pull pipeline https://projects.blender.org/studio/blender-studio-pipeline/issues/85 # Steps to Success 1. Make an operator that checks if the files meets conventions based on current Kitsu Task, report to Kitsu if Convention Checks have passed/failed. 2. Include existing Convention Enforcement Operators (e.g. [`Check Action Names`](https://projects.blender.org/studio/blender-studio-pipeline/src/commit/32b7fc0c4925be75a48c9f99c951c16286b08ef0/scripts-blender/addons/blender_kitsu/anim/ops.py#L102-L278) or [`Check Output Collection`](https://projects.blender.org/studio/blender-studio-pipeline/src/commit/32b7fc0c4925be75a48c9f99c951c16286b08ef0/scripts-blender/addons/blender_kitsu/anim/ops.py#L36-L99)) in Convention Checks
❤️ 1
Nick Alberelli added the
Kind
Proposal
 label 2023-06-16 20:37:01 +02:00
Nick Alberelli changed title from Convention Check Proposal to Proposal: Create Convention Check System 2023-06-16 21:25:29 +02:00
Nick Alberelli changed title from Proposal: Create Convention Check System to Proposal: Create 'Convention Check' System 2023-06-16 21:26:29 +02:00
Fiona Cohen commented 2023-06-20 16:38:07 +02:00
Collaborator
Copy Link

Hi Nick, would you already have some ideas/proposals for "Determine what convention checks need to be run based on Kitsu task type"? Other or complementary to the examples listed in the link.
Even a rough draft, as a start for the next conversations with the team?

Hi Nick, would you already have some ideas/proposals for "Determine what convention checks need to be run based on Kitsu task type"? Other or complementary to the examples listed in the link. Even a rough draft, as a start for the next conversations with the team?
Nick Alberelli commented 2023-06-21 15:10:50 +02:00
Author
Member
Copy Link

Hey @FoxGlove thanks for the feedback, so two things, firstly I think we should have a large of convention checks that can be run on any task type, and I in my opinion what conventions are run should be determine by the production, this way we can react to new things that pop up, or conventions that are defined exclusively for a certain asset or type of shot. For example on Pets we have lots of outline FX we want to check for, but maybe on the next production that wont be the case.

That being said, secondly I do still have a list of generic checks we can run across every production as follows.

Conventions Per Task Type

All Tasks

  • Relative Paths to all external Files

Shot Tasks

  • Frame Range matches Kitsu

  • Resolution/FPS matches Kitsu

    Animation

    • Output Collection matches Convention
    • Output Collection contains only data casted in Kitsu
    • Action Names match Convention

    Lighting

    • Animation Collection is linked
    • Collection Hierarchy & Names matches Convention

Asset Task Types

  • Armature Name matches convention

    Rigging Layer

    • Check objects match convention
    • Collection Hierarchy & Names matches Convention
    • Relative Paths are set

    Shading Layer

    • Armature & Collections match parent task layer
Hey @FoxGlove thanks for the feedback, so two things, firstly I think we should have a large of convention checks that can be run on any task type, and I in my opinion what conventions are run should be determine by the production, this way we can react to new things that pop up, or conventions that are defined exclusively for a certain asset or type of shot. For example on Pets we have lots of outline FX we want to check for, but maybe on the next production that wont be the case. That being said, secondly I do still have a list of generic checks we can run across every production as follows. # Conventions Per Task Type # All Tasks - Relative Paths to all external Files ## Shot Tasks - Frame Range matches Kitsu - Resolution/FPS matches Kitsu ### Animation - Output Collection matches Convention - Output Collection contains only data casted in Kitsu - Action Names match Convention ### Lighting - Animation Collection is linked - Collection Hierarchy & Names matches Convention ## Asset Task Types - Armature Name matches convention ### Rigging Layer - Check objects match convention - Collection Hierarchy & Names matches Convention - Relative Paths are set ### Shading Layer - Armature & Collections match parent task layer
👍 1
Nick Alberelli commented 2023-06-21 18:03:28 +02:00
Author
Member
Copy Link

@FoxGlove just a heads up I had a meeting with @fsiddi to go over this proposal, we have decided that I will move forward with a prototype of the convention check operator.

The part of this proposal that covered asset pipeline has been moved into it's own separate proposal studio/blender-studio-pipeline#92 as there are still some thinking that needs to be done about how that will work.

Next Steps: I am moving forward to prototype the convention check operator, and I am ready to discuss with the team what conventions need to be checked :)

@FoxGlove just a heads up I had a meeting with @fsiddi to go over this proposal, we have decided that I will move forward with a prototype of the convention check operator. The part of this proposal that covered asset pipeline has been moved into it's own separate proposal https://projects.blender.org/studio/blender-studio-pipeline/issues/92 as there are still some thinking that needs to be done about how that will work. Next Steps: I am moving forward to prototype the convention check operator, and I am ready to discuss with the team what conventions need to be checked :)
👍 1
Nick Alberelli commented 2023-06-21 18:45:23 +02:00
Author
Member
Copy Link

TODO

  • Generate config file based on task types available in kitsu
  • Create config.py that contains the task types definitions what operators go with what task type
# TODO - [ ] Generate config file based on task types available in kitsu - [ ] Create config.py that contains the task types definitions what operators go with what task type
Demeter Dzadik commented 2023-07-03 16:45:18 +02:00
Member
Copy Link

Convention checker things to check, in no particular order:

Assets Pipeline Conventions
Before pull:

  • If multiple objects use the same object data, make sure their ownership data is in sync. If not, give user an option to copy the ownership data from one of the objects to all others.
  • Warn about bones with Quaternion rotation mode.
  • Collections must never ever have use_fake_user=True, even though that is the default. (Update: It is no longer the default, yay! Still, doesn't hurt to check.)
  • Materials should be linked to Object DATA, NOT Object. (related).
  • ID naming convention update. The new conventions have now been set, now it's just a matter of implementing some input fields and checks for them.
    • Object names
    • All other ID names
  • Names that use Blender's default duplicate suffixing (.001) must be avoided, as this can result in mis-association of pointers between duplicate copies of overridden assets. In an ideal world this shouldn't happen, but for our own sanity's sake, if we want to be sure to avoid it, we should enforce this (even during layout/previz).
  • Mesh/Armature/Curve/Shapekey/etc datablocks should be named same as object.
    • (Multi-user obdatas are not yet handled)
  • NodeTrees should be prefixed with their type, GN/SH.
  • Don't allow unlocked object transforms, except on Armature objects. Update: Empties, also.
  • Don't allow invalid drivers. This is difficult because Blender's is_valid flag currently always returns True. TODO: Make a report?
  • Don't allow missing or absolute-path libraries.
  • Make sure Metarig is not part of the asset.
  • Make sure no object or collection outside of the asset is referenced by anything inside of the asset.

After push:

  • Unhide mesh vertices, edit bones, pose bones (unless this gets too expensive)
  • Lock object transforms, except Armatures
  • Hide helper collections. (Python property on Collections, displayed in Asset Pipeline UI)

Shot file conventions / tooling:

  • Don't allow linking .blend files from outside a certain directory (in our case SVN/pro)
  • Warn if two linked IDs share name; This will cause one to get a .001 suffix when both are overridden. Happened with cat and dog having same names for teeth/eyes/line objects/tons of stuff.
  • Operator to auto-rename overridden objects that have an incorrect number suffix.
  • Operator to delete an asset, and then also purge. This should be used instead of the regular "Delete Hierarchy".
  • Operator to link, override, and manage the ID names of assets when they have multiple copies in a shot, to make sure every object of an asset has the same number suffix.
  • Warn if the name (minus number suffix) of a local ID representing an override is not the same as the linked ID it is overriding.
  • Operator to create an override of a linked collection in such a way that only Armature objects are Editable Overrides, but all other objects are System Overrides.
  • Warn if non-Armature objects have editable overrides. This shouldn't normally happen.
  • If any property of any bone of any local Armature is animatable and overridden, it must be keyed in the active action, or the override should be removed. Else, pose won't be an exact match when rebuilding the scene.
  • Local Action names should adhere to the New ID naming convention.
  • No disabled animation curves in any Actions that are used by any local Armature, same problem there.
  • Warn if a different rotation mode is keyed from what is active.
  • Warn if Object.matrix_parent_inverse or HookModifier.matrix_inverse is different between the override and the linked ID. Parenting should not be overridden because there are more reliable alternatives.
  • Warn if parent object is different between override and the linked ID.
  • Warn if a Child Of constraint is present on any object or bone.
  • Warn if any object parenting type other than the default ("Object") is used. All other options should be avoided because there are more reliable alternatives.
  • Lattice Magic: Warn if affected objects have gone missing.
  • (Implemented in Animation Cupboard, could move elsewhere) Last resort Operator to nuke and re-link an asset while preserving as much data as possible.
  • (Implemented in Animation Cupboard, could move elsewhere) UI to display users and dependencies of a datablock in the Outliner.
  • (Implemented in Animation Cupboard, could move elsewhere) A better "Remap Users" operator.)
  • (Fixed on Blender's side.) Blender currently propagates the use_fake_user flag of linked IDs, resulting in un-purgable IDs. This is terrible, so before running purge, we need to Python-override those flags to False. On every single linked ID.
Convention checker things to check, in no particular order: **Assets Pipeline Conventions** Before pull: - [ ] If multiple objects use the same object data, make sure their ownership data is in sync. If not, give user an option to copy the ownership data from one of the objects to all others. - [ ] Warn about bones with Quaternion rotation mode. - [x] Collections must never ever have use_fake_user=True, ~~even though that is the default.~~ (Update: It is no longer the default, yay! Still, doesn't hurt to check.) - [x] Materials should be linked to Object DATA, NOT Object. ([related](https://projects.blender.org/blender/blender/issues/99996)). - [ID naming convention update](https://projects.blender.org/studio/blender-studio-pipeline/pulls/149). The new conventions have now been set, now it's just a matter of implementing some input fields and checks for them. - [x] Object names - [ ] All other ID names - [x] Names that use Blender's default duplicate suffixing (.001) must be avoided, as this can result in mis-association of pointers between duplicate copies of overridden assets. In an ideal world this shouldn't happen, but for our own sanity's sake, if we want to be sure to avoid it, we should enforce this (even during layout/previz). - [x] Mesh/Armature/Curve/Shapekey/etc datablocks should be named same as object. - [ ] (Multi-user obdatas are not yet handled) - [ ] NodeTrees should be prefixed with their type, GN/SH. - [x] Don't allow unlocked object transforms, except on Armature objects. Update: Empties, also. - [ ] Don't allow invalid drivers. This is difficult because Blender's `is_valid` flag currently always returns True. TODO: Make a report? - [x] Don't allow missing or absolute-path libraries. - [x] Make sure Metarig is not part of the asset. - [x] Make sure no object or collection outside of the asset is referenced by anything inside of the asset. After push: - [ ] Unhide mesh vertices, edit bones, pose bones (unless this gets too expensive) - [x] Lock object transforms, except Armatures - [ ] Hide helper collections. (Python property on Collections, displayed in Asset Pipeline UI) **Shot file conventions / tooling:** - [ ] Don't allow linking .blend files from outside a certain directory (in our case `SVN/pro`) - [ ] Warn if two linked IDs share name; This will cause one to get a .001 suffix when both are overridden. Happened with cat and dog having same names for teeth/eyes/line objects/tons of stuff. - [ ] Operator to auto-rename overridden objects that have an incorrect number suffix. - [ ] Operator to delete an asset, and then also purge. This should be used instead of the regular "Delete Hierarchy". - [ ] Operator to link, override, and manage the ID names of assets when they have multiple copies in a shot, to make sure every object of an asset has the same number suffix. - [ ] Warn if the name (minus number suffix) of a local ID representing an override is not the same as the linked ID it is overriding. - [ ] Operator to create an override of a linked collection in such a way that only Armature objects are Editable Overrides, but all other objects are System Overrides. - [ ] Warn if non-Armature objects have editable overrides. This shouldn't normally happen. - [ ] If any property of any bone of any local Armature is animatable and overridden, it must be keyed in the active action, or the override should be removed. Else, pose won't be an exact match when rebuilding the scene. - [ ] Local Action names should adhere to the [New ID naming convention](https://projects.blender.org/studio/blender-studio-pipeline/pulls/149). - [ ] No disabled animation curves in any Actions that are used by any local Armature, same problem there. - [ ] Warn if a different rotation mode is keyed from what is active. - [ ] Warn if Object.matrix_parent_inverse or HookModifier.matrix_inverse is different between the override and the linked ID. Parenting should not be overridden because there are more reliable alternatives. - [ ] Warn if parent object is different between override and the linked ID. - [ ] Warn if a Child Of constraint is present on any object or bone. - [ ] Warn if any object parenting type other than the default ("Object") is used. All other options should be avoided because there are more reliable alternatives. - [ ] Lattice Magic: Warn if affected objects have gone missing. - [x] *(Implemented in Animation Cupboard, could move elsewhere)* ~~Last resort Operator to nuke and re-link an asset while preserving as much data as possible.~~ - [x] *(Implemented in Animation Cupboard, could move elsewhere)* ~~UI to display users and dependencies of a datablock in the Outliner.~~ - [x] *(Implemented in Animation Cupboard, could move elsewhere)* ~~A better "Remap Users" operator.)~~ - [x] *(Fixed on Blender's side.)* ~~Blender currently propagates the use_fake_user flag of linked IDs, resulting in un-purgable IDs. This is terrible, so before running purge, we need to Python-override those flags to False. On every single linked ID.~~
❤️ 1
Simon Thommes commented 2023-07-03 16:57:07 +02:00
Member
Copy Link

Would love to talk about these things in detail once there is time after Pets

Would love to talk about these things in detail once there is time after Pets
❤️ 1
Demeter Dzadik commented 2023-09-15 20:04:51 +02:00
Member
Copy Link

Additional things we learned would be useful in the Convention Checker, from the Pets cleanup process, have been added to my previous comment.

Additional things we learned would be useful in the Convention Checker, from the Pets cleanup process, have been added to my previous comment.
Demeter Dzadik commented 2023-11-15 15:21:50 +01:00
Member
Copy Link

Re-collected, re-categorized, and re-phrased all of the above points, now that we're getting closer to production and having a convention checker. It's a big friendly list.

Re-collected, re-categorized, and re-phrased all of the above points, now that we're getting closer to production and having a convention checker. It's a big friendly list.
Demeter Dzadik commented 2024-02-15 15:27:15 +01:00
Member
Copy Link

We never came together on a final, approved, clear design for the convention check system that makes everybody happy. Tbh it seemed impossible. With the reality of production setting in, I started to feel like my needs are quite clear and quite simple, so I could address them pretty quickly. Yesterday I created the Blender Log add-on, and added some interactions between it, and our Asset Pipeline, and our asset hooks, and I hope you guys will like the results.

I think this can be a great starting point to move this discussion forward, as it previously felt quite stuck. Now we have something we can look at and talk about, whenever somebody wants to integrate this with more areas of our pipeline, we can see if it's possible and convenient, or if it needs to be taken back to the drawing board. It only took one day, so my heart won't be broken if we decide to throw it all out.

I updated the big list above once again, to keep track of which points are currently being addressed in one way or another.

We never came together on a final, approved, clear design for the convention check system that makes everybody happy. Tbh it seemed impossible. With the reality of production setting in, I started to feel like my needs are quite clear and quite simple, so I could address them pretty quickly. Yesterday I created the [Blender Log](https://projects.blender.org/studio/blender-studio-pipeline/src/branch/main/scripts-blender/addons/blender_log) add-on, and added some interactions between it, and our Asset Pipeline, and our asset hooks, and I hope you guys will like the results. I think this can be a great starting point to move this discussion forward, as it previously felt quite stuck. Now we have something we can look at and talk about, whenever somebody wants to integrate this with more areas of our pipeline, we can see if it's possible and convenient, or if it needs to be taken back to the drawing board. It only took one day, so my heart won't be broken if we decide to throw it all out. I updated the big list above once again, to keep track of which points are currently being addressed in one way or another.
Nick Alberelli commented 2024-02-15 16:43:36 +01:00
Author
Member
Copy Link

@Mets I am a big fan of your implementation of Blender Log, I wish we had a centralized reporting system for a while now. I was always concerned because I was told that an "add-on operating system" is to be avoided, but truely, I don't see a better way to accomplish this then doing that.

@Mets I am a big fan of your implementation of Blender Log, I wish we had a centralized reporting system for a while now. I was always concerned because I was told that an "add-on operating system" is to be avoided, but truely, I don't see a better way to accomplish this then doing that.
❤️ 1
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels   
Kind
Breaking
Breaking change that won't be backward compatible

  
Kind
Bug
Something is not working

  
Kind: Community

Issues opened by members of the community / outside the studio

  
Kind
Documentation
Documentation changes

  
Kind
Easy
Simple to complete

  
Kind
Enhancement
Improve existing functionality

  
Kind
Feature
New functionality

  
Kind
Proposal
A proposal or idea that requires feedback

  
Kind
Security
This is security issue

  
Kind
Studio Request
Requested by a user at the Blender-Studio

  
Kind
Testing
Issue or pull request related to testing

  
Priority
Critical
The priority is critical

  
Priority
High
The priority is high

  
Priority
Low
The priority is low

  
Priority
Medium
The priority is medium

  
Reviewed
Confirmed
Issue has been confirmed

  
Reviewed
Duplicate
This issue or pull request already exists

  
Reviewed
Invalid
Invalid issue

  
Reviewed
Won't Fix
This issue won't be fixed

  
Status
Abandoned
Somebody has started to work on this but abandoned work

  
Status
Blocked
Something is blocking this issue or pull request

  
Status
Need More Info
Feedback is required to reproduce issue or to continue work

No Label
Kind
Breaking
Kind
Bug
Kind: Community
Kind
Documentation
Kind
Easy
Kind
Enhancement
Kind
Feature
Kind
Proposal
Kind
Security
Kind
Studio Request
Kind
Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: studio/blender-studio-tools#84
No description provided.
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?