New naming convention proposal #149

Merged
Demeter Dzadik merged 12 commits from Mets/blender-studio-pipeline:mets-naming-conventions into main 2023-11-23 17:11:47 +01:00
Member

A major overhaul of our in-blend naming conventions.

Goals:

  • Every asset datablock must have a unique name to avoid Override issues.
  • Separators could be more consistent and meaningful.
  • Cover more cases, eg. node groups, bones, etc.
  • Simply bring things up to date to align with our new preferences based on previous production experiences.
  • Make sure there's some reasoning behind everything, and it's not just a legacy convention carried from old times.

These changes have now been discussed and agreed upon between myself, Andy and Simon, so as far as I'm concerned, they can be committed soon if nobody comes up with any new objections.

This can also provide some foundation for the naming aspects of an upcoming Convention Checker system.

A major overhaul of our in-blend naming conventions. Goals: - Every asset datablock must have a unique name to avoid Override issues. - Separators could be more consistent and meaningful. - Cover more cases, eg. node groups, bones, etc. - Simply bring things up to date to align with our new preferences based on previous production experiences. - Make sure there's some reasoning behind everything, and it's not just a legacy convention carried from old times. These changes have now been discussed and agreed upon between myself, Andy and Simon, so as far as I'm concerned, they can be committed soon if nobody comes up with any new objections. This can also provide some foundation for the naming aspects of an upcoming Convention Checker system.
Demeter Dzadik added 1 commit 2023-09-15 22:28:12 +02:00
A major overhaul of our in-blend naming conventions.

Goals:
- Every asset datablock must have a unique name to avoid Override issues
- Separators could be more consistent and meaningful
- No unnecessary strictness, eg. for Pose Library
Demeter Dzadik added 1 commit 2023-09-15 22:30:35 +02:00
Demeter Dzadik added 1 commit 2023-09-15 22:33:10 +02:00
Member

Looks good to me! :)

I appreciate that you added more detailed descriptions for things.

Looks good to me! :) I appreciate that you added more detailed descriptions for things.
Demeter Dzadik added 1 commit 2023-09-18 11:01:52 +02:00
- Make it explicit that if a name includes the full name of the asset,
using the mnemonic is not necessary.
- Add `TMP` name prefix suggestion
- Add more detailed but less wordy explanation about why unique ID names
are important.
Andy Goralczyk reviewed 2023-09-18 13:01:27 +02:00
Andy Goralczyk reviewed 2023-09-18 13:04:39 +02:00
@ -8,2 +10,3 @@
We use prefixes only for top level collections of an asset. This is important to distinguish types of assets. The name of the asset itself should be lowercase.
All local datablocks across all assets of a production must have a unique name. To faciliate this, each asset is assigned a mnemonic that must remain unique within a given production. For example, a character named "Elder Sprite" might get the mnemonic "ES". The max length of this mnemonic is not limited, but the shorter the better. If a minor prop in the production is called "Electric Switch", it can not have the mnemonic "ES", because it's already taken. So, "ELSW" would be fine.
Member

The mnemonics should have more 4 letters (more than the 2 and 3) to distinguish them from asset prefixes. otherwise it would be easy to confuse the two. maybe making them lower case would help too.

The mnemonics should have more 4 letters (more than the 2 and 3) to distinguish them from asset prefixes. otherwise it would be easy to confuse the two. maybe making them lower case would help too.
Author
Member

If we don't mind longer ID names, I'm happy to use just a slightly shortened version of names, eg. "elder" for Elder Sprite or "rocket_int" for Rocket Interior.

If we don't mind longer ID names, I'm happy to use just a slightly shortened version of names, eg. "elder" for Elder Sprite or "rocket_int" for Rocket Interior.
Andy Goralczyk reviewed 2023-09-18 13:07:38 +02:00
@ -22,3 +54,3 @@
All objects should have a prefix, followed by a dash that determines their type:
It is advisable to give Objects an additional prefix to provide information about the object's purpose, especially when it is NOT part of an asset. This is purely for organizational and comfort purposes. Here are some recommendations:
Member

This needs stronger wording than "should be advisable", every object datablock should have that prefix to make it clear when linking/appending what that object is and to visually distinguish the objects in the outliner and python query.

This needs stronger wording than "should be advisable", every object datablock should have that prefix to make it clear when linking/appending what that object is *and* to visually distinguish the objects in the outliner and python query.
Author
Member

Gotcha, will replace "should" with "must".

Gotcha, will replace "should" with "must".
Andy Goralczyk reviewed 2023-09-18 13:14:40 +02:00
@ -36,2 +69,3 @@
Example: `GEO-WRDB-drawer_knob.L` for the left-side drawer knob of a wardrobe prop.
If a name contains a *of* relationship - in the above example the `drawer` of the `dresser` - these should not be separated by a dot, but rather with an underscore.
If a name contains an "*of*" relationship - in the above example the `knob` *of* the `drawer` - these should NOT be separated by `.`, but rather with `_`.
Member

according to the first section, _ is used only as spacebar replacement, it's not a separator. so an of relationship could be - instead

according to the first section, `_` is used only as spacebar replacement, it's not a separator. so an _of_ relationship could be `-` instead
Author
Member

Thinking about it now, I actually don't think an of relationship needs to be signified by the ID name at all, since it has no technical significance in the pipeline. To me, it's just the same as any other word separation. Collections also help with this.

Thinking about it now, I actually don't think an *of* relationship needs to be signified by the ID name at all, since it has no technical significance in the pipeline. To me, it's just the same as any other word separation. Collections also help with this.
Andy Goralczyk reviewed 2023-09-18 13:16:05 +02:00
@ -51,1 +91,3 @@
- `ANI` Animation to be used in a shot
Examples:
```
Hand_Fist
Member

we established the "no caps, no gaps" rule at some point, should be used here as well. so no uppercase letters except for prefix and suffixes.

we established the "no caps, no gaps" rule at some point, should be used here as well. so no uppercase letters except for prefix and suffixes.
Simon Thommes reviewed 2023-09-18 13:18:20 +02:00
Simon Thommes left a comment
Member

I have a few notes, marked in the 'code'.

In the future I'd propose to do design tasks first as design tasks. That makes it easier to have a back and forth, as this is not about implementation details until we agree on the design.

We should probably also just talk about this in a meeting. That seems more efficient for such a broad topic.

I have a few notes, marked in the 'code'. In the future I'd propose to do design tasks first as design tasks. That makes it easier to have a back and forth, as this is not about implementation details until we agree on the design. We should probably also just talk about this in a meeting. That seems more efficient for such a broad topic.
@ -6,0 +3,4 @@
## Separators
We only have 3 separators to work with, so we try keep their meaning consistent across conventions:
`-` : Separates prefixes from each other and from the rest of the name.
`_` : Used instead of spacebar in composite words, eg. `eifel_tower_base`.
Member

I agree with this characterization of _ which to me stands in confilct of the proposal to use it for hierarchies/parts later in this document.

I agree with this characterization of `_` which to me stands in confilct of the proposal to use it for hierarchies/parts later in this document.
Author
Member

I think that later part of the document is over-explaining a bit, and makes things that I see as part of the base name (without any suffixes or prefixes) sound as if they were something technically significant. I'll try to clarify this better.

I think that later part of the document is over-explaining a bit, and makes things that I see as part of the base name (without any suffixes or prefixes) sound as if they were something technically significant. I'll try to clarify this better.
@ -6,0 +4,4 @@
We only have 3 separators to work with, so we try keep their meaning consistent across conventions:
`-` : Separates prefixes from each other and from the rest of the name.
`_` : Used instead of spacebar in composite words, eg. `eifel_tower_base`.
`.` : Suffixes, used only for symmetry sides, ie `.L`/`.R`, and for variations, `clock.normal`, `clock.broken`, `clock.destroyed`
Member

To me these two categories are completely separate from each other. There could also be variations of symmetric objects.

I'd propose to use . generally for parts/hierarchies like we have in the past. .L/R is compatible with that definition imo.
Variations are for me something orthogonal to that. Could we introduce :? Not sure why we are limited to those 3, as compatibility shouldn't be an issue.

To me these two categories are completely separate from each other. There could also be variations of symmetric objects. I'd propose to use `.` generally for parts/hierarchies like we have in the past. `.L/R` is compatible with that definition imo. Variations are for me something orthogonal to that. Could we introduce `:`? Not sure why we are limited to those 3, as compatibility shouldn't be an issue.
Author
Member

I agree with introducing : as a separator for variants, I only didn't do this because I wasn't sure if you guys are open to it.

Only note is that I would still always put .L/.R at the very end, even if there is a variation, otherwise blender's built-in name flipping function wouldn't work. (Which to be fair isn't used for any built-in operators in Object mode, but for add-ons it's still handy.)

I agree with introducing `:` as a separator for variants, I only didn't do this because I wasn't sure if you guys are open to it. Only note is that I would still always put `.L/.R` at the very end, even if there is a variation, otherwise blender's built-in name flipping function wouldn't work. (Which to be fair isn't used for any built-in operators in Object mode, but for add-ons it's still handy.)
@ -6,2 +7,3 @@
`.` : Suffixes, used only for symmetry sides, ie `.L`/`.R`, and for variations, `clock.normal`, `clock.broken`, `clock.destroyed`
## Collection names
## Asset Mnemonic
Member

I had to google what mnemonic means. Maybe we can move to something more intuitive like identifier?

I had to google what mnemonic means. Maybe we can move to something more intuitive like identifier?
Author
Member

Agreed!

Agreed!
@ -16,2 +28,3 @@
Note that there's no technical distinction between different types of assets. This is purely for organizational purposes and comfort.
Example: `CH-phileas`
The immediate sub-collections of the root collection are strictly defined by our Asset Pipeline add-on, as being `{mnemonic}-{task_layer}`. Task Layers are the different data layers that make up an asset, such as Modeling, Rigging, and Shading. Inside these Task Layer Collections, each artist responsible for their own Task Layer may create sub-collections freely, as long as each sub-collection still starts with `{mnemonic}-{task_layer}-`.
Member

I don't remember this being a necessity in the current design of the asset pipeline. Maybe I'm misremembering but I don't generally think that binding the tasklayer association with the name is a good idea, since that will make changing this without breakage impossible.

I don't remember this being a necessity in the current design of the asset pipeline. Maybe I'm misremembering but I don't generally think that binding the tasklayer association with the name is a good idea, since that will make changing this without breakage impossible.
Author
Member

It is, it's necessary for each task layer to be assigned its own piece of the hierarchy, otherwise the pipeline doesn't know which hierarchy to keep; The one being pulled in, or the one already in the current file. So when pulling into Rigging currently, it will preserve the current hierarchy for the Rigging collection, but take the hierarchy of the other collections from the publish.

It is, it's necessary for each task layer to be assigned its own piece of the hierarchy, otherwise the pipeline doesn't know which hierarchy to keep; The one being pulled in, or the one already in the current file. So when pulling into Rigging currently, it will preserve the current hierarchy for the Rigging collection, but take the hierarchy of the other collections from the publish.
Member

But I thought we agreed on storing this data as custom properties and keeping the hierarchies out of it, no?
Because this isn't just about the exceptions, right? Every collection would have that. Having the tasklayer name in every single collection of an asset seems quite off to me, that's not something I remember considering. Maybe that was a misunderstanding at the time.

But I thought we agreed on storing this data as custom properties and keeping the hierarchies out of it, no? Because this isn't just about the exceptions, right? Every collection would have that. Having the tasklayer name in every single collection of an asset seems quite off to me, that's not something I remember considering. Maybe that was a misunderstanding at the time.
Member

So as far as the Asset Pipeline is concerned, we did agree on storing ownership information inside property groups on the individual objects, and that is how ownership of objects is handled by the Asset Pipeline.

But for consistency in the organization of collections each task layer has a collection, the user can assign objects to their task layer collection. These top-level collections are named after the task layers and are purely organizational and don't influence who owns what object, but they are necessary to avoid conflicts in collection assignments during Push/Pull. Personally I prefer the collection set-up like this, but I'm open to discussing alternatives.

It is possible to replace this feature with some kind of ownership system and UI similar to objects for collections, but that not within the scope of things we are currently focused on. We are more focused on finalizing the transfer logic and getting 'ownership' working on non-object IDs like GeoNode groups.

So as far as the Asset Pipeline is concerned, we did agree on storing ownership information inside property groups on the individual objects, and that is how ownership of objects is handled by the Asset Pipeline. But for consistency in the organization of collections each task layer has a collection, the user can assign objects to their task layer collection. These top-level collections are named after the task layers and are purely organizational and don't influence who owns what object, but they are necessary to avoid conflicts in collection assignments during Push/Pull. Personally I prefer the collection set-up like this, but I'm open to discussing alternatives. It is possible to replace this feature with some kind of ownership system and UI similar to objects for collections, but that not within the scope of things we are currently focused on. We are more focused on finalizing the transfer logic and getting 'ownership' working on non-object IDs like GeoNode groups.
Author
Member

It's actually not necessary for the sub-collections' names to include the task layer name, that's true. That was more of a stylistic choice on my end. So you're right, this is in fact legal:

CH-elder_sprite
    esprite-model
        esprite-head
        esprite-eyes
            esprite-eye.L
            esprite-eye.R
    esprite-rig
    esprite-shading

I think it might be a bit nicer with the task layer names, when seeing the collections as a flat list, like when appending or using the Blend File view of the Outliner. Also because multiple task layers might have their own collection relating to a given part of a character. But I don't mind too much if this isn't strictly enforced.

It's actually not necessary for the sub-collections' names to include the task layer name, that's true. That was more of a stylistic choice on my end. So you're right, this is in fact legal: ``` CH-elder_sprite esprite-model esprite-head esprite-eyes esprite-eye.L esprite-eye.R esprite-rig esprite-shading ``` I think it might be a bit nicer with the task layer names, when seeing the collections as a flat list, like when appending or using the Blend File view of the Outliner. Also because multiple task layers might have their own collection relating to a given part of a character. But I don't mind too much if this isn't strictly enforced.
@ -18,0 +31,4 @@
So, here's what an asset's collection hierarchy might look like:
```txt
CH-elder_sprite
ES-model
Member

I find it quite confusing if the main asset collection prefix and any other collection prefix share the exact same formatting. I'd propose to keep those clearly different to avoid confusion and improve readability.

So either making the identifier not capitalized or using a different separator or both.

I find it quite confusing if the main asset collection prefix and any other collection prefix share the exact same formatting. I'd propose to keep those clearly different to avoid confusion and improve readability. So either making the identifier not capitalized or using a different separator or both.
Author
Member

This seems to align with Andy's suggestion of keeping these identifiers longer. In that case, I agree with lower-case, so instead of "ES" or "elder_sprite", we could just have "esprite" or "elder" (up to whoever is first creating the asset). For most assets, especially characters, our names tend to be quite short, so additional shortening isn't always needed. Only downside is that maybe sometimes we'll end up with some pretty long names, but it's not the end of the world.

This seems to align with Andy's suggestion of keeping these identifiers longer. In that case, I agree with lower-case, so instead of "ES" or "elder_sprite", we could just have "esprite" or "elder" (up to whoever is first creating the asset). For most assets, especially characters, our names tend to be quite short, so additional shortening isn't always needed. Only downside is that maybe sometimes we'll end up with some pretty long names, but it's not the end of the world.
Author
Member

We should probably also just talk about this in a meeting. That seems more efficient for such a broad topic.

We should definitely have a meeting! Depending on how many people we want to pull into that, since this affects basically the entire team, it's really valuable to get some feedback in advance. I think I can incorporate pretty much all of it, and then if people can find time to give further feedback here, they can, and it's much appreciated. Otherwise, let's have a meeting next week, since this week I don't want to pull away people's focus from working with Jerrica.

> We should probably also just talk about this in a meeting. That seems more efficient for such a broad topic. We should definitely have a meeting! Depending on how many people we want to pull into that, since this affects basically the entire team, it's really valuable to get some feedback in advance. I think I can incorporate pretty much all of it, and then if people can find time to give further feedback here, they can, and it's much appreciated. Otherwise, let's have a meeting next week, since this week I don't want to pull away people's focus from working with Jerrica.
Demeter Dzadik added 2 commits 2023-09-19 15:41:27 +02:00
Demeter Dzadik added 1 commit 2023-09-20 15:44:59 +02:00
Demeter Dzadik added 1 commit 2023-11-07 17:12:44 +01:00
Demeter Dzadik added 1 commit 2023-11-15 18:47:42 +01:00
- Dashes can be used to signify a sort of hierarchy
- Variants are still separated by : but in a way that sort of merges
into the hierarchy defined by the dashes,
eg. `wardrobe:burned-shelf-book`
- Asset identifier mostly non-controversial; Shortened name is optional,
but try to use it. Asset identifiers can have underscores and numbers,
no problem. (coral_001)
- Collection prefixes should only be used for root collection.
(Put this right after the Asset Collection Hierarchy header!)
- Task layer names no longer have to be included in collection names.
- Expand the concept of "Asset Identifier" to be more like a
"Namespace Identifier", and mention that it should also be used for library IDs
that aren't part of an (Asset Pipeline) asset. (instead of the asset's
name, use the file's name, for example.)
- Add SH-, GN-, CM- prefix conventions for node groups.
- Small correction: Batch Rename Datablocks is not an addon.
- Small correction: `GEO-house-wooden_plank.023` had inconsistent names
- "Of" relationships should no longer be part of the base name.
Recommended hierarchy based naming using dashes.
- Variants are now a bit more complex, try to explain that properly.
- Pose library poses should also have the asset identifier.
- Animation versions now separated by dash instead of dot.
- Change camera rig prefix from CAM to CA.
- Only one prefix, still separated by dash.
Demeter Dzadik added 1 commit 2023-11-16 14:46:06 +01:00
Andy Goralczyk requested changes 2023-11-20 12:16:13 +01:00
@ -32,0 +75,4 @@
For complex assets, try to use dashes to indicate a nested hierarchy of objects, eg:
```
PR-wardrobe
wardrobe.model
Member

dot . should become a dash -
everything else looks good and can be merged as far as I am concerned :)

dot `.` should become a dash `-` everything else looks good and can be merged as far as I am concerned :)
Author
Member

Thanks, fixed!

Thanks, fixed!
Demeter Dzadik added 1 commit 2023-11-23 16:45:45 +01:00
Demeter Dzadik closed this pull request 2023-11-23 17:06:35 +01:00
Demeter Dzadik reopened this pull request 2023-11-23 17:06:56 +01:00
Demeter Dzadik added 1 commit 2023-11-23 17:10:03 +01:00
Demeter Dzadik merged commit f2e2a70f9d into main 2023-11-23 17:11:47 +01:00
Sign in to join this conversation.
No reviewers
No Milestone
No project
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#149
No description provided.