Fix for T53415

Authored by Danrae Pray (spockTheGray) on Dec 1 2017, 4:28 AM.



Fix for T53415:

Updated collection_delete_exec() so we don't try to delete elements as we search the outliner tree anymore.

Now we search the whole tree first for the selected nodes that need to be deleted and delete them afterward.

Diff Detail

rB Blender
Dalai Felinto (dfelinto) requested changes to this revision.Dec 1 2017, 7:34 PM

Thanks for the patch. I added a few remarks in-line.

Some other things:

  • Use arc diff --origin/blender2.8 to submit your patch.
  • Even better then using BLI_genericNodeN() is to use `GSet. It may be a a bit overkill for this, but it's more efficient (and elegant)

Instead do:

BLI_addtail(&(data->collections_to_delete), BLI_genericNodeN(scene_collection));

And adapt the rest of the code accordingly.

- BKE_collection_remove(data->scene, (SceneCollection*)link->data);
+ BKE_collection_remove(&data->scene->id, (SceneCollection*)link->data);

Nothing wrong with the original patch, but the code in blender2.8 changed since you did this.


There is no need for this convoluted next = link->next. We do this only when we need to remove elements of the list while iterating over it.
In this case a regular loop over the listbase will work fine, since you clean the list only afterwards.

- struct CollectionDeleteData data = { .scene = scene,.soops = soops, { NULL, NULL } };
+ struct CollectionDeleteData data = {
+    .scene = scene,
+    .soops = soops,
+    .collections_to_delete = { NULL, NULL },

It's actually ok to keep them inline, but use named arguments to initialize the struct.

- // we first walk over and find the SceneCollections we actually want to delete, *ignoring duplicates*
+ /* We first walk over and find the SceneCollections we actually want to delete (ignoring duplicates). */
-  // now actually delete those bad dudes...
+  /* Effectively delete the collections. */
This revision now requires changes to proceed.Dec 1 2017, 7:34 PM

@Dalai Felinto (dfelinto) thanks for the feedback! Updated to use GSet API instead of ListBase (nice suggestion :). Also submitted patch using arc this time, but it created a new revision (D2940) presumably b.c. I didn't use arc for the original patch...

This revision was automatically updated to reflect the committed changes.

Hi, I committed your patch, many thanks for the fix.
Also, it's being awhile since I committed someone's else patch and I did it slightly wrong. As in, you won't see your name in the final commit. I'm embarrassed of that, really. I should've been more careful.