Asset Pipeline: Improvements and Fixes #170

Open
opened 2023-11-24 20:31:39 +01:00 by Nick Alberelli · 17 comments
Member

TODO copied from studio/blender-studio-pipeline#145

High

Medium

Low

  • Un-Surrender Option (to cancel a surrender before Push)
  • When Claiming Surrendered remove old owner and assign to local or default if u can 478554a409
  • When surrendered data is claimed and then deleted, there is nothing in place to enforce that deletion source
  • Operator to split up existing file (hard)
  • Add UI to revert file after successful Pull 20b903e7fd
  • Task Layer Locking
  • Show New Transfer Data after Pull
  • Consider integration with Kitsu (meta data etc)
  • Fancy changing ownership user should be able to surrender a ownership 432e9b840b
  • Consider New Validate Ownership Feature (By Pulling but not Merging)
  • Issue with Saving on Push source & Enforcing Pulling on Push source studio/blender-studio-pipeline#199

For Next Production

  • Switch to ID Level Ownership

BUGS

  • Switch Parent Relationship to always exist like materials (parents don't get removed) 272600a546
  • Fix Bug where update collection ownership runs before invalid objects 1af2d41bdc
  • Bug where Vertex Groups wont transfer in 4.0 9846af3db2
  • Merge Fails if not in object mode (always go back to OBJ Mode during merge) 1af2d41bdc
  • unmark external collection as asset during merge 20b903e7fd
  • Staged/Review Publishes are marked as asset 20b903e7fd
  • Refactor Transfer Data Merge Code to avoid Claimed Surrendered Conflict d31a624df1
    • If data as been surrendered and claimed, any out of date file will report the conflict between these two datas as a merge conflict, even if neither is part of the local task layer.
  • UI Error if Collection is not set & Collection Selector is unset on an Asset c8f5e04936
  • Modifiers Move in Stack when Owner Changes ab1d98b025
  • Geo-Nodes sockets don't get initialized Correctly
TODO copied from https://projects.blender.org/studio/blender-studio-pipeline/pulls/145 ### High - [x] Add UV Seams to Transfer Data https://projects.blender.org/studio/blender-studio-pipeline/pulls/197 - [x] Add Category Dropdown based on `blender_assets.cats.txt` https://projects.blender.org/studio/blender-studio-pipeline/pulls/185 - [x] Improve performance: https://projects.blender.org/studio/blender-studio-pipeline/pulls/235 ### Medium - [x] Convention Checker Integration / Enforce Naming Conventions https://projects.blender.org/studio/blender-studio-pipeline/pulls/208, https://projects.blender.org/studio/blender-studio-pipeline/commit/df6e8943e8904668ce5a668ed6d7f6f9788f1c97 - [x] @Mets to deliver list of conventions to check https://projects.blender.org/studio/blender-studio-pipeline/issues/84 - [x] Check for prefix name conflicts `RIG.name == MOD.name` https://projects.blender.org/studio/blender-studio-pipeline/commit/ab1d98b025db6df4217952014f420be63e06ce6b - ~~Post Merge Actions like resetting armatures, removing actions and applying modifiers~~ https://projects.blender.org/studio/blender-studio-pipeline/pulls/208 - ~~Create Separate Operator for Publish Review~~ - [x] Ensure OBJS don’t share mesh data blocks until we have ID level ownership https://projects.blender.org/studio/blender-studio-pipeline/commit/1af2d41bdc71c414c17568bbf8162b99d47b5a64 - [x] Rename review to Sandbox / Test Version https://projects.blender.org/studio/blender-studio-pipeline/pulls/242 - [x] Add button to open current Push/Pull Target https://projects.blender.org/studio/blender-studio-pipeline/pulls/241 ### Low - [ ] Un-Surrender Option (to cancel a surrender before Push) - [x] When Claiming Surrendered remove old owner and assign to local or default if u can https://projects.blender.org/studio/blender-studio-pipeline/commit/478554a409167375a7d2cb939492d42ea2ab4558 - [ ] When surrendered data is claimed and then deleted, there is nothing in place to enforce that deletion [source](https://projects.blender.org/studio/blender-studio-pipeline/issues/170#issuecomment-1098186) - [ ] Operator to split up existing file (hard) - [x] Add UI to revert file after successful Pull https://projects.blender.org/studio/blender-studio-pipeline/commit/20b903e7fd4ddcc05e52fa7756b4bfa07391eb70 - ~~Task Layer Locking~~ - [ ] Show New Transfer Data after Pull - ~~Consider integration with Kitsu (meta data etc)~~ - [x] Fancy changing ownership user should be able to surrender a ownership https://projects.blender.org/studio/blender-studio-pipeline/commit/432e9b840b019bbaac3ed00d819c9d5bd0ecd463 - ~~Consider New Validate Ownership Feature (By Pulling but not Merging)~~ - [x] Issue with Saving on Push [source](https://projects.blender.org/studio/blender-studio-pipeline/issues/170#issuecomment-1098854) & Enforcing Pulling on Push [source](https://projects.blender.org/studio/blender-studio-pipeline/issues/170#issuecomment-1098897) https://projects.blender.org/studio/blender-studio-pipeline/pulls/199 ### For Next Production - [ ] Switch to ID Level Ownership ### BUGS - [x] Switch Parent Relationship to always exist like materials (parents don't get removed) https://projects.blender.org/studio/blender-studio-pipeline/commit/272600a546b60f42a21bdcdbc930c589d9e218c6 - [x] Fix Bug where update collection ownership runs before invalid objects https://projects.blender.org/studio/blender-studio-pipeline/commit/1af2d41bdc71c414c17568bbf8162b99d47b5a64 - [x] Bug where Vertex Groups wont transfer in 4.0 https://projects.blender.org/studio/blender-studio-pipeline/commit/9846af3db2048fe906b3812c58102487a5659148 - [x] Merge Fails if not in object mode (always go back to OBJ Mode during merge) https://projects.blender.org/studio/blender-studio-pipeline/commit/1af2d41bdc71c414c17568bbf8162b99d47b5a64 - [x] unmark external collection as asset during merge https://projects.blender.org/studio/blender-studio-pipeline/commit/20b903e7fd4ddcc05e52fa7756b4bfa07391eb70 - [x] Staged/Review Publishes are marked as asset https://projects.blender.org/studio/blender-studio-pipeline/commit/20b903e7fd4ddcc05e52fa7756b4bfa07391eb70 - [x] Refactor Transfer Data Merge Code to avoid Claimed Surrendered Conflict https://projects.blender.org/studio/blender-studio-pipeline/commit/d31a624df1631fc021702b076f7cb05570bc277c - _If data as been surrendered and claimed, any out of date file will report the conflict between these two datas as a merge conflict, even if neither is part of the local task layer._ - [x] UI Error if Collection is not set & Collection Selector is unset on an Asset https://projects.blender.org/studio/blender-studio-pipeline/commit/c8f5e04936c01b50fa0e6a4f157cc51218105f0e - [x] Modifiers Move in Stack when Owner Changes https://projects.blender.org/studio/blender-studio-pipeline/commit/ab1d98b025db6df4217952014f420be63e06ce6b - [ ] Geo-Nodes sockets don't get initialized Correctly
Nick Alberelli added the
Kind
Bug
Kind
Studio Request
Kind
Feature
labels 2023-11-24 20:31:39 +01:00
Member

Convention checker list is currently here: studio/blender-studio-pipeline#84 (comment)
Can be moved somewhere else if needed.

Also added a bullet point there just now about multi-user object datas.

Convention checker list is currently here: https://projects.blender.org/studio/blender-studio-pipeline/issues/84#issuecomment-971447 Can be moved somewhere else if needed. Also added a bullet point there just now about multi-user object datas.

Hi!
Thanks for that big work!
I am currently testing the V2 asset pipeline. And I am trying to contribute by giving my feedback.

First of all, the tool looks really good and performs these functions, which is really cool and completes a real missing functionnality in Blender. Too bad, it’s not co-branded with a reference system(link).

In the V1, it was possible to versionning the tasks in order to group them in work/publish(staged/active) versions, this is not the case for the moment with the V2. Did I miss some things? Don’t you have plans?

When working, it is sometimes necessary to switch from modeling file to rigging file and vice versa. However it is necessary to go to get the file by hand in the folder. Knowing the location and the name of the files of the tasks, wouldn’t it be useful to have buttons in order to open the file staged or publish or modeling, rigging, shading etc in order to access it in a single click? (a bit like the addon "Edit Linked Library"), that should be really usefull, isn't it ?

Can someone like me help this project by coding some features, ui etc?

Hi! Thanks for that big work! I am currently testing the V2 asset pipeline. And I am trying to contribute by giving my feedback. First of all, the tool looks really good and performs these functions, which is really cool and completes a real missing functionnality in Blender. Too bad, it’s not co-branded with a reference system(link). In the V1, it was possible to versionning the tasks in order to group them in work/publish(staged/active) versions, this is not the case for the moment with the V2. Did I miss some things? Don’t you have plans? When working, it is sometimes necessary to switch from modeling file to rigging file and vice versa. However it is necessary to go to get the file by hand in the folder. Knowing the location and the name of the files of the tasks, wouldn’t it be useful to have buttons in order to open the file staged or publish or modeling, rigging, shading etc in order to access it in a single click? (a bit like the addon "Edit Linked Library"), that should be really usefull, isn't it ? Can someone like me help this project by coding some features, ui etc?
Member

group them in work/publish(staged/active) versions

I believe previously a publish had a status, which was only stored in a metadata .xml file, and possibly synced to Kitsu. Today, there's Published, Staged and Review states, which are their own folders, and they are meant for specific use cases (and their names might still change). In short: "Published" means the asset is considered ready to be used in shots. "Staged" means the asset needs work from other asset artists before it can be used in shots. "Review" is sandbox to share ideas.

Knowing the location and the name of the files of the tasks

Currently, the file names of the working files aren't hardcoded to any convention, they can be named anything. But you can just press Ctrl+O to open a neighbouring working file pretty fast as it is.

Can someone like me help this project by coding some features, ui etc?

Thanks for picking up the project so quick and testing it and wanting to contribute. I would say the project is still too young to welcome contributors, but Nick can correct me.

> group them in work/publish(staged/active) versions I believe previously a publish had a status, which was only stored in a metadata .xml file, and possibly synced to Kitsu. Today, there's Published, Staged and Review states, which are their own folders, and they are meant for specific use cases (and their names might still change). In short: "Published" means the asset is considered ready to be used in shots. "Staged" means the asset needs work from other asset artists before it can be used in shots. "Review" is sandbox to share ideas. > Knowing the location and the name of the files of the tasks Currently, the file names of the working files aren't hardcoded to any convention, they can be named anything. But you can just press Ctrl+O to open a neighbouring working file pretty fast as it is. > Can someone like me help this project by coding some features, ui etc? Thanks for picking up the project so quick and testing it and wanting to contribute. I would say the project is still too young to welcome contributors, but Nick can correct me.
Author
Member

Yes, @Mets is correct we aren't ready yet for outside contributors yet, as this add-on is very new and most of the changes are being driven by the Blender Studio but I will update this issue with information about how to contribute and some clear TODOs for contribution once we are ready to accept contributions. Thank you for your interest @Eqkoss Any comments or ideas you have are welcome.

Yes, @Mets is correct we aren't ready yet for outside contributors yet, as this add-on is very new and most of the changes are being driven by the Blender Studio but I will update this issue with information about how to contribute and some clear TODOs for contribution once we are ready to accept contributions. Thank you for your interest @Eqkoss Any comments or ideas you have are welcome.

Thanks Demeter and Nick for your answers.
No problem, I will continu to explore the addon and try to give you relevant feedbacks.

atm what I can tell you is:

  • For me, it is not clear that it is necessary to push(+save ?) after claiming a surrender, couldn't it be automated ? Maybe I did something wrong ?

  • I'm not sure for this point, but I think the task_layers.json reading is a little bit bugged.
    I create my own one and for "MATERIAL" I change the "default_owner" from "Modeling" to "Shading" and change "auto_surrender" from "true" to "false". But When I create an asset the materials data are owned by the Modeling and not surrended.(the task_layers that I used is share in this message. To let you know, Yes I add the dir in the "Custom Task Layers" Preference. The Grooming Task Layer was perfectly add)

  • It seems that the Push&Pull system is unidirectional: from tasks to staged/active files. Shouldn’t it be bidirectional? (having the possibility to push to MOD/SHD/RIG/etc from the staged/active file and pull MOD/SHD/RIG from tasks file ?)

  • When a staged (or an active) file is create, a “Task Layer Config is invalid” label appears, because no “task_layers.json” is created in the folder. Is it relevant/usefull ? If it’s to “lock” the Push&Pull system in the staged/publish file, shouldn’t you disable “Asset Management” menu or recreate an ui without the top part (only keep, avanced, tools and ownership Inspector) ?
    image

Hope that's feedback will be relevant for you (And I’m sorry if they’re misuses from me)

Thanks Demeter and Nick for your answers. No problem, I will continu to explore the addon and try to give you relevant feedbacks. atm what I can tell you is: - For me, it is not clear that it is necessary to push(+save ?) after claiming a surrender, couldn't it be automated ? Maybe I did something wrong ? - I'm not sure for this point, but I think the task_layers.json reading is a little bit bugged. I create my own one and for "MATERIAL" I change the "default_owner" from "Modeling" to "Shading" and change "auto_surrender" from "true" to "false". But When I create an asset the materials data are owned by the Modeling and not surrended.(the task_layers that I used is share in this message. To let you know, Yes I add the dir in the "Custom Task Layers" Preference. The Grooming Task Layer was perfectly add) - It seems that the Push&Pull system is unidirectional: from tasks to staged/active files. Shouldn’t it be bidirectional? (having the possibility to push to MOD/SHD/RIG/etc from the staged/active file and pull MOD/SHD/RIG from tasks file ?) - When a staged (or an active) file is create, a “Task Layer Config is invalid” label appears, because no “task_layers.json” is created in the folder. Is it relevant/usefull ? If it’s to “lock” the Push&Pull system in the staged/publish file, shouldn’t you disable “Asset Management” menu or recreate an ui without the top part (only keep, avanced, tools and ownership Inspector) ? ![image](/attachments/e4d5f555-6fec-4338-b206-06a0a749a951) Hope that's feedback will be relevant for you (And I’m sorry if they’re misuses from me)
Author
Member

Thanks for the feedback @Eqkoss here are my replies.

For me, it is not clear that it is necessary to push(+save ?) after claiming a surrender, couldn't it be automated ? Maybe I did something wrong ?

It could be automated, but we try to let the user do things explicitly we don't want to auto-save after each surrender claim in case the user needs to revert.

I'm not sure for this point, but I think the task_layers.json reading is a little bit bugged.
I create my own one and for "MATERIAL" I change the "default_owner" from "Modeling" to "Shading" and change "auto_surrender" from "true" to "false". But When I create an asset the materials data are owned by the Modeling and not surrended.(the task_layers that I used is share in this message. To let you know, Yes I add the dir in the "Custom Task Layers" Preference. The Grooming Task Layer was perfectly add)

I don't think there is a bug based on ur feedback. auto_surrender If it is set to false the data is not surrendered, which is the behaviour you have reported, so that is correct. Default Owner is only used if the default is part of your local task layers for that file, the software cannot assign the transfer data to Shading, unless Shading is a task layer that is local to your file. We could consider renaming default owner to prefferred owner as it is more accurate.

It seems that the Push&Pull system is unidirectional: from tasks to staged/active files. Shouldn’t it be bidirectional? (having the possibility to push to MOD/SHD/RIG/etc from the staged/active file and pull MOD/SHD/RIG from tasks file ?)

No, this is not the behaviour the users at blender studio want, each user needs to explicity push/pull from the active/staged file, otherwise things will change in their file without the user being informed.

When a staged (or an active) file is create, a “Task Layer Config is invalid” label appears, because no “task_layers.json” is created in the folder. Is it relevant/usefull ? If it’s to “lock” the Push&Pull system in the staged/publish file, shouldn’t you disable “Asset Management” menu or recreate an ui without the top part (only keep, avanced, tools and ownership Inspector) ?

As discussed above the add-on is still in early development staged, the invalid message appears on published files when it shouldn't that is an issue we will address in the future, but it is non-critical.

You cannot push/pull from the published file, only the task layer files.

Please leave your additional feedback on the channel https://blender.chat/channel/blender-studio-pipeline because this issue is focused on the needs of the Blender Studio users.

Thanks for the feedback @Eqkoss here are my replies. > For me, it is not clear that it is necessary to push(+save ?) after claiming a surrender, couldn't it be automated ? Maybe I did something wrong ? It could be automated, but we try to let the user do things explicitly we don't want to auto-save after each surrender claim in case the user needs to revert. > I'm not sure for this point, but I think the task_layers.json reading is a little bit bugged. > I create my own one and for "MATERIAL" I change the "default_owner" from "Modeling" to "Shading" and change "auto_surrender" from "true" to "false". But When I create an asset the materials data are owned by the Modeling and not surrended.(the task_layers that I used is share in this message. To let you know, Yes I add the dir in the "Custom Task Layers" Preference. The Grooming Task Layer was perfectly add) I don't think there is a bug based on ur feedback. `auto_surrender` If it is set to `false` the data is not surrendered, which is the behaviour you have reported, so that is correct. `Default Owner` is only used if the default is part of your local task layers for that file, the software cannot assign the transfer data to Shading, unless Shading is a task layer that is local to your file. We could consider renaming `default owner` to `prefferred owner` as it is more accurate. > It seems that the Push&Pull system is unidirectional: from tasks to staged/active files. Shouldn’t it be bidirectional? (having the possibility to push to MOD/SHD/RIG/etc from the staged/active file and pull MOD/SHD/RIG from tasks file ?) No, this is not the behaviour the users at blender studio want, each user needs to explicity push/pull from the active/staged file, otherwise things will change in their file without the user being informed. > When a staged (or an active) file is create, a “Task Layer Config is invalid” label appears, because no “task_layers.json” is created in the folder. Is it relevant/usefull ? If it’s to “lock” the Push&Pull system in the staged/publish file, shouldn’t you disable “Asset Management” menu or recreate an ui without the top part (only keep, avanced, tools and ownership Inspector) ? As discussed above the add-on is still in early development staged, the invalid message appears on published files when it shouldn't that is an issue we will address in the future, but it is non-critical. You cannot push/pull from the published file, only the task layer files. Please leave your additional feedback on the channel https://blender.chat/channel/blender-studio-pipeline because this issue is focused on the needs of the Blender Studio users.

@TinyNick thanks a lot for your replies.
That seams pretty clear now with your explaination.

Please leave your additional feedback on the channel https://blender.chat/channel/blender-studio-pipeline because this issue is focused on the needs of the Blender Studio users.

Sorry for that ! I Will do it

@TinyNick thanks a lot for your replies. That seams pretty clear now with your explaination. > > Please leave your additional feedback on the channel https://blender.chat/channel/blender-studio-pipeline because this issue is focused on the needs of the Blender Studio users. Sorry for that ! I Will do it
Author
Member

Per a discussion I had with @SimonThommes I have added/updated the following Feature Requests to the main TODO

  • Un-Surrender Option (to cancel a surrender before Push)
  • Use Default Ownership when Claiming Surrendered

We have also added the following Bugs

  • Refactor Transfer Data Merge Code to avoid Claimed Surrendered Conflict
    • If data as been surrendered and claimed, any out of date file will report the conflict between these two datas as a merge conflict, even if neither is part of the local task layer.
  • UI Error if Collection is not set & Collection Selector is unset on an Asset
  • Geo-Nodes don't get their inputs initialized Correctly
Per a discussion I had with @SimonThommes I have added/updated the following Feature Requests to the main TODO > - [ ] Un-Surrender Option (to cancel a surrender before Push) > - [ ] Use Default Ownership when Claiming Surrendered We have also added the following Bugs > - [ ] Refactor Transfer Data Merge Code to avoid Claimed Surrendered Conflict > - _If data as been surrendered and claimed, any out of date file will report the conflict between these two datas as a merge conflict, even if neither is part of the local task layer._ > - [ ] UI Error if Collection is not set & Collection Selector is unset on an Asset > - [ ] Geo-Nodes don't get their inputs initialized Correctly
Member

There are some additional things that need to be owned in the shading process:

  • UV seams
  • active UV layer
  • active color attribute

Currently these are just owned by the object owner together with the object data. But we need to be able to own them on a separate task layer.

One reason is the workflow:
The active data context is used for editing and having this change when you pull from the publish is a major risk for losing data by working on the wrong data layer.
This could in the future also be solved by always keeping the active layer selection from the local workfile.

The second (main) reason is that the active layer matters for viewport display. For example the active UV layer is used to map the active texture when this mode is set in the viewport display settings.

There are some additional things that need to be owned in the shading process: - UV seams - active UV layer - active color attribute Currently these are just owned by the object owner together with the object data. But we need to be able to own them on a separate task layer. One reason is the workflow: The active data context is used for editing and having this change when you pull from the publish is a major risk for losing data by working on the wrong data layer. This could in the future also be solved by always keeping the active layer selection from the local workfile. The second (main) reason is that the active layer matters for viewport display. For example the active UV layer is used to map the active texture when this mode is set in the viewport display settings.
Member

I found another bug:

When changing the ownership of a modifier by surrendering and claiming, the modifier moves to the end of the stack.
I'm guessing it's being recreated but then not moved to the right spot.

I found another bug: When changing the ownership of a modifier by surrendering and claiming, the modifier moves to the end of the stack. I'm guessing it's being recreated but then not moved to the right spot.
Member

And something that might be a bigger problem:
When surrendered data is claimed and then deleted, there is nothing in place to enforce that deletion, so it is effectively still owned by the task layer that surrendered it.

Example:
From rigging

  • Surrender modifier

From modeling

  • Claim modifier
  • Delete modifier
  • Pull and it comes back

This can be worked around by just pushing before deleting the modifier to establish the change in ownership. But the problem comes if this change in ownership is not pushed/pulled all the way back into the task layer that initially surrendered the ownership. Unless that happens it will always come back in a surrendered state.

And something that might be a bigger problem: When surrendered data is claimed and then deleted, there is nothing in place to enforce that deletion, so it is effectively still owned by the task layer that surrendered it. Example: From rigging - Surrender modifier From modeling - Claim modifier - Delete modifier - Pull and it comes back This can be worked around by just pushing before deleting the modifier to establish the change in ownership. But the problem comes if this change in ownership is not pushed/pulled all the way back into the task layer that initially surrendered the ownership. Unless that happens it will always come back in a surrendered state.
Author
Member

There are some additional things that need to be owned in the shading process:

  • UV seams
  • active UV layer
  • active color attribute

Currently these are just owned by the object owner together with the object data. But we need to be able to own them on a separate task layer.

One reason is the workflow:
The active data context is used for editing and having this change when you pull from the publish is a major risk for losing data by working on the wrong data layer.
This could in the future also be solved by always keeping the active layer selection from the local workfile.

The second (main) reason is that the active layer matters for viewport display. For example the active UV layer is used to map the active texture when this mode is set in the viewport display settings.

OK as we discussed on the phone I have prioritized this task. Active UV and Active Color Attribute have been completed. I have started the UVseam transfer as a PR,

I found another bug:

When changing the ownership of a modifier by surrendering and claiming, the modifier moves to the end of the stack.
I'm guessing it's being recreated but then not moved to the right spot.

I have added a task to the bug's section of the TODO

And something that might be a bigger problem:
When surrendered data is claimed and then deleted, there is nothing in place to enforce that deletion, so it is effectively still owned by the task layer that surrendered it.

Example:
From rigging

  • Surrender modifier

From modeling

  • Claim modifier
  • Delete modifier
  • Pull and it comes back

This can be worked around by just pushing before deleting the modifier to establish the change in ownership. But the problem comes if this change in ownership is not pushed/pulled all the way back into the task layer that initially surrendered the ownership. Unless that happens it will always come back in a surrendered state.

This will be a hard one to resolve will have to think on it, but per our discussion this is lower priority. I added it to the low priority TODOs

> There are some additional things that need to be owned in the shading process: > - UV seams > - active UV layer > - active color attribute > > Currently these are just owned by the object owner together with the object data. But we need to be able to own them on a separate task layer. > > One reason is the workflow: > The active data context is used for editing and having this change when you pull from the publish is a major risk for losing data by working on the wrong data layer. > This could in the future also be solved by always keeping the active layer selection from the local workfile. > > The second (main) reason is that the active layer matters for viewport display. For example the active UV layer is used to map the active texture when this mode is set in the viewport display settings. OK as we discussed on the phone I have prioritized this task. [Active UV](https://projects.blender.org/studio/blender-studio-pipeline/commit/22750ec911e06d012ed2c29a5ce1dd9addb6ea60) and [Active Color Attribute](https://projects.blender.org/studio/blender-studio-pipeline/commit/18111e9235bce4cb3dc3c677fa43d30b670e595c) have been completed. I have started the [UVseam transfer as a PR](https://projects.blender.org/studio/blender-studio-pipeline/pulls/197), > I found another bug: > > When changing the ownership of a modifier by surrendering and claiming, the modifier moves to the end of the stack. > I'm guessing it's being recreated but then not moved to the right spot. I have added a task to the [bug's section of the TODO](https://projects.blender.org/studio/blender-studio-pipeline/issues/170#:~:text=Modifiers%20Move%20in%20Stack%20when%20Owner%20Changes) > And something that might be a bigger problem: > When surrendered data is claimed and then deleted, there is nothing in place to enforce that deletion, so it is effectively still owned by the task layer that surrendered it. > > Example: > From rigging > - Surrender modifier > > From modeling > - Claim modifier > - Delete modifier > - Pull and it comes back > > This can be worked around by just pushing before deleting the modifier to establish the change in ownership. But the problem comes if this change in ownership is not pushed/pulled all the way back into the task layer that initially surrendered the ownership. Unless that happens it will always come back in a surrendered state. This will be a hard one to resolve will have to think on it, but per our discussion this is lower priority. I added it to the [low priority TODOs](https://projects.blender.org/studio/blender-studio-pipeline/issues/170#:~:text=When%20surrendered%20data%20is%20claimed%20and%20then%20deleted%2C%20there%20is%20nothing%20in%20place%20to%20enforce%20that%20deletion%20source)
Member

Issue with Saving on Push

Right now, when I disable saving the file in the push operator it still saves.

Nick from the chat:

The push operator has to save to Save because we open the published file while that operator is running and append the content of your work file into the publish.

Then why is there even an option in the operator to save or not? If it's really absolutely necessary to always save before a push that option shouldn't even exist. Right now it just saves, even though the user says not to.
But additionally, I think it would be nice if only optionally saving would actually be possible. I don't see an issue with alternatively saving to a temp file next to the working file and appending from there. But this is lower priority.

### Issue with Saving on Push Right now, when I disable saving the file in the push operator it still saves. Nick from the chat: > The push operator has to save to Save because we open the published file while that operator is running and append the content of your work file into the publish. Then why is there even an option in the operator to save or not? If it's really absolutely necessary to always save before a push that option shouldn't even exist. Right now it just saves, even though the user says not to. But additionally, I think it would be nice if only optionally saving would actually be possible. I don't see an issue with alternatively saving to a temp file next to the working file and appending from there. But this is lower priority.
Author
Member

Issue with Saving on Push

Right now, when I disable saving the file in the push operator it still saves.

Nick from the chat:

The push operator has to save to Save because we open the published file while that operator is running and append the content of your work file into the publish.

Then why is there even an option in the operator to save or not? If it's really absolutely necessary to always save before a push that option shouldn't even exist. Right now it just saves, even though the user says not to.
But additionally, I think it would be nice if only optionally saving would actually be possible. I don't see an issue with alternatively saving to a temp file next to the working file and appending from there. But this is lower priority.

Would appreciate your feedback on this as well @Mets. In the past I have suggested we rename the operator PUSH to SYNC because Sync implies both Pulling and Pushing. And in theory yes we can remove the save bool. Overall considering the long list of existing TODOs I agree this isn't high priority. I also don't like changing some many things about this add-on right before production starts.

Also I have added UV Seams to Material Transfer

> ### Issue with Saving on Push > > Right now, when I disable saving the file in the push operator it still saves. > > Nick from the chat: > > The push operator has to save to Save because we open the published file while that operator is running and append the content of your work file into the publish. > > Then why is there even an option in the operator to save or not? If it's really absolutely necessary to always save before a push that option shouldn't even exist. Right now it just saves, even though the user says not to. > But additionally, I think it would be nice if only optionally saving would actually be possible. I don't see an issue with alternatively saving to a temp file next to the working file and appending from there. But this is lower priority. Would appreciate your feedback on this as well @Mets. In the past I have suggested we rename the operator PUSH to SYNC because Sync implies both Pulling and Pushing. And in theory yes we can remove the save bool. Overall considering the long list of existing TODOs I agree this isn't high priority. I also don't like changing some many things about this add-on right before production starts. Also I have added UV Seams to Material Transfer
Member

Enforcing Pulling on Push

We had multiple discussions about how we enforce pulling before a push in a way that it's reliable but also not restricting to the workflow. The way it's done currently is, in my opinion, not sufficient to allow for a smooth workflow.

First off:
Regardless of any potential functionality changes. The label of the push button has to change if it works the way does now. Effectively, right now it always does a pull before a push. Calling this button 'Push' while there is a separate button for pulling next to it is misleading at best.

On top of that:
In my opinion there should be an accessible option to push without pulling. It is part of my regular workflow to push or pull very intentionally, since I'm aware whenever the publish can even have changed between two pushes.

Enforcing redundant pulls at all times with the cost of doubling the runtime of the entire process is undermining a smooth workflow and taking away the judgement of the user to make the right decision.

Right now the ability to push without pulling exists, but it is hidden behind the advanced toggle in the settings and needs to be toggled every time the push operator is run. This is not a sufficient solution imo.

I'd propose to rename the current Push button to Pull and Push and add an additional button that only pushes. Enforcing to use this button can still be done in a 'soft' way rather than the 'hard' way it currently works, by choices in the UI. For example ordering that button at the top, making it bigger and such.

### Enforcing Pulling on Push We had multiple discussions about how we enforce pulling before a push in a way that it's reliable but also not restricting to the workflow. The way it's done currently is, in my opinion, not sufficient to allow for a smooth workflow. First off: Regardless of any potential functionality changes. The label of the push button has to change if it works the way does now. Effectively, right now it always does a pull before a push. Calling this button 'Push' while there is a separate button for pulling next to it is misleading at best. On top of that: In my opinion there should be an accessible option to push without pulling. It is part of my regular workflow to push or pull very intentionally, since I'm aware whenever the publish can even have changed between two pushes. Enforcing redundant pulls at all times with the cost of doubling the runtime of the entire process is undermining a smooth workflow and taking away the judgement of the user to make the right decision. Right now the ability to push without pulling exists, but it is hidden behind the advanced toggle in the settings and needs to be toggled every time the push operator is run. This is not a sufficient solution imo. I'd propose to rename the current `Push` button to `Pull and Push` and add an additional button that only pushes. Enforcing to use this button can still be done in a 'soft' way rather than the 'hard' way it currently works, by choices in the UI. For example ordering that button at the top, making it bigger and such.
Author
Member

Enforcing Pulling on Push

I have created a Pull Request to satisfy your request, studio/blender-studio-pipeline#199 we can merge it after Met has had a chance to provide feedback and he agrees.

I have updated the TODOs with this feedback, consolidating your last two comments into one task.

For saving I will remove the bool, and updated the tooltip to explain saving is mandatory. We cannot allow users to Push from a temp save, because this can easily cause out of sync issues.

> ### Enforcing Pulling on Push I have created a Pull Request to satisfy your request, https://projects.blender.org/studio/blender-studio-pipeline/pulls/199 we can merge it after Met has had a chance to provide feedback and he agrees. I have updated the TODOs with this feedback, consolidating your last two comments into one [task](https://projects.blender.org/studio/blender-studio-pipeline/issues/170#:~:text=Issue%20with%20Saving%20on%20Push%20source%20%26%20Enforcing%20Pulling%20on%20Push%20source). For saving I will remove the bool, and updated the tooltip to explain saving is mandatory. We cannot allow users to Push from a temp save, because this can easily cause out of sync issues.
Member

@TinyNick

I also don't like changing some many things about this add-on right before production starts.

The problem is that most of the UX topics can only come up now, that we're actively using the addon. I briefly talked to Francesco and he confirmed that fixing these things, when they are necessary is fine to prioritize. When that interferes with your schedule regarding other things on your plate, make sure to let him know. But some things we'll just have to tackle now, so that we can create the assets.

I'll try to be more clear about what parts are imo necessary for a smooth asset creation process and what can wait for later.

If something turns out to require a major change in the merge process, of course that's a different topic and we should find a solution that is not disruptive.

@TinyNick > I also don't like changing some many things about this add-on right before production starts. The problem is that most of the UX topics can only come up now, that we're actively using the addon. I briefly talked to Francesco and he confirmed that fixing these things, when they are necessary is fine to prioritize. When that interferes with your schedule regarding other things on your plate, make sure to let him know. But some things we'll just have to tackle now, so that we can create the assets. I'll try to be more clear about what parts are imo necessary for a smooth asset creation process and what can wait for later. If something turns out to require a major change in the merge process, of course that's a different topic and we should find a solution that is not disruptive.
Sign in to join this conversation.
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#170
No description provided.