Assets: add function to copy asset data to another ID #108547

Sybren A. Stüvel · 2023-06-02T16:08:26+02:00

Sybren A. Stüvel commented

2023-06-02 16:08:26 +02:00

Add a function some_id.asset_data.copy_to_id(other_id) to copy the asset
data from one data-block to another. This is intended to be used in the
pose library, when updating a pose by simply creating a new asset and
having that replace the old one.

This is intentionally taking a copy, even though the above use case could
have sufficed with a higher-level 'move' function. By exposing this as a
copy, it can be used in a wider range of situations, from whatever Python
code wants to use it. This could include copying the asset data from the
active asset to all the other selected ones.

Any pre-existing asset data is freed before the copy is assigned. The
target ID MUST be marked as asset for this function to work. Clearing
the asset status by assigning None is not allowed. This will keep the
API clear, and ensures that marking & clearing is done via their respective
functions.

In the attached blend file, just press the 'Play' button in the text editor to see the code in effect. And then see the intentional error, comment out the line, see another intentional error, etc. This is the script in that blend file:

import bpy

D = bpy.data
src = D.objects['Suzanne']
dst = D.objects['Cube']

# This will fail, as assigning None is not allowed.
# Comment out to move forward with the test:
src.asset_data = None

# This will fail. Comment out to move forward with the test:
dst.asset_data = src.asset_data

# Destination datablock should already be an asset:
dst.asset_mark()
dst.asset_data = src.asset_data

# Also copy the preview image, which is not part of the asset data but nicer to see in the asset browser:
dst_preview = dst.preview_ensure()
dst_preview.image_size = src.preview.image_size
dst_preview.image_pixels = src.preview.image_pixels

Add a function `some_id.asset_data.copy_to_id(other_id)` to copy the asset data from one data-block to another. This is intended to be used in the pose library, when updating a pose by simply creating a new asset and having that replace the old one. This is intentionally taking a copy, even though the above use case could have sufficed with a higher-level 'move' function. By exposing this as a copy, it can be used in a wider range of situations, from whatever Python code wants to use it. This could include copying the asset data from the active asset to all the other selected ones. Any pre-existing asset data is freed before the copy is assigned. The target ID MUST be marked as asset for this function to work. Clearing the asset status by assigning `None` is not allowed. This will keep the API clear, and ensures that marking & clearing is done via their respective functions. In the attached blend file, just press the 'Play' button in the text editor to see the code in effect. And then see the intentional error, comment out the line, see another intentional error, etc. This is the script in that blend file: ```python import bpy D = bpy.data src = D.objects['Suzanne'] dst = D.objects['Cube'] # This will fail, as assigning None is not allowed. # Comment out to move forward with the test: src.asset_data = None # This will fail. Comment out to move forward with the test: dst.asset_data = src.asset_data # Destination datablock should already be an asset: dst.asset_mark() dst.asset_data = src.asset_data # Also copy the preview image, which is not part of the asset data but nicer to see in the asset browser: dst_preview = dst.preview_ensure() dst_preview.image_size = src.preview.image_size dst_preview.image_pixels = src.preview.image_pixels ```

108547-copy-asset-data-demo.blend

114 KiB

❤️ 1

Sybren A. Stüvel added the

Module

Pipeline, Assets & IO

label 2023-06-02 16:08:26 +02:00

Sybren A. Stüvel added 1 commit 2023-06-02 16:08:36 +02:00

2cd1343af0 Assets: add function to copy asset data to another ID

Add a function `some_id.asset_data.copy_to_id(other_id)` to copy the asset
data from one data-block to another. This is intended to be used in the
pose library, when updating a pose by simply creating a new asset and
having that replace the old one.

This is intentionally taking a copy, even though the above use case could
have sufficed with a higher-level 'move' function. By exposing this as a
copy, it can be used in a wider range of situations, from whatever Python
code wants to use it. This could include copying the asset data from the
active asset to all the other selected ones.

Any pre-existing asset data is freed before the copy is assigned. The
target ID doesn't have to be marked as asset yet for this function to work.
Note that using this function will make the destination ID an asset,
without going through the regular operator.

Sybren A. Stüvel requested review from Julian Eisel 2023-06-02 16:08:46 +02:00

Julian Eisel reviewed 2023-06-02 16:29:11 +02:00

Julian Eisel left a comment

I wonder if some_id.asset_data.copy_to_id(other_id) is "the right way" around for this (not strongly against it though). I'd expect that I call a function on the ID I want to modify, not the one I want to get the value from. Esp given that this is also how I mark/clear an asset.

We could also make ID.asset_data editable, and do a deep copy in the setter callback. I guess that would be the expected behavior when doing one_id.asset_data = other_id.asset_data?

I wonder if `some_id.asset_data.copy_to_id(other_id)` is "the right way" around for this (not strongly against it though). I'd expect that I call a function on the ID I want to modify, not the one I want to get the value from. Esp given that this is also how I mark/clear an asset. We could also make `ID.asset_data` editable, and do a deep copy in the setter callback. I guess that would be the expected behavior when doing `one_id.asset_data = other_id.asset_data`?

source/blender/makesrna/intern/rna_asset.c Outdated

						
				@ -340,1 +343,4 @@

				void rna_AssetMetaData_copy_to_id(AssetMetaData *self, ReportList *reports, ID *destination)

				{

				  if (destination == NULL) {

Julian Eisel commented

2023-06-02 16:19:08 +02:00

For an RNA callback this has a bit too much knowledge/logic for my taste, I'd prefer having this in some ED_asset_xxx() function. Similar to how we have ED_asset_mark_id()/ED_asset_clear_id().

For an RNA callback this has a bit too much knowledge/logic for my taste, I'd prefer having this in some `ED_asset_xxx()` function. Similar to how we have `ED_asset_mark_id()`/`ED_asset_clear_id()`.

Sybren A. Stüvel commented

2023-06-02 16:58:48 +02:00

"the right way" around

The advantage of this approach is that source.asset_data.copy_to_id(dest) works regardless of whether dest is an asset or not, and it can only work when source already has asset data. This seemed like good preconditions to me. The alternative dest.asset_data.copy_from(source.asset_data) implies that dest would have to be marked as asset first, which might also involve rendering the preview image. That implication falls away when using direct assignment, dest.asset_data = source.asset_data. I have no strong feelings about this either, except that it would be a bit more work ;-)

For an RNA callback this has a bit too much knowledge/logic for my taste

Fair, I'll move the code to an ED_xxx function.

> "the right way" around The advantage of this approach is that `source.asset_data.copy_to_id(dest)` works regardless of whether `dest` is an asset or not, and it can only work when `source` already has asset data. This seemed like good preconditions to me. The alternative `dest.asset_data.copy_from(source.asset_data)` implies that `dest` would have to be marked as asset first, which might also involve rendering the preview image. That implication falls away when using direct assignment, `dest.asset_data = source.asset_data`. I have no strong feelings about this either, except that it would be a bit more work ;-) > For an RNA callback this has a bit too much knowledge/logic for my taste Fair, I'll move the code to an `ED_xxx` function.

Julian Eisel commented

2023-06-02 17:47:39 +02:00

except that it would be a bit more work ;-)

Is it though? I think it's pretty much the same thing, just in a RNA setter for ID.asset_data.

> except that it would be a bit more work ;-) Is it though? I think it's pretty much the same thing, just in a RNA setter for `ID.asset_data`.

Sybren A. Stüvel added 1 commit 2023-06-05 14:19:21 +02:00

e7a21c6eda Review feedback

- Move functionality from RNA to ED
- Change from `src.asset_data.copy_to_id(dst)` function to direct
  assignment `dst.asset_data = src.asset_data`

Sybren A. Stüvel added 2 commits 2023-06-05 14:23:33 +02:00

9e1bcc75e2 Allow 'clear asset' by assigning `None`

`id.asset_data = None` is now a way to clear the 'is asset' status. This
makes the assignment symmetrical, as `id.asset_data = some_other_data` can
also be used to actually mark the ID as asset.

3bceeda146 Slightly improve error message when assigning to non-assettable ID type

Sybren A. Stüvel added 1 commit 2023-06-05 14:24:25 +02:00

a65f2437f8 Include comment to explain why you can clear the asset status by assigning None

Sybren A. Stüvel referenced this issue from a commit

2023-06-05 18:11:37 +02:00

Animation: pose library, add 'Replace Pose' operator

Sybren A. Stüvel referenced this pull request from blender/blender-addons

2023-06-05 18:11:50 +02:00

WIP: Animation: operators to update the pose library #104673

Julian Eisel commented

2023-06-06 15:24:03 +02:00

Thinking some more about this, what I'm unsure about is if this way of "marking" or "clearing" an asset is future proof. What if this process involves more than just setting/getting the metadata? E.g. I could imagine we'd want to call some callback or app handler when marking/clearing an asset.

So we could say setting it to None is not valid, or setting it on an ID that is not marked as asset.

Thinking some more about this, what I'm unsure about is if this way of "marking" or "clearing" an asset is future proof. What if this process involves more than just setting/getting the metadata? E.g. I could imagine we'd want to call some callback or app handler when marking/clearing an asset. So we could say setting it to `None` is not valid, or setting it on an ID that is not marked as asset.

Sybren A. Stüvel added 2 commits 2023-06-06 17:47:52 +02:00

84b42045c3 Merge remote-tracking branch 'origin/main' into pr/asset-data-copy-to-id

df0ec2b1c9 Disallow mark/clear asset by assigning to `asset_data`

Sybren A. Stüvel commented

2023-06-06 17:50:29 +02:00

Agreed, I've updated the code, PR description, and attached blend file for this.

Sybren A. Stüvel referenced this pull request

2023-06-06 17:52:46 +02:00

WIP: pull request to test a pose library feature #108670

Julian Eisel approved these changes 2023-06-06 17:53:35 +02:00

Julian Eisel left a comment

Nice, I think this is the way to go.