Add a basic support to BKE library linking code to generate liboverrides. #104746

Merged
Bastien Montagne merged 1 commits from mont29/blender:F-link-override-basics into main 2023-03-02 17:22:54 +01:00

This commit adds the ability to generate liboverrides of linked data at
the BKE_blendfile_link_append BKE level, and through the Python API
(the BPY_library_load context manager, aka bpy.data.libraries.load).

The python API was updated essentially to allow easy testing of the new
code. This commit also adds tests for the new 'override' behavior, and
for existing basic link one.

Current code only generates 'basic' overrides, without any handing of
hierarchies or dependencies, as for brush assets only the Brush ID needs
to be overridden.

That new feature does not aim at being exposed to user through the
link/append operations in its current state, as it is way too simplistic.

This change is a requirement for the Brush Asset project (#101908).

This commit adds the ability to generate liboverrides of linked data at the `BKE_blendfile_link_append` BKE level, and through the Python API (the `BPY_library_load` context manager, aka `bpy.data.libraries.load`). The python API was updated essentially to allow easy testing of the new code. This commit also adds tests for the new 'override' behavior, and for existing basic link one. Current code only generates 'basic' overrides, without any handing of hierarchies or dependencies, as for brush assets only the Brush ID needs to be overridden. That new feature does not aim at being exposed to user through the link/append operations in its current state, as it is way too simplistic. This change is a requirement for the Brush Asset project (#101908).
Brecht Van Lommel added this to the Core project 2023-02-15 09:53:52 +01:00
Bastien Montagne requested review from Campbell Barton 2023-02-15 10:18:32 +01:00
Bastien Montagne requested review from Julian Eisel 2023-02-15 10:18:32 +01:00
Author
Owner

@blender-bot build

@blender-bot build
Bastien Montagne force-pushed F-link-override-basics from a5bb77aff6 to 247be14fa0 2023-02-15 12:22:45 +01:00 Compare
Campbell Barton approved these changes 2023-03-01 08:21:05 +01:00
Campbell Barton left a comment
Owner

Minor requests inline, no need for an additional review iteration.

Minor requests inline, no need for an additional review iteration.
@ -1319,0 +1349,4 @@
const bool do_use_exisiting_liboverrides = (flags &
BKE_LIBLINK_OVERRIDE_USE_EXISTING_LIBOVERRIDES) != 0;
LinkNode *itemlink;

picky can be declared in the for loop.

*picky* can be declared in the for loop.
mont29 marked this conversation as resolved
@ -150,3 +154,3 @@
" :type relative: bool\n"
" :arg assets_only: If True, only list data-blocks marked as assets.\n"
" :type assets_only: bool\n");
" :type assets_only: bool\n"

The new arguments are missing from the declaration which currently reads.

.. method:: load(filepath, link=False, relative=False, assets_only=False)

Useful as it provides defaults.

The new arguments are missing from the declaration which currently reads. ``.. method:: load(filepath, link=False, relative=False, assets_only=False)`` Useful as it provides defaults.
mont29 marked this conversation as resolved
@ -151,2 +155,3 @@
" :arg assets_only: If True, only list data-blocks marked as assets.\n"
" :type assets_only: bool\n");
" :type assets_only: bool\n"
" :arg create_liboverrides: If True, liboverrides will be created for linked data \n"

picky no space before newline needed.

*picky* no space before newline needed.
mont29 marked this conversation as resolved
@ -152,1 +156,3 @@
" :type assets_only: bool\n");
" :type assets_only: bool\n"
" :arg create_liboverrides: If True, liboverrides will be created for linked data \n"
" (only valid when `link` is True).\n"

picky only 3 additional spaces of indentation is required.

*picky* only 3 additional spaces of indentation is required.
mont29 marked this conversation as resolved
@ -153,0 +157,4 @@
" :arg create_liboverrides: If True, liboverrides will be created for linked data \n"
" (only valid when `link` is True).\n"
" :type create_liboverrides: bool\n"
" :arg reuse_liboverrides: If True and `create_liboverride` is True, \n"

literal quoted values should need to be double-back-tick quoted.

literal quoted values should need to be double-back-tick quoted.
mont29 marked this conversation as resolved
@ -153,0 +160,4 @@
" :arg reuse_liboverrides: If True and `create_liboverride` is True, \n"
" search for existing liboverride first.\n"
" :type reuse_liboverrides: bool\n"
" :arg create_runtime: If True and `create_liboverride` is True, \n"

create_runtime reads ambiguously and doesn't obviously relate to library-overrides.

While it's a bit of a mouthful, Prefer create_liboverrides_runtime.

`create_runtime` reads ambiguously and doesn't obviously relate to library-overrides. While it's a bit of a mouthful, Prefer `create_liboverrides_runtime`.
mont29 marked this conversation as resolved
@ -159,3 +172,1 @@
bool is_rel = false, is_link = false, use_assets_only = false;
static const char *_keywords[] = {"filepath", "link", "relative", "assets_only", NULL};
bool is_rel = false, is_link = false, use_assets_only = false, create_liboverrides = false,

Code in the body of this function accounts for link being false & create_liboverrides being true. e.g.

const bool create_liboverrides = !do_append && self->create_liboverrides;

This adds some uncertainty in how arguments should be used, implying that checking self->create_liboverrides without do_append might cause bugs or unexpected behavior.


Suggest to raise an exception when invalid arguments combination are passed in. This way there is no need to check combinations of options are valid in the functions body.

Alternatively, set create_liboverrides false when link is false, and warn that invalid combination of arguments were passed in.

Code in the body of this function accounts for `link` being false & `create_liboverrides` being true. e.g. `const bool create_liboverrides = !do_append && self->create_liboverrides;` This adds some uncertainty in how arguments should be used, implying that checking `self->create_liboverrides` without `do_append` might cause bugs or unexpected behavior. ---- Suggest to raise an exception when invalid arguments combination are passed in. This way there is no need to check combinations of options are valid in the functions body. Alternatively, set `create_liboverrides` false when `link` is false, and warn that invalid combination of arguments were passed in.
mont29 marked this conversation as resolved
@ -162,0 +179,4 @@
"create_liboverrides",
"reuse_liboverrides",
"create_runtime",
NULL};

picky NULL, }; to reduce right shift.

*picky* ```NULL, };``` to reduce right shift.
mont29 marked this conversation as resolved
@ -339,6 +377,8 @@ static bool bpy_lib_exit_lapp_context_items_cb(BlendfileLinkAppendContext *lapp_
const int py_list_index = POINTER_AS_INT(
BKE_blendfile_link_append_context_item_userdata_get(lapp_context, item));
ID *new_id = BKE_blendfile_link_append_context_item_newid_get(lapp_context, item);
ID *liboverride_id = BKE_blendfile_link_append_context_item_liboverrideid_get(lapp_context,

It's a bit odd to call this function even when create_liboverrides isn't set.

This reads more strightforward to me.

  ID *liboverride_id = data->py_library->create_liboverrides ?
                           BKE_blendfile_link_append_context_item_liboverrideid_get(lapp_context,
                                                                                    item) :
                           NULL;
  /* ... snip ... */
  if (liboverride_id != NULL) {
    /* ... assign py_item ... */
  }

  
It's a bit odd to call this function even when `create_liboverrides` isn't set. This reads more strightforward to me. ``` ID *liboverride_id = data->py_library->create_liboverrides ? BKE_blendfile_link_append_context_item_liboverrideid_get(lapp_context, item) : NULL; /* ... snip ... */ if (liboverride_id != NULL) { /* ... assign py_item ... */ } ```
mont29 marked this conversation as resolved
@ -552,0 +593,4 @@
assert len(bpy.data.collections) == 2 # The linked one and its local liboverride.
# Link + LibOverride, so linked data should have remained linked.
assert bpy.data.meshes[-1].library != None

picky is None / is not None (autopep8 even makes this change automatically).

*picky* `is None` / `is not None` (autopep8 even makes this change automatically).
Bastien Montagne force-pushed F-link-override-basics from 247be14fa0 to bb60ef93f6 2023-03-02 16:13:20 +01:00 Compare
Author
Owner

@blender-bot build

@blender-bot build
Bastien Montagne force-pushed F-link-override-basics from bb60ef93f6 to 4a22cb4557 2023-03-02 16:16:43 +01:00 Compare
Author
Owner

@blender-bot build

@blender-bot build
Bastien Montagne merged commit ff6ce25926 into main 2023-03-02 17:22:54 +01:00
Bastien Montagne deleted branch F-link-override-basics 2023-03-02 17:22:54 +01:00
Bastien Montagne removed this from the Core project 2023-07-03 12:49:26 +02:00
Sign in to join this conversation.
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
2 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: blender/blender#104746
No description provided.