Fix #109021: Collection "Exclude" property lost when parent and grand-parent is disabled #109051

Dalai Felinto · 2023-06-16T12:03:19+02:00

Dalai Felinto commented

2023-06-16 12:03:19 +02:00

This patch started on D9849 by Dirk Wallenstein. I'm moving it here for further discussions.

I added a separate commit on top of that to handle the initialization of the flags. This is required for #109021 (although not needed for #84305).

I'm not sure:

If the original fix is the right approach.
If the initialization should be done once (in doversion), or always (in readfile).

This patch started on [D9849](https://archive.blender.org/developer/D9849) by [Dirk Wallenstein](https://projects.blender.org/d-egg). I'm moving it here for further discussions. I added a separate commit on top of that to handle the initialization of the flags. This is required for #109021 (although not needed for #84305). I'm not sure: * If the original fix is the right approach. * If the initialization should be done once (in doversion), or always (in readfile).

Dalai Felinto added 1 commit 2023-06-16 12:03:31 +02:00

2b6746893e Fix preserving user selected exclude-state of nested collections

Original patch: https://archive.blender.org/developer/D9849

Dalai Felinto added this to the Core project 2023-06-16 12:04:31 +02:00

Dalai Felinto requested review from Brecht Van Lommel 2023-06-16 12:07:11 +02:00

Dalai Felinto force-pushed fix-109021 from cf2e3286d3 to f7269b9d91

2023-06-16 12:12:50 +02:00

Compare

Dalai Felinto commented

2023-06-16 12:23:58 +02:00

By the way, I created this patch against 3.6. Though it may make more sense to have this against main, let it see if any unforeseen bug shows up, then we backport to 3.6 LTS. It depends on the confidence of the reviewer :)

I didn't really dig into the original patch by the way, I'm mostly acting as the messenger here (and tester).

By the way, I created this patch against 3.6. Though it may make more sense to have this against main, let it see if any unforeseen bug shows up, then we backport to 3.6 LTS. It depends on the confidence of the reviewer :) I didn't really dig into the original patch by the way, I'm mostly acting as the messenger here (and tester).

Bastien Montagne requested changes 2023-07-03 12:41:24 +02:00

Bastien Montagne left a comment

The commit message of the first commit is essentially empty.

There is a critical lack of documentation (comments) about layer_collection_flag_recursive_set expected behavior, and LAYER_COLLECTION_PREVIOUSLY_EXCLUDED could also use some comments about its exact meaning, with reference to layer_collection_flag_recursive_set.

The problem of this patch is that the new behavior is just as obscure and undocumented as the original code it tries to fix. So actually understanding if it's doing what it should do is... not trivial, to say the least.

Besides that, I would rather run the do_version code systematically (or at least add a debug check that when not run, data is in expected state). Not sure where exactly, maybe as part of BKE_collections_after_lib_link, or as a new similar call next to it?

The commit message of the first commit is essentially empty. There is a critical lack of documentation (comments) about `layer_collection_flag_recursive_set` expected behavior, and `LAYER_COLLECTION_PREVIOUSLY_EXCLUDED` could also use some comments about its exact meaning, with reference to `layer_collection_flag_recursive_set`. The problem of this patch is that the new behavior is just as obscure and undocumented as the original code it tries to fix. So actually understanding if it's doing what it should do is... not trivial, to say the least. ------------------- Besides that, I would rather run the `do_version` code systematically (or at least add a debug check that when not run, data is in expected state). Not sure where exactly, maybe as part of `BKE_collections_after_lib_link`, or as a new similar call next to it?

Dalai Felinto closed this pull request

2023-10-12 16:43:59 +02:00

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Fix #109021: Collection "Exclude" property lost when parent and grand-parent is disabled #109051

Pull request closed