LibOverrides: Refactor how diffing of RNA collections is handled.
ClosedPublic
Actions

Authored by Bastien Montagne (mont29) on Nov 19 2021, 9:40 AM.

Details

Reviewers

Jeroen Bakker (jbakker)

Maniphest Tasks

T82160: LibOverrides - Refactor how diffing of RNA collections is handled, and extend diffing possibilities.

Commits

rB33c5e7bcd5e5: LibOverrides: Refactor how diffing of RNA collections is handled.

Summary

Original implementation was a quick prototype which should have never
landed as-is in master. It had very limiting constraints and did not
allow for any real further development.

This commit fixes the internal implementation to make more sensible,
maintainable and evolutive.

NOTE: this commit introduces another forward-incompatibility in the Blender file format: Files saved after this commit won't open properly in older versions of blender regarding local inserted constraints or modifiers into overrides of linked data.

Milestone I of T82160.

Diff Detail

Repository

rB Blender

Branch

T82160 (branched from master)

Build Status

Buildable 18782
Build 18782: arc lint + arc unit

Event Timeline

Bastien Montagne (mont29) requested review of this revision.Nov 19 2021, 9:40 AM

Bastien Montagne (mont29) created this revision.

Harbormaster completed remote builds in B18782: Diff 44961.Nov 19 2021, 9:40 AM

Fix merge conflict.

Harbormaster completed remote builds in B18783: Diff 44962.Nov 19 2021, 9:43 AM

The patch seems straight forward.

Would add a bit about the actual change in the description (use referenced names in stead of local names when looking up)
Consider having a function that first looks up by name and if not found lookup by index.
Would increase the readability and cross checking if the copied pattern doesn't have copy paste errors.

I don't consider mentioned comments as must haves.
Would rather have this one in to check for hidden artifacts.

source/blender/blenloader/intern/versioning_300.c
1434	Would mention the functions (`version_liboverride_rnacollections_*`) or add a region surrounding this comment with the functions.
1461	Perhaps we want to log these asserts even in release builds. CLG?