Asset Browser: Disable metadata editing for external asset libraries
ClosedPublic
Actions

Authored by Julian Eisel (Severin) on Aug 4 2021, 3:57 PM.

Details

Reviewers

Sybren A. Stüvel (sybren)

Group Reviewers

Asset Browser

Maniphest Tasks

T82943: Disable meta-data editing for external repositories

Commits

rB222fd1abf09a: Asset Browser: Disable metadata editing for external asset libraries

Summary

Had to do some RNA trickery to disable the metadata properties at RNA level, not at UI script level.

No need for an in-depth review, just want the general idea to be checked.
Basically:

Local data-block assets set the data-block as owning ID for the asset metadata RNA pointer now (PointerRNA.owner_id).
That way we can use the owner ID to see where the metadata belongs to and decide if it's editable that way.
Additionally, some Python operators needed better polling so they show as grayed out, and don't just fail.
Added "disabled hints" to explain why buttons are disabled.

One important thing: Custom properties of the metadata can still be edited. The edits won't be saved however. Would be nice to disable that, but it's currently not supported on BPY/IDProperty/RNA level.

Addresses T82943.

Diff Detail

Repository

rB Blender

Branch

temp-asset-browser-disable-metadata (branched from master)

Build Status

Buildable 16218
Build 16218: arc lint + arc unit

Event Timeline

Julian Eisel (Severin) requested review of this revision.Aug 4 2021, 3:57 PM

Julian Eisel (Severin) created this revision.

Harbormaster completed remote builds in B16218: Diff 40314.Aug 4 2021, 3:57 PM

Julian Eisel (Severin) edited the summary of this revision. (Show Details)Aug 4 2021, 3:58 PM

Julian Eisel (Severin) added a task: T82943: Disable meta-data editing for external repositories.Aug 4 2021, 4:01 PM

Julian Eisel (Severin) added a project: Asset Browser.Aug 11 2021, 2:35 PM

Julian Eisel (Severin) added a reviewer: Asset Browser.

Julian Eisel (Severin) edited the summary of this revision. (Show Details)

Sybren A. Stüvel (sybren) added a subscriber: Sybren A. Stüvel (sybren).Aug 11 2021, 4:41 PM

Sybren A. Stüvel (sybren) added inline comments.

source/blender/makesrna/intern/rna_space.c
2610–2611	This comment is a bit hard to follow. How's this? Local IDs (`entry->id`) own their own metadata, which can be edited. Asset metadata from other blend files are owned by the file browser (`entry`), and are not editable.
2632	Why bother with two return statements if the same value is always returned, by design?

Address minor points from review

Harbormaster completed remote builds in B16367: Diff 40604.Aug 11 2021, 5:38 PM

Julian Eisel (Severin) added inline comments.Aug 11 2021, 5:38 PM

source/blender/makesrna/intern/rna_space.c
2610–2611	I think this can be made even clearer, so extended your proposal a bit.

Julian Eisel (Severin) marked an inline comment as done.Aug 11 2021, 5:39 PM

LGTM, just some minor notes. No need to revisit after adjusting.

release/scripts/startup/bl_operators/assets.py
39	The sentence is incomplete, "... only from the Current File asset library can" would be complete. However, this may become obsolete soon. How about "... only assets stored in the current file can"? That way it's independent of the selected asset library.
68	This should be `super().poll(context)` instead.
source/blender/makesrna/intern/rna_asset.c
47	I think it's better to reduce the cognitive complexity by inverting the condition and removing the nesting: { if (owner_id && asset_data && owner_id->asset_data == asset_data) { return true; } if (r_info) { *r_info = "Asset metadata from external asset libraries can't be edited, only from the Current " "File asset library"; } return false; }