Page MenuHome
Differential D12633 Diff 42412 source/blender/blenloader/intern/versioning_300.c

Changeset View

Standalone View

View Options

source/blender/blenloader/intern/versioning_300.c

Show All 40 Lines
#include "DNA_lineart_types.h" #include "DNA_lineart_types.h"
#include "DNA_listBase.h" #include "DNA_listBase.h"
#include "DNA_material_types.h" #include "DNA_material_types.h"
#include "DNA_modifier_types.h" #include "DNA_modifier_types.h"
#include "DNA_text_types.h" #include "DNA_text_types.h"
#include "DNA_workspace_types.h" #include "DNA_workspace_types.h"
#include "BKE_action.h" #include "BKE_action.h"
#include "BKE_anim_data.h"
#include "BKE_animsys.h" #include "BKE_animsys.h"
#include "BKE_asset.h" #include "BKE_asset.h"
#include "BKE_collection.h" #include "BKE_collection.h"
#include "BKE_deform.h" #include "BKE_deform.h"
#include "BKE_fcurve.h" #include "BKE_fcurve.h"
#include "BKE_fcurve_driver.h" #include "BKE_fcurve_driver.h"
#include "BKE_idprop.h" #include "BKE_idprop.h"
#include "BKE_lib_id.h" #include "BKE_lib_id.h"
▲ Show 20 Lines • Show All 387 Lines • ▼ Show 20 Lines else if (seq->type == SEQ_TYPE_META) {
do_versions_sequencer_speed_effect_recursive(scene, &seq->seqbase); do_versions_sequencer_speed_effect_recursive(scene, &seq->seqbase);
} }
} }
#undef SEQ_SPEED_INTEGRATE #undef SEQ_SPEED_INTEGRATE
#undef SEQ_SPEED_COMPRESS_IPO_Y #undef SEQ_SPEED_COMPRESS_IPO_Y
} }
/* Swap drivers and keyframe data for the Icosphere inputs */
sybrenUnsubmitted
Done
Inline Actions

This is not what the function does. It is what it's being used for, but that's not the right kind of thing to document here. Something like this would be better:

/**
 * Ensure the FCurve with the 1st RNA path gets the 2nd RNA path, and vice versa.
 * \param list the list of FCurves to use to find the curves.
 * \param sock0_rna_path the first RNA path.
 * \param sock1_rna_path the second RNA path.
 */

Maybe you'd also want to change the code to count from 1 instead of from 0, so that it's more natural when the documentation mentions "first" and "second" (I'm never a fan of "zeroeth" and "first").

sybren: This is not what the function does. It is what it's being used for, but that's not the right…
static void do_versions_geo_node_icosphere_input_swap(Main *bmain, bNode *node, AnimData *adt)
sybrenUnsubmitted
Done
Inline Actions

The name doesn't really convey the functionality of the function. It doesn't swap the fcurves, it swaps their rna_path. swap_fcurve_rna_path would be a better name.

sybren: The name doesn't really convey the functionality of the function. It doesn't swap the fcurves…
sybrenUnsubmitted
Done
Inline Actions

I wouldn't mind if this function were to be moved to versioning_common.cc, as it's generic enough to be reused later.

sybren: I wouldn't mind if this function were to be moved to `versioning_common.cc`, as it's generic…
sybrenUnsubmitted
Done
Inline Actions

Change ListBase *list to ListBase /*FCurve*/ *fcurves, so that it's clearer what's actually contained in the list (note that I added a comment and renamed the parameter).

sybren: Change `ListBase *list` to `ListBase /*FCurve*/ *fcurves`, so that it's clearer what's actually…
{
const size_t node_name_length = strlen(node->name);
const size_t node_name_escaped_max_length = (node_name_length * 2);
char *node_name_escaped = MEM_mallocN(node_name_escaped_max_length + 1, "escaped name");
BLI_str_escape(node_name_escaped, node->name, node_name_escaped_max_length);
char *sock0_rna_path = BLI_sprintfN("nodes[\"%s\"].inputs[0].default_value", node_name_escaped);
char *sock1_rna_path = BLI_sprintfN("nodes[\"%s\"].inputs[1].default_value", node_name_escaped);
sybrenUnsubmitted
Done
Inline Actions

You can return early here, and avoid an else. This also makes it clear that after this code, nothing more happens (instead of having to look down and inspect the rest of the code to come to the same conclusion).

sybren: You can `return` early here, and avoid an `else`. This also makes it clear that after this code…
FCurve *fcurve0 = BKE_fcurve_find(&adt->drivers, sock0_rna_path, 0);
FCurve *fcurve1 = BKE_fcurve_find(&adt->drivers, sock1_rna_path, 0);
if (fcurve0 != NULL && fcurve1 != NULL) {
const char *temp = fcurve0->rna_path;
fcurve0->rna_path = fcurve1->rna_path;
fcurve1->rna_path = temp;
}
fcurve0 = BKE_fcurve_find(&adt->action->curves, sock0_rna_path, 0);
fcurve1 = BKE_fcurve_find(&adt->action->curves, sock1_rna_path, 0);
sybrenUnsubmitted
Done
Inline Actions

It might be nice to have a little documentation here that says what the code does.

sybren: It might be nice to have a little documentation here that says what the code does.
if (fcurve0 != NULL && fcurve1 != NULL) {
const char *temp = fcurve0->rna_path;
fcurve0->rna_path = fcurve1->rna_path;
sybrenUnsubmitted
Not Done
Inline Actions

This should get a comment as to why the * 2 is there (and why it's enough).

sybren: This should get a comment as to why the `* 2` is there (and why it's enough).
fcurve1->rna_path = temp;
}
MEM_freeN(sock0_rna_path);
MEM_freeN(sock1_rna_path);
MEM_freeN(node_name_escaped);
}
void do_versions_after_linking_300(Main *bmain, ReportList *UNUSED(reports)) void do_versions_after_linking_300(Main *bmain, ReportList *UNUSED(reports))
{ {
if (MAIN_VERSION_ATLEAST(bmain, 300, 0) && !MAIN_VERSION_ATLEAST(bmain, 300, 1)) { if (MAIN_VERSION_ATLEAST(bmain, 300, 0) && !MAIN_VERSION_ATLEAST(bmain, 300, 1)) {
/* Set zero user text objects to have a fake user. */ /* Set zero user text objects to have a fake user. */
LISTBASE_FOREACH (Text *, text, &bmain->texts) { LISTBASE_FOREACH (Text *, text, &bmain->texts) {
if (text->id.us == 0) { if (text->id.us == 0) {
id_fake_user_set(&text->id); id_fake_user_set(&text->id);
} }
Show All 40 Linesvoid do_versions_after_linking_300(Main *bmain, ReportList *UNUSED(reports))
if (!MAIN_VERSION_ATLEAST(bmain, 300, 13)) { if (!MAIN_VERSION_ATLEAST(bmain, 300, 13)) {
LISTBASE_FOREACH (Scene *, scene, &bmain->scenes) { LISTBASE_FOREACH (Scene *, scene, &bmain->scenes) {
if (scene->ed != NULL) { if (scene->ed != NULL) {
do_versions_sequencer_speed_effect_recursive(scene, &scene->ed->seqbase); do_versions_sequencer_speed_effect_recursive(scene, &scene->ed->seqbase);
} }
} }
} }
/** if (!MAIN_VERSION_ATLEAST(bmain, 300, 26)) {
* Versioning code until next subversion bump goes here.
*
* \note Be sure to check when bumping the version:
* - #blo_do_versions_300 in this file.
* - "versioning_userdef.c", #blo_do_versions_userdef
* - "versioning_userdef.c", #do_versions_theme
*
* \note Keep this message at the bottom of the function.
*/
{
/* Keep this block, even when empty. */
do_versions_idproperty_ui_data(bmain); do_versions_idproperty_ui_data(bmain);
LISTBASE_FOREACH (Scene *, scene, &bmain->scenes) { LISTBASE_FOREACH (Scene *, scene, &bmain->scenes) {
ToolSettings *tool_settings = scene->toolsettings; ToolSettings *tool_settings = scene->toolsettings;
ImagePaintSettings *imapaint = &tool_settings->imapaint; ImagePaintSettings *imapaint = &tool_settings->imapaint;
if (imapaint->canvas != NULL && if (imapaint->canvas != NULL &&
ELEM(imapaint->canvas->type, IMA_TYPE_R_RESULT, IMA_TYPE_COMPOSITE)) { ELEM(imapaint->canvas->type, IMA_TYPE_R_RESULT, IMA_TYPE_COMPOSITE)) {
imapaint->canvas = NULL; imapaint->canvas = NULL;
} }
if (imapaint->stencil != NULL && if (imapaint->stencil != NULL &&
ELEM(imapaint->stencil->type, IMA_TYPE_R_RESULT, IMA_TYPE_COMPOSITE)) { ELEM(imapaint->stencil->type, IMA_TYPE_R_RESULT, IMA_TYPE_COMPOSITE)) {
imapaint->stencil = NULL; imapaint->stencil = NULL;
} }
if (imapaint->clone != NULL && if (imapaint->clone != NULL &&
ELEM(imapaint->clone->type, IMA_TYPE_R_RESULT, IMA_TYPE_COMPOSITE)) { ELEM(imapaint->clone->type, IMA_TYPE_R_RESULT, IMA_TYPE_COMPOSITE)) {
imapaint->clone = NULL; imapaint->clone = NULL;
} }
} }
LISTBASE_FOREACH (Brush *, brush, &bmain->brushes) { LISTBASE_FOREACH (Brush *, brush, &bmain->brushes) {
if (brush->clone.image != NULL && if (brush->clone.image != NULL &&
ELEM(brush->clone.image->type, IMA_TYPE_R_RESULT, IMA_TYPE_COMPOSITE)) { ELEM(brush->clone.image->type, IMA_TYPE_R_RESULT, IMA_TYPE_COMPOSITE)) {
brush->clone.image = NULL; brush->clone.image = NULL;
} }
} }
FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
AnimData *adt = BKE_animdata_from_id(&ntree->id);
if (ntree->type == NTREE_GEOMETRY && adt != NULL) {
sybrenUnsubmitted
Not Done
Inline Actions

Swap the condition & continue early. This reduces the cognitive complexity of the remainder of the loop body.

sybren: Swap the condition & `continue` early. This reduces the cognitive complexity of the remainder…
LISTBASE_FOREACH (bNode *, node, &ntree->nodes) {
if (node->type == GEO_NODE_MESH_PRIMITIVE_ICO_SPHERE) {
do_versions_geo_node_icosphere_input_swap(bmain, node, adt);
}
}
}
}
FOREACH_NODETREE_END;
}
/**
* Versioning code until next subversion bump goes here.
*
* \note Be sure to check when bumping the version:
* - #blo_do_versions_300 in this file.
* - "versioning_userdef.c", #blo_do_versions_userdef
* - "versioning_userdef.c", #do_versions_theme
*
* \note Keep this message at the bottom of the function.
*/
{
/* Keep this block, even when empty. */
} }
} }
static void version_switch_node_input_prefix(Main *bmain) static void version_switch_node_input_prefix(Main *bmain)
{ {
FOREACH_NODETREE_BEGIN (bmain, ntree, id) { FOREACH_NODETREE_BEGIN (bmain, ntree, id) {
if (ntree->type == NTREE_GEOMETRY) { if (ntree->type == NTREE_GEOMETRY) {
LISTBASE_FOREACH (bNode *, node, &ntree->nodes) { LISTBASE_FOREACH (bNode *, node, &ntree->nodes) {
▲ Show 20 Lines • Show All 949 Lines • ▼ Show 20 Lines LISTBASE_FOREACH (bNodeTree *, ntree, &bmain->nodetrees) {
BLI_snprintf(node->idname, BLI_snprintf(node->idname,
sizeof(node->idname), sizeof(node->idname),
"FunctionNodeLegacy%s", "FunctionNodeLegacy%s",
temp_idname + strlen("FunctionNode")); temp_idname + strlen("FunctionNode"));
} }
} }
} }
} }
No newline at end of file
sybrenUnsubmitted
Not Done
Inline Actions

Keep formatting changes out of functional patches.

sybren: Keep formatting changes out of functional patches.