Cleanup/Update comments in liboverride code.

source/blender/blenkernel/intern/lib_override.c

@@ -129,8 +129,8 @@ void BKE_lib_override_library_copy(ID *dst_id, const ID *src_id, const bool do_f
     BKE_lib_override_library_init(dst_id, NULL);
   }
 
-  /* Source is already overriding data, we copy it but reuse its reference for dest ID.
+  /* If source is already overriding data, we copy it but reuse its reference for dest ID.
-   * otherwise, source is only an override template, it then becomes reference of dest ID. */
+   * Otherwise, source is only an override template, it then becomes reference of dest ID. */
   dst_id->override_library->reference = src_id->override_library->reference ?
                                             src_id->override_library->reference :
                                             (ID *)src_id;
@@ -232,7 +232,7 @@ ID *BKE_lib_override_library_create_from_id(Main *bmain,
     FOREACH_MAIN_ID_BEGIN (bmain, other_id) {
       if ((other_id->tag & LIB_TAG_DOIT) != 0 && other_id->lib == NULL) {
         /* Note that using ID_REMAP_SKIP_INDIRECT_USAGE below is superfluous, as we only remap
-         * local IDs usages anyway... */
+         * local IDs usages anyway. */
         BKE_libblock_relink_ex(bmain,
                                other_id,
                                reference_id,
@@ -256,7 +256,7 @@ ID *BKE_lib_override_library_create_from_id(Main *bmain,
 /**
  * Create overridden local copies of all tagged data-blocks in given Main.
  *
- * \note Set id->newid of overridden libs with newly created overrides,
+ * \note Set `id->newid` of overridden libs with newly created overrides,
  * caller is responsible to clean those pointers before/after usage as needed.
  *
  * \note By default, it will only remap newly created local overriding data-blocks between
@@ -289,13 +289,14 @@ bool BKE_lib_override_library_create_from_tag(Main *bmain)
   for (todo_id_iter = todo_ids.first; todo_id_iter != NULL; todo_id_iter = todo_id_iter->next) {
     reference_id = todo_id_iter->data;
     if (reference_id->newid == NULL) {
-      /* If newid is already set, assume it has been handled by calling code.
+      /* If `newid` is already set, assume it has been handled by calling code.
        * Only current use case: re-using proxy ID when converting to liboverride. */
       if ((reference_id->newid = lib_override_library_create_from(bmain, reference_id)) == NULL) {
         success = false;
         break;
       }
-    } /* We also tag the new IDs so that in next step we can remap their pointers too. */
+    }
+    /* We also tag the new IDs so that in next step we can remap their pointers too. */
     reference_id->newid->tag |= LIB_TAG_DOIT;
 
     Key *reference_key;
@@ -333,7 +334,7 @@ bool BKE_lib_override_library_create_from_tag(Main *bmain)
       FOREACH_MAIN_ID_BEGIN (bmain, other_id) {
         if ((other_id->tag & LIB_TAG_DOIT) != 0 && other_id->lib == NULL) {
           /* Note that using ID_REMAP_SKIP_INDIRECT_USAGE below is superfluous, as we only remap
-           * local IDs usages anyway... */
+           * local IDs usages anyway. */
           BKE_libblock_relink_ex(bmain,
                                  other_id,
                                  reference_id,
@@ -381,7 +382,7 @@ static bool lib_override_hierarchy_recursive_tag(Main *bmain,
     id->tag |= tag;
   }
 
-  /* This way we won't process again that ID should we encounter it again through another
+  /* This way we won't process again that ID, should we encounter it again through another
    * relationship hierarchy.
    * Note that this does not free any memory from relations, so we can still use the entries.
    */
@@ -436,7 +437,7 @@ void BKE_lib_override_library_dependencies_tag(Main *bmain,
 
 /**
  * Tag all IDs in given \a bmain that are part of the same \a id_root liboverride ID group.
- * That is, all other liboverrides IDs (in)directly used by \a is_root one, sharing the same
+ * That is, all other liboverride IDs (in)directly used by \a is_root one, and sharing the same
  * library for their reference IDs.
  *
  * \param id_root: The root of the hierarchy of liboverride dependencies to be tagged.
@@ -481,8 +482,7 @@ static int lib_override_library_make_tag_ids_cb(LibraryIDLinkCallbackData *cb_da
   BLI_assert(id_owner->lib == library_root);
 
   if (id->tag & LIB_TAG_DOIT) {
-    /* Already processed, but maybe not with the same chain of dependency, so we need to check that
+    /* Already processed and tagged, nothing else to do here. */
-     * one nonetheless. */
     return IDWALK_RET_STOP_RECURSION;
   }
 
@@ -492,7 +492,7 @@ static int lib_override_library_make_tag_ids_cb(LibraryIDLinkCallbackData *cb_da
   }
 
   /* We tag all collections and objects for override. And we also tag all other data-blocks which
-   * would user one of those. */
+   * would use one of those. */
   if (ELEM(GS(id->name), ID_OB, ID_GR)) {
     id->tag |= LIB_TAG_DOIT;
   }
@@ -502,12 +502,12 @@ static int lib_override_library_make_tag_ids_cb(LibraryIDLinkCallbackData *cb_da
 
 static bool lib_override_library_create_do(Main *bmain, ID *id_root)
 {
-  /* Tag all collections and objects, as well as other IDs using them. */
   id_root->tag |= LIB_TAG_DOIT;
 
   BKE_main_relations_create(bmain, 0);
 
   if (ELEM(GS(id_root->name), ID_OB, ID_GR)) {
+    /* Tag all collections and objects. */
     BKE_library_foreach_ID_link(bmain,
                                 id_root,
                                 lib_override_library_make_tag_ids_cb,
@@ -515,7 +515,7 @@ static bool lib_override_library_create_do(Main *bmain, ID *id_root)
                                 IDWALK_READONLY | IDWALK_RECURSE);
 
     /* Then, we remove (untag) bone shape objects, you shall never want to override those
-     * (hopefully)... */
+     * (hopefully). */
     LISTBASE_FOREACH (Object *, ob, &bmain->objects) {
       if (ob->type == OB_ARMATURE && ob->pose != NULL && (ob->id.tag & LIB_TAG_DOIT)) {
         for (bPoseChannel *pchan = ob->pose->chanbase.first; pchan != NULL; pchan = pchan->next) {
@@ -527,6 +527,10 @@ static bool lib_override_library_create_do(Main *bmain, ID *id_root)
     }
   }
 
+  /* Now tag all non-object/collection IDs 'in-between' two tagged ones, as those are part of an
+   * override chain and therefore alos need to be overridden.
+   * One very common cases are e.g. drivers on geometry or materials of an overridden object, that
+   * are using another overridden object as parameter. */
   /* Note that this call will also free the main relations data we created above. */
   BKE_lib_override_library_dependencies_tag(bmain, id_root, LIB_TAG_DOIT, false);
 
@@ -636,16 +640,13 @@ static void lib_override_library_create_post_process(
  * \note Currently it only does special things if given \a id_root is an object of collection, more
  * specific behaviors may be added in the future for other ID types.
  *
- * \note It will overrides all IDs tagged with \a LIB_TAG_DOIT, and it does not clear that tag at
+ * \note It will override all IDs tagged with \a LIB_TAG_DOIT, and it does not clear that tag at
  * its beginning, so caller code can add extra data-blocks to be overridden as well.
  *
- * \note In the future that same function may be extended to support 'refresh' of overrides
- * (rebuilding overrides from linked data, trying to preserve local overrides already defined).
- *
  * \param id_root: The root ID to create an override from.
  * \param id_reference: Some reference ID used to do some post-processing after overrides have been
- * created, may be NULL. Typically, the Empty object instantiating the linked
+ * created, may be NULL. Typically, the Empty object instantiating the linked collection we
- * collection we override, currently.
+ * override, currently.
  * \return true if override was successfully created.
  */
 bool BKE_lib_override_library_create(
@@ -667,10 +668,10 @@ bool BKE_lib_override_library_create(
 }
 
 /**
- * Converts a given proxy object into a library override.
+ * Convert a given proxy object into a library override.
  *
- * \note This is actually a thin wrapper around \a BKE_lib_override_library_create, only extra work
+ * \note This is a thin wrapper around \a BKE_lib_override_library_create, only extra work is to
- * is to actually convert the proxy itself into an override first.
+ * actually convert the proxy itself into an override first.
  *
  * \return true if override was successfully created.
  */
@@ -679,7 +680,7 @@ bool BKE_lib_override_library_proxy_convert(Main *bmain,
                                             ViewLayer *view_layer,
                                             Object *ob_proxy)
 {
-  /* proxy_group, if defined, is the empty instantiating the collection from which the proxy is
+  /* `proxy_group`, if defined, is the empty instantiating the collection from which the proxy is
    * coming. */
   Object *ob_proxy_group = ob_proxy->proxy_group;
   const bool is_override_instancing_object = ob_proxy_group != NULL;
@@ -688,7 +689,8 @@ bool BKE_lib_override_library_proxy_convert(Main *bmain,
   ID *id_reference = is_override_instancing_object ? &ob_proxy_group->id : &ob_proxy->id;
 
   /* We manually convert the proxy object into a library override, further override handling will
-   * then be handled by BKE_lib_override_library_create() just as for a regular override creation.
+   * then be handled by `BKE_lib_override_library_create()` just as for a regular override
+   * creation.
    */
   ob_proxy->proxy->id.tag |= LIB_TAG_DOIT;
   ob_proxy->proxy->id.newid = &ob_proxy->id;
@@ -728,7 +730,7 @@ bool BKE_lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_
     if (id->tag & LIB_TAG_DOIT && ID_IS_OVERRIDE_LIBRARY_REAL(id)) {
       /* While this should not happen in typical cases (and won't be properly supported here), user
        * is free to do all kind of very bad things, including having different local overrides of a
-       * same linked ID in a same hierarchy... */
+       * same linked ID in a same hierarchy. */
       if (!BLI_ghash_haskey(linkedref_to_old_override, id->override_library->reference)) {
         BLI_ghash_insert(linkedref_to_old_override, id->override_library->reference, id);
         id->override_library->reference->tag |= LIB_TAG_DOIT;
@@ -738,7 +740,7 @@ bool BKE_lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_
   FOREACH_MAIN_ID_END;
 
   /* Make new override from linked data. */
-  /* Note that this call also remap all pointers of tagged IDs from old override IDs to new
+  /* Note that this call also remaps all pointers of tagged IDs from old override IDs to new
    * override IDs (including within the old overrides themselves, since those are tagged too
    * above). */
   const bool success = lib_override_library_create_do(bmain, id_root_reference);
@@ -760,7 +762,7 @@ bool BKE_lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_
           memcpy(id_name_buf, id_override_old->name, sizeof(id_name_buf));
           memcpy(id_override_old->name, id_override_new->name, sizeof(id_override_old->name));
           memcpy(id_override_new->name, id_name_buf, sizeof(id_override_new->name));
-          /* Note that this is very efficient way to keep BMain IDs ordered as expected after
+          /* Note that this is a very efficient way to keep BMain IDs ordered as expected after
            * swapping their names.
            * However, one has to be very careful with this when iterating over the listbase at the
            * same time. Here it works because we only execute this code when we are in the linked
@@ -816,7 +818,7 @@ bool BKE_lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_
    * This improves performances anyway, so everything is fine. */
   FOREACH_MAIN_ID_BEGIN (bmain, id) {
     if (id->tag & LIB_TAG_DOIT) {
-      /* Note that this work because linked IDs are always after local ones (including overrides),
+      /* Note that this works because linked IDs are always after local ones (including overrides),
        * so we will only ever tag an old override ID after we have already checked it in this loop,
        * hence we cannot untag it later. */
       if (id->newid != NULL && ID_IS_LINKED(id)) {
@@ -833,7 +835,8 @@ bool BKE_lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_
   FOREACH_MAIN_ID_END;
   BKE_id_multi_tagged_delete(bmain);
 
-  /* At this point, id_root has very likely been deleted, we need to update it to its new version.
+  /* At this point, `id_root` has very likely been deleted, we need to update it to its new
+   * version.
    */
   id_root = id_root_reference->newid;
 
@@ -855,7 +858,7 @@ bool BKE_lib_override_library_resync(Main *bmain, Scene *scene, ViewLayer *view_
  *
  * \note All IDs tagged with `LIB_TAG_DOIT` will be deleted.
  *
- * \param id_root: The root liboverride ID to resync from.
+ * \param id_root: The root liboverride ID to delete.
  */
 void BKE_lib_override_library_delete(Main *bmain, ID *id_root)
 {
@@ -864,8 +867,7 @@ void BKE_lib_override_library_delete(Main *bmain, ID *id_root)
   /* Tag all collections and objects, as well as other IDs using them. */
   id_root->tag |= LIB_TAG_DOIT;
 
-  /* Make a mapping 'linked reference IDs' -> 'Local override IDs' of existing overrides, and tag
+  /* Tag all library overrides in the chains of dependencies from the given root one. */
-   * linked reference ones to be overridden again. */
   BKE_lib_override_library_override_group_tag(bmain, id_root, LIB_TAG_DOIT, true);
 
   ID *id;
@@ -889,7 +891,7 @@ void BKE_lib_override_library_delete(Main *bmain, ID *id_root)
   }
   FOREACH_MAIN_ID_END;
 
-  /* Should not actually be needed here... */
+  /* Should not actually be needed here. */
   BKE_main_id_tag_all(bmain, LIB_TAG_DOIT, false);
 }
 
@@ -1069,7 +1071,7 @@ IDOverrideLibraryPropertyOperation *BKE_lib_override_library_property_operation_
     return ELEM(subitem_locindex, -1, opop->subitem_local_index) ? opop : NULL;
   }
 
-  /* index == -1 means all indices, that is valid fallback in case we requested specific index.
+  /* `index == -1` means all indices, that is a valid fallback in case we requested specific index.
    */
   if (!strict && (subitem_locindex != subitem_defindex) &&
       (opop = BLI_listbase_bytes_find(
@@ -1230,7 +1232,7 @@ bool BKE_lib_override_library_status_check_local(Main *bmain, ID *local)
   BLI_assert(GS(local->name) == GS(reference->name));
 
   if (GS(local->name) == ID_OB) {
-    /* Our beloved pose's bone cross-data pointers.. Usually, depsgraph evaluation would
+    /* Our beloved pose's bone cross-data pointers. Usually, depsgraph evaluation would
      * ensure this is valid, but in some situations (like hidden collections etc.) this won't
      * be the case, so we need to take care of this ourselves. */
     Object *ob_local = (Object *)local;
@@ -1290,16 +1292,16 @@ bool BKE_lib_override_library_status_check_reference(Main *bmain, ID *local)
 
   if (reference->override_library && (reference->tag & LIB_TAG_OVERRIDE_LIBRARY_REFOK) == 0) {
     if (!BKE_lib_override_library_status_check_reference(bmain, reference)) {
-      /* If reference is also override of another data-block, and its status is not OK,
+      /* If reference is also an override of another data-block, and its status is not OK,
        * then this override is not OK either.
-       * Note that this should only happen when reloading libraries... */
+       * Note that this should only happen when reloading libraries. */
       local->tag &= ~LIB_TAG_OVERRIDE_LIBRARY_REFOK;
       return false;
     }
   }
 
   if (GS(local->name) == ID_OB) {
-    /* Our beloved pose's bone cross-data pointers.. Usually, depsgraph evaluation would
+    /* Our beloved pose's bone cross-data pointers. Usually, depsgraph evaluation would
      * ensure this is valid, but in some situations (like hidden collections etc.) this won't
      * be the case, so we need to take care of this ourselves. */
     Object *ob_local = (Object *)local;
@@ -1332,7 +1334,7 @@ bool BKE_lib_override_library_status_check_reference(Main *bmain, ID *local)
 }
 
 /**
- * Compares local and reference data-blocks and create new override operations as needed,
+ * Compare local and reference data-blocks and create new override operations as needed,
  * or reset to reference values if overriding is not allowed.
  *
  * \note Defining override operations is only mandatory before saving a `.blend` file on disk
@@ -1341,9 +1343,9 @@ bool BKE_lib_override_library_status_check_reference(Main *bmain, ID *local)
  *
  * \note This is by far the biggest operation (the more time-consuming) of the three so far,
  * since it has to go over all properties in depth (all overridable ones at least).
- * Generating diff values and applying overrides are much cheaper.
+ * Generating differential values and applying overrides are much cheaper.
  *
- * \return true if new overriding op was created, or some local data was reset. */
+ * \return true if a new overriding op was created, or some local data was reset. */
 bool BKE_lib_override_library_operations_create(Main *bmain, ID *local)
 {
   BLI_assert(local->override_library != NULL);
@@ -1352,7 +1354,7 @@ bool BKE_lib_override_library_operations_create(Main *bmain, ID *local)
 
   if (!is_template) {
     /* Do not attempt to generate overriding rules from an empty place-holder generated by link
-     * code when it cannot find to actual library/ID. Much better to keep the local data-block as
+     * code when it cannot find the actual library/ID. Much better to keep the local data-block as
      * is in the file in that case, until broken lib is fixed. */
     if (ID_MISSING(local->override_library->reference)) {
       return ret;
@@ -1566,7 +1568,7 @@ static void lib_override_library_id_hierarchy_recursive_reset(Main *bmain, ID *i
 
   lib_override_library_id_reset_do(bmain, id_root);
 
-  /* This way we won't process again that ID should we encounter it again through another
+  /* This way we won't process again that ID, should we encounter it again through another
    * relationship hierarchy.
    * Note that this does not free any memory from relations, so we can still use the entries.
    */
@@ -1609,7 +1611,7 @@ void BKE_lib_override_library_id_hierarchy_reset(Main *bmain, ID *id_root)
   FOREACH_MAIN_ID_END;
 }
 
-/** Set or clear given tag in all operations as unused in that override property data. */
+/** Set or clear given tag in all operations in that override property data. */
 void BKE_lib_override_library_operations_tag(struct IDOverrideLibraryProperty *override_property,
                                              const short tag,
                                              const bool do_set)
@@ -1699,13 +1701,13 @@ void BKE_lib_override_library_update(Main *bmain, ID *local)
   }
 
   /* Do not attempt to apply overriding rules over an empty place-holder generated by link code
-   * when it cannot find to actual library/ID. Much better to keep the local data-block as loaded
+   * when it cannot find the actual library/ID. Much better to keep the local data-block as loaded
    * from the file in that case, until broken lib is fixed. */
   if (ID_MISSING(local->override_library->reference)) {
     return;
   }
 
-  /* Recursively do 'ancestors' overrides first, if any. */
+  /* Recursively do 'ancestor' overrides first, if any. */
   if (local->override_library->reference->override_library &&
       (local->override_library->reference->tag & LIB_TAG_OVERRIDE_LIBRARY_REFOK) == 0) {
     BKE_lib_override_library_update(bmain, local->override_library->reference);
@@ -1717,7 +1719,7 @@ void BKE_lib_override_library_update(Main *bmain, ID *local)
 
   /* XXX We need a way to get off-Main copies of IDs (similar to localized mats/texts/ etc.)!
    *     However, this is whole bunch of code work in itself, so for now plain stupid ID copy
-   *     will do, as inn-efficient as it is. :/
+   *     will do, as innefficient as it is. :/
    *     Actually, maybe not! Since we are swapping with original ID's local content, we want to
    *     keep user-count in correct state when freeing tmp_id
    *     (and that user-counts of IDs used by 'new' local data also remain correct). */
@@ -1738,7 +1740,7 @@ void BKE_lib_override_library_update(Main *bmain, ID *local)
    * manual handling here. */
   BLI_strncpy(tmp_id->name, local->name, sizeof(tmp_id->name));
 
-  /* Those ugly loop-back pointers again... Luckily we only need to deal with the shape keys here,
+  /* Those ugly loop-back pointers again. Luckily we only need to deal with the shape keys here,
    * collections' parents are fully runtime and reconstructed later. */
   Key *local_key = BKE_key_from_id(local);
   Key *tmp_key = BKE_key_from_id(tmp_id);
@@ -1762,7 +1764,7 @@ void BKE_lib_override_library_update(Main *bmain, ID *local)
   BKE_lib_id_swap(bmain, local, tmp_id);
 
   if (local_key != NULL && tmp_key != NULL) {
-    /* This is some kind of hard-coded 'always enforced override'... */
+    /* This is some kind of hard-coded 'always enforced override'. */
     BKE_lib_id_swap(bmain, &local_key->id, &tmp_key->id);
     tmp_key->id.flag |= (local_key->id.flag & LIB_EMBEDDED_DATA_LIB_OVERRIDE);
     /* The swap of local and tmp_id inverted those pointers, we need to redefine proper
@@ -1773,9 +1775,8 @@ void BKE_lib_override_library_update(Main *bmain, ID *local)
     tmp_key->from = tmp_id;
   }
 
-  /* Again, horribly inn-efficient in our case, we need something off-Main
+  /* Again, horribly innefficient in our case, we need something off-Main (aka more generic nolib
-   * (aka more generic nolib copy/free stuff)! */
+   * copy/free stuff)! */
-  /* XXX And crashing in complex cases (e.g. because depsgraph uses same data...). */
   BKE_id_free_ex(bmain, tmp_id, LIB_ID_FREE_NO_UI_USER, true);
 
   if (GS(local->name) == ID_AR) {
@@ -1803,7 +1804,7 @@ void BKE_lib_override_library_update(Main *bmain, ID *local)
   local->tag |= LIB_TAG_OVERRIDE_LIBRARY_REFOK;
 
   /* Full rebuild of Depsgraph! */
-  /* Note: this is really brute force, in theory updates from RNA should have handle this already,
+  /* Note: this is really brute force, in theory updates from RNA should have handled this already,
    * but for now let's play it safe. */
   DEG_id_tag_update_ex(bmain, local, ID_RECALC_ALL);
   DEG_relations_tag_update(bmain);
@@ -1834,12 +1835,11 @@ void BKE_lib_override_library_main_update(Main *bmain)
  * Storage (how to store overriding data into `.blend` files).
  *
  * Basically:
- * 1) Only 'differential' storage needs special handling here. All others (replacing values or
+ * 1) Only 'differential' overrides needs special handling here. All others (replacing values or
  *    inserting/removing items from a collection) can be handled with simply storing current
  *    content of local data-block.
- * 2) We store the differential value into a second 'ghost' data-block,
+ * 2) We store the differential value into a second 'ghost' data-block, which is an empty ID of
- *    which is an empty ID of same type as local one,
+ *    same type as the local one, where we only define values that need differential data.
- *    where we only define values that need differential data.
  *
  * This avoids us having to modify 'real' data-block at write time (and restoring it afterwards),
  * which is inefficient, and potentially dangerous (in case of concurrent access...), while not