Hrmmm.... This smells like some ID being added or changed without proper tagging, or without proper call to collections/layercollections update functions (guys like BKE_main_collection_sync_remap). Or maybe a missing object cache clearing (BKE_collection_object_cache_free)?

Indeed TIP_ is mandatory here. Besides picky note about UI messages style, looks fine.

@Sergey Sharybin (sergey) in that case I would say yes.

In D10907#284681, @Sybren A. Stüvel (sybren) wrote:

What's the reason to not just fix the depsgraph in those places where it created or removed some of those evaluated dependencies? What's against taking the appropriate action at that time, instead of always performing this post-build check?

LGTM, don't see any reason no to check that here indeed.

AFAIK Z-up is the convention when there is no way to get a proper normal from vco...

LGTM from quick look. :)

Am not sure I understand that 'fix'? How having a NULL normal is ever a good thing? I would rather fix the code using vco to detect zero case and generate that dummy 'Z-up' normal then?

Thanks (but please avoid assigning patches to people yourself in the future).

In D10983#282173, @Brecht Van Lommel (brecht) wrote:

Text datablocks have always been handled similar to scenes as far as I know. It's not clear to me why that is being changed to fake user now as a fix, instead of adding it to the list of exceptions.

The new fake user button in the text editor is not helpful to most end users, it's mostly a button that helps you lose data by accident. If the datablock selector is redesigned and this moves into a menu it's less of a risk. I guess for text datablocks autogenerated by scripts there may be a use case.

In D10983#282174, @Brecht Van Lommel (brecht) wrote:

Also, we should stop adding things like this in new code:

if (ELEM(GS(id->name), ID_WM, ID_WS, ID_SCE, ID_SCR, ID_LI)) {

That's what IDTypeInfo is for.

Patch looks good to me now...

Well... First of all, think this should be tagged to Dependency Graph more than Modifiers really... Also please avoid assigning reviewers directly.

Once again I think we should rather seriously spend time on T61209: Always write unused IDs on save, this would also solve that case imho.

Looks mostly fine, mainly missing a sub-version bump.

@Demeter Dzadik (Mets) would keep both for now, while related they are not entirely the same. We can re-iterate on designs and such once we actually start working on those.

Also related to T61209: Always write unused IDs on save

For the records, proposal LGTM... Personnaly I would be very conservative about what module we switch to type hinting though.

In general proposal LGTM.

Minor updates.

To be clear, what I call overhead here is:

• Adding extra function to API only to deal with const/non-const cases (this remains C code for now, this is fairly annoying imho).
• Adding hard-to-decipher things like * const *;

Minor updates from review.

@Sybren A. Stüvel (sybren) please do not add whole projects as reviewers (tagging is fine of course). This only adds lots of noise for many people.

Committed as listed above.

Thanks for the report, but this is absolutely not a bug, everything is working as expected.

NodeTrees are definitely not supported currently by overrides, and are indeed a known TODO, no bug here.

@Guillaume Pinchon (Tableuraz) please do not quote spammers, now I also have to nuke your comments ;)

Updated from review, thanks.

Scratch that, while it does fix the issue, it is not a proper way nor is it addressing the root of the problem.

@Sergey Sharybin (sergey) would not mind getting your advice on that, even though i don't think this is enough/fully fixing all possible issues there?