Page MenuHome
Differential D13803 Diff 46919 source/blender/blenkernel/intern/blendfile_link_append.c

Changeset View

Standalone View

View Options

source/blender/blenkernel/intern/blendfile_link_append.c

Show First 20 Lines • Show All 675 Lines • ▼ Show 20 Lines for (itemlink = lapp_context->items.list; itemlink; itemlink = itemlink->next) {
ID *id = loose_data_instantiate_process_check(instantiate_context, item); ID *id = loose_data_instantiate_process_check(instantiate_context, item);
if (id == NULL || GS(id->name) != ID_GR) { if (id == NULL || GS(id->name) != ID_GR) {
continue; continue;
} }
Collection *collection = (Collection *)id; Collection *collection = (Collection *)id;
bool do_add_collection = (id->tag & LIB_TAG_DOIT) != 0; bool do_add_collection = (id->tag & LIB_TAG_DOIT) != 0;
/* When instantiated into view-layer, do not add collections if one of their parents is also /* When instantiated into view-layer, do not add collections if one of their parents is also
* instantiated. In case of empty-instantiation though, instantiation of all user-selected * instantiated. In case of empty-instantiation though, instantiation of all user-selected
* collections is the desired behavior. */ * collections is the desired behavior. */
if (!do_add_collection || if (!do_add_collection ||
(!do_instantiate_as_empty && (!do_instantiate_as_empty &&
loose_data_instantiate_collection_parents_check_recursive(collection))) { loose_data_instantiate_collection_parents_check_recursive(collection))) {
continue; continue;
} }
mont29Unsubmitted
Done
Inline Actions

Think this can be split in two, and then add a third condition, with appropriate comment about why. Something like that:

if (!do_add_collection) {
  continue;
}
/* When instantiated into view-layer, do not add collections if one of their parents is also instantiated. */
if (!do_instantiate_as_empty && loose_data_instantiate_collection_parents_check_recursive(collection)) {
  continue;
}
/* When instantiated as empties, do not add indirectly linked (i.e. non-user-selected) collections. */
if (do_instantiate_as_empty && (item->tag & LINK_APPEND_TAG_INDIRECT) != 0) {
  continue;
}
mont29: Think this can be split in two, and then add a third condition, with appropriate comment about…
SeverinAuthorUnsubmitted
Done
Inline Actions

Agree, this makes things more readable.

Severin: Agree, this makes things more readable.
loose_data_instantiate_ensure_active_collection(instantiate_context); loose_data_instantiate_ensure_active_collection(instantiate_context);
Collection *active_collection = instantiate_context->active_collection; Collection *active_collection = instantiate_context->active_collection;
if (do_instantiate_as_empty && (item->tag & LINK_APPEND_TAG_INDIRECT) != 0) {
/* In case user requested instantiation of collections as empties, do so for the one they /* In case user requested instantiation of collections as empties, do so for the one they
* explicitly selected (originally directly linked IDs) only. */ * explicitly selected (originally directly linked IDs) only. */
if (do_instantiate_as_empty && (item->tag & LINK_APPEND_TAG_INDIRECT) == 0) { }
else if (do_instantiate_as_empty) {
mont29Unsubmitted
Done
Inline Actions

This change is not making logic of the code easier to follow, rather the opposite. Suggest to move that 'skip' case above with the other ones. Then you can just do if (do_instantiate_as_empty) here.

mont29: This change is not making logic of the code easier to follow, rather the opposite. Suggest to…
/* BKE_object_add(...) messes with the selection. */ /* BKE_object_add(...) messes with the selection. */
Object *ob = BKE_object_add_only_object(bmain, OB_EMPTY, collection->id.name + 2); Object *ob = BKE_object_add_only_object(bmain, OB_EMPTY, collection->id.name + 2);
ob->type = OB_EMPTY; ob->type = OB_EMPTY;
ob->empty_drawsize = U.collection_instance_empty_size; ob->empty_drawsize = U.collection_instance_empty_size;
const bool set_selected = (lapp_context->params->flag & FILE_AUTOSELECT) != 0; const bool set_selected = (lapp_context->params->flag & FILE_AUTOSELECT) != 0;
/* TODO: why is it OK to make this active here but not in other situations? /* TODO: why is it OK to make this active here but not in other situations?
* See other callers of #object_base_instance_init */ * See other callers of #object_base_instance_init */
▲ Show 20 Lines • Show All 939 Lines • Show Last 20 Lines