diff --git a/source/blender/blenkernel/BKE_lib_id.hh b/source/blender/blenkernel/BKE_lib_id.hh index 1d5e6718bdf..d9b541f0c1c 100644 --- a/source/blender/blenkernel/BKE_lib_id.hh +++ b/source/blender/blenkernel/BKE_lib_id.hh @@ -34,6 +34,7 @@ #include "BLI_compiler_attrs.h" #include "BLI_set.hh" +#include "BLI_string_ref.hh" #include "BLI_utildefines.h" #include "BLI_vector.hh" @@ -256,13 +257,88 @@ void *BKE_libblock_copy(Main *bmain, const ID *id) ATTR_WARN_UNUSED_RESULT ATTR_ */ void BKE_id_move_to_same_lib(Main &bmain, ID &id, const ID &owner_id); +/** How to handle ID rename in case requested name is already used by another ID. */ +enum class IDNewNameMode { + /** + * Never rename another existing ID if the target name is already in use. The renamed ID will get + * a name modified with a numerical suffix instead. + */ + RenameExistingNever = 0, + /** + * Always rename another existing ID if the target name is already in use. The renamed ID will + * get the requested unmodified name. + */ + RenameExistingAlways = 1, + /** + * Only rename another existing ID if the target name is already in use, when the current name of + * the renamed ID has the same root as the other ID name (i.e. they have the same name, besides + * the numeric suffix). + * E.g. Assuming there is already an existing ID named `Object`: + * - Renaming `Object.001` to `Object`: rename to `Object`, the existing `Object` ID is renamed + * to e.g. `Object.001` + * - Renaming `Cube` to `Object`: rename to `Object.001`, the existing `Object` ID is not + * renamed. + */ + RenameExistingSameRoot = 2, +}; + +/** Information about how an ID rename went on. */ +struct IDNewNameResult { + /** How the renaming wnet on. */ + enum class Action { + /** ID was not renamed, because requested new name was already the ID's name. */ + UNCHANGED = 0, + /** + * ID was not renamed, because requested new name would collide with another existing ID's + * name, and the first available unique name is already current ID's name. + */ + UNCHANGED_COLLISION = 1, + /** Successfully renamed, wihtout any collision with another ID's name. */ + RENAMED_NO_COLLISION = 2, + /** Successfully renamed, requested new name was adjusted to avoid collision with another ID. + */ + RENAMED_COLLISION_ADJUSTED = 3, + /** + * Successfully renamed, requested new name was enforced onto given ID, and another ID had to + * be renamed to avoid name collision. + */ + RENAMED_COLLISION_FORCED = 4, + } action = Action::UNCHANGED; + + /** The colliding ID, if any. + * + * \warning Currently will be `nullptr` in #RENAMED_COLLISION_ADJUSTED case, for performance + * reasons (avoid an ID lookup by name) when doing 'standard' #RenameExistingNever renames. + */ + ID *other_id = nullptr; +}; + /** * Sets the name of a block to name, suitably adjusted for uniqueness. + * + * \return true if the name of the ID was actually modified. */ -void BKE_libblock_rename(Main *bmain, ID *id, const char *name) ATTR_NONNULL(); +IDNewNameResult BKE_libblock_rename(Main &bmain, + ID &id, + blender::StringRefNull name, + const IDNewNameMode mode = IDNewNameMode::RenameExistingNever); -ID *BKE_libblock_find_name(Main *bmain, short type, const char *name) ATTR_WARN_UNUSED_RESULT - ATTR_NONNULL(); +/** + * Like #BKE_libblock_rename, but also performs additional higher-level updates like depsgraph + * tagging when renaming Metaball objects, etc. + * + * \return true if the name of the ID was actually modified. + */ +IDNewNameResult BKE_id_rename(Main &bmain, + ID &id, + blender::StringRefNull name, + const IDNewNameMode mode = IDNewNameMode::RenameExistingNever); + +ID *BKE_libblock_find_name(Main *bmain, + short type, + const char *name, + const std::optional lib = std::nullopt) + ATTR_WARN_UNUSED_RESULT ATTR_NONNULL(); ID *BKE_libblock_find_session_uid(Main *bmain, short type, uint32_t session_uid); ID *BKE_libblock_find_name_and_library(Main *bmain, short type, @@ -619,14 +695,14 @@ void BKE_lib_id_expand_local(Main *bmain, ID *id, int flags); * \param do_linked_data: if true, also ensure a unique name in case the given ID is linked * (otherwise, just ensure that it is properly sorted). * - * \return true if the ID's name has been modified (either from given `name` parameter, or because - * its current name was colliding with another existing ID). + * \return How renaming went on, see #IDNewNameResult for details. */ -bool BKE_id_new_name_validate(Main *bmain, - ListBase *lb, - ID *id, - const char *newname, - bool do_linked_data) ATTR_NONNULL(1, 2, 3); +IDNewNameResult BKE_id_new_name_validate(Main &bmain, + ListBase &lb, + ID &id, + const char *newname, + IDNewNameMode mode, + bool do_linked_data); /** * Pull an ID out of a library (make it local). Only call this for IDs that @@ -747,6 +823,8 @@ void BKE_library_make_local(Main *bmain, void BKE_id_tag_set_atomic(ID *id, int tag); void BKE_id_tag_clear_atomic(ID *id, int tag); +/** Check that given ID pointer actually is in given `bmain`. */ +bool BKE_id_is_in_main(Main *bmain, ID *id); /** * Check that given ID pointer actually is in G_MAIN. * Main intended use is for debug asserts in places we cannot easily get rid of #G_MAIN. diff --git a/source/blender/blenkernel/intern/asset_edit.cc b/source/blender/blenkernel/intern/asset_edit.cc index b1120dbdd24..069f00b2c23 100644 --- a/source/blender/blenkernel/intern/asset_edit.cc +++ b/source/blender/blenkernel/intern/asset_edit.cc @@ -221,7 +221,7 @@ static bool asset_write_in_library(Main &bmain, Main *new_main = asset_main_create_from_ID(bmain, id, &new_id); std::string new_name = name; - BKE_libblock_rename(new_main, new_id, new_name.c_str()); + BKE_libblock_rename(*new_main, *new_id, new_name); id_fake_user_set(new_id); BlendFileWriteParams blend_file_write_params{}; diff --git a/source/blender/blenkernel/intern/blendfile.cc b/source/blender/blenkernel/intern/blendfile.cc index ef1a595eb05..75dc41fc3c1 100644 --- a/source/blender/blenkernel/intern/blendfile.cc +++ b/source/blender/blenkernel/intern/blendfile.cc @@ -348,7 +348,8 @@ static bool reuse_bmain_move_id(ReuseOldBMainData *reuse_data, id->lib = lib; BLI_addtail(new_lb, id); - BKE_id_new_name_validate(new_bmain, new_lb, id, nullptr, true); + BKE_id_new_name_validate( + *new_bmain, *new_lb, *id, nullptr, IDNewNameMode::RenameExistingNever, true); BKE_lib_libblock_session_uid_renew(id); /* Remap to itself, to avoid re-processing this ID again. */ diff --git a/source/blender/blenkernel/intern/lib_id.cc b/source/blender/blenkernel/intern/lib_id.cc index f18a3dcbd9e..d9de808b3dd 100644 --- a/source/blender/blenkernel/intern/lib_id.cc +++ b/source/blender/blenkernel/intern/lib_id.cc @@ -36,6 +36,7 @@ #include "BLI_ghash.h" #include "BLI_linklist.h" #include "BLI_memarena.h" +#include "BLI_string_ref.hh" #include "BLI_string_utils.hh" #include "BLT_translation.hh" @@ -219,7 +220,16 @@ void BKE_lib_id_clear_library_data(Main *bmain, ID *id, const int flags) id->tag &= ~(ID_TAG_INDIRECT | ID_TAG_EXTERN); id->flag &= ~ID_FLAG_INDIRECT_WEAK_LINK; if (id_in_mainlist) { - if (BKE_id_new_name_validate(bmain, which_libbase(bmain, GS(id->name)), id, nullptr, false)) { + IDNewNameResult result = BKE_id_new_name_validate(*bmain, + *which_libbase(bmain, GS(id->name)), + *id, + nullptr, + IDNewNameMode::RenameExistingNever, + false); + if (!ELEM(result.action, + IDNewNameResult::Action::UNCHANGED, + IDNewNameResult::Action::UNCHANGED_COLLISION)) + { bmain->is_memfile_undo_written = false; } } @@ -863,8 +873,9 @@ void BKE_id_move_to_same_lib(Main &bmain, ID &id, const ID &owner_id) id.tag |= ID_TAG_INDIRECT; BKE_main_namemap_remove_name(&bmain, &id, BKE_id_name(id)); - ListBase *lb = which_libbase(&bmain, GS(id.name)); - BKE_id_new_name_validate(&bmain, lb, &id, BKE_id_name(id), true); + ListBase &lb = *which_libbase(&bmain, GS(id.name)); + BKE_id_new_name_validate( + bmain, lb, id, BKE_id_name(id), IDNewNameMode::RenameExistingNever, true); } static void id_embedded_swap(ID **embedded_id_a, @@ -1104,7 +1115,7 @@ void BKE_libblock_management_main_add(Main *bmain, void *idv) BLI_addtail(lb, id); /* We need to allow adding extra datablocks into libraries too, e.g. to support generating new * overrides for recursive resync. */ - BKE_id_new_name_validate(bmain, lb, id, nullptr, true); + BKE_id_new_name_validate(*bmain, *lb, *id, nullptr, IDNewNameMode::RenameExistingNever, true); /* alphabetic insertion: is in new_id */ id->tag &= ~(ID_TAG_NO_MAIN | ID_TAG_NO_USER_REFCOUNT); bmain->is_memfile_undo_written = false; @@ -1242,7 +1253,8 @@ void BKE_main_id_repair_duplicate_names_listbase(Main *bmain, ListBase *lb) } for (i = 0; i < lb_len; i++) { if (!BLI_gset_add(gset, BKE_id_name(*id_array[i]))) { - BKE_id_new_name_validate(bmain, lb, id_array[i], nullptr, false); + BKE_id_new_name_validate( + *bmain, *lb, *id_array[i], nullptr, IDNewNameMode::RenameExistingNever, false); } } BLI_gset_free(gset, nullptr); @@ -1361,7 +1373,7 @@ void *BKE_libblock_alloc_in_lib(Main *bmain, BKE_main_lock(bmain); BLI_addtail(lb, id); - BKE_id_new_name_validate(bmain, lb, id, name, true); + BKE_id_new_name_validate(*bmain, *lb, *id, name, IDNewNameMode::RenameExistingNever, true); bmain->is_memfile_undo_written = false; /* alphabetic insertion: is in new_id */ BKE_main_unlock(bmain); @@ -1639,11 +1651,22 @@ void *BKE_libblock_copy(Main *bmain, const ID *id) /* ***************** ID ************************ */ -ID *BKE_libblock_find_name(Main *bmain, const short type, const char *name) +ID *BKE_libblock_find_name(Main *bmain, + const short type, + const char *name, + const std::optional lib) { ListBase *lb = which_libbase(bmain, type); BLI_assert(lb != nullptr); - return static_cast(BLI_findstring(lb, name, offsetof(ID, name) + 2)); + + ID *id = static_cast(BLI_findstring(lb, name, offsetof(ID, name) + 2)); + if (lib) { + while (id && id->lib != *lib) { + id = static_cast(BLI_listbase_findafter_string_ptr( + reinterpret_cast(id), name, offsetof(ID, name) + 2)); + } + } + return id; } ID *BKE_libblock_find_session_uid(Main *bmain, const short type, const uint32_t session_uid) @@ -1821,29 +1844,32 @@ void id_sort_by_name(ListBase *lb, ID *id, ID *id_sorting_hint) #undef ID_SORT_STEP_SIZE } -bool BKE_id_new_name_validate( - Main *bmain, ListBase *lb, ID *id, const char *newname, const bool do_linked_data) +IDNewNameResult BKE_id_new_name_validate(Main &bmain, + ListBase &lb, + ID &id, + const char *newname, + IDNewNameMode mode, + const bool do_linked_data) { - bool result = false; char name[MAX_ID_NAME - 2]; /* If library, don't rename (unless explicitly required), but do ensure proper sorting. */ - if (!do_linked_data && ID_IS_LINKED(id)) { - id_sort_by_name(lb, id, nullptr); + if (!do_linked_data && ID_IS_LINKED(&id)) { + id_sort_by_name(&lb, &id, nullptr); - return result; + return {IDNewNameResult::Action::UNCHANGED, nullptr}; } /* If no name given, use name of current ID. */ if (newname == nullptr) { - newname = BKE_id_name(*id); + newname = BKE_id_name(id); } /* Make a copy of given name (newname args can be const). */ STRNCPY(name, newname); if (name[0] == '\0') { /* Disallow empty names. */ - STRNCPY_UTF8(name, DATA_(BKE_idtype_idcode_to_name(GS(id->name)))); + STRNCPY_UTF8(name, DATA_(BKE_idtype_idcode_to_name(GS(id.name)))); } else { /* disallow non utf8 chars, @@ -1851,13 +1877,67 @@ bool BKE_id_new_name_validate( BLI_str_utf8_invalid_strip(name, strlen(name)); } - result = BKE_main_namemap_get_name(bmain, id, name, false); - if (!result && !STREQ(BKE_id_name(*id), name)) { - result = true; + /* Store original requested new name, in modes that may solve name conflict by renaming the + * existing conflicting ID. */ + char orig_name[MAX_ID_NAME - 2]; + if (ELEM(mode, IDNewNameMode::RenameExistingAlways, IDNewNameMode::RenameExistingSameRoot)) { + STRNCPY(orig_name, name); } - BLI_strncpy(id->name + 2, name, sizeof(id->name) - 2); - id_sort_by_name(lb, id, nullptr); + const bool had_name_collision = BKE_main_namemap_get_name(&bmain, &id, name, false); + + if (had_name_collision && + ELEM(mode, IDNewNameMode::RenameExistingAlways, IDNewNameMode::RenameExistingSameRoot)) + { + char prev_name[MAX_ID_NAME - 2]; + char prev_name_root[MAX_ID_NAME - 2]; + int prev_number = 0; + char new_name_root[MAX_ID_NAME - 2]; + int new_number = 0; + STRNCPY(prev_name, BKE_id_name(id)); + if (mode == IDNewNameMode::RenameExistingSameRoot) { + BLI_string_split_name_number(BKE_id_name(id), '.', prev_name_root, &prev_number); + BLI_string_split_name_number(name, '.', new_name_root, &new_number); + } + + ID *id_other = BKE_libblock_find_name(&bmain, GS(id.name), orig_name, id.lib); + BLI_assert(id_other); + + /* In case of #RenameExistingSameRoot, the existing ID (`id_other`) is only renamed if it has + * the same 'root' name as the current name of the renamed `id`. */ + if (mode == IDNewNameMode::RenameExistingAlways || + (mode == IDNewNameMode::RenameExistingSameRoot && STREQ(prev_name_root, new_name_root))) + { + BLI_strncpy(id_other->name + 2, name, sizeof(id_other->name) - 2); + id_sort_by_name(&lb, id_other, nullptr); + + const bool is_idname_changed = !STREQ(BKE_id_name(id), orig_name); + IDNewNameResult result = {IDNewNameResult::Action::UNCHANGED_COLLISION, id_other}; + if (is_idname_changed) { + BLI_strncpy(id.name + 2, orig_name, sizeof(id.name) - 2); + result.action = IDNewNameResult::Action::RENAMED_COLLISION_FORCED; + } + id_sort_by_name(&lb, &id, nullptr); + return result; + } + } + + /* The requested new name may be available (not collide with any other existing ID name), but + * still differ from the current name of the renamed ID. + * Conversely, the requested new name may have been colliding with an existing one, and the + * generated unique name may end up being the current ID's name. */ + const bool is_idname_changed = !STREQ(BKE_id_name(id), name); + + IDNewNameResult result = {IDNewNameResult::Action::UNCHANGED, nullptr}; + if (is_idname_changed) { + BLI_strncpy(id.name + 2, name, sizeof(id.name) - 2); + result.action = had_name_collision ? IDNewNameResult::Action::RENAMED_COLLISION_ADJUSTED : + IDNewNameResult::Action::RENAMED_NO_COLLISION; + } + else if (had_name_collision) { + result.action = IDNewNameResult::Action::UNCHANGED_COLLISION; + } + id_sort_by_name(&lb, &id, nullptr); return result; } @@ -2230,17 +2310,53 @@ void BKE_library_make_local(Main *bmain, #endif } -void BKE_libblock_rename(Main *bmain, ID *id, const char *name) +IDNewNameResult BKE_libblock_rename(Main &bmain, + ID &id, + blender::StringRefNull name, + const IDNewNameMode mode) { - BLI_assert(ID_IS_EDITABLE(id)); - if (STREQ(BKE_id_name(*id), name)) { - return; + BLI_assert(BKE_id_is_in_main(&bmain, &id)); + + if (STREQ(BKE_id_name(id), name.c_str())) { + return {IDNewNameResult::Action::UNCHANGED, nullptr}; } - BKE_main_namemap_remove_name(bmain, id, BKE_id_name(*id)); - ListBase *lb = which_libbase(bmain, GS(id->name)); - if (BKE_id_new_name_validate(bmain, lb, id, name, true)) { - bmain->is_memfile_undo_written = false; + BKE_main_namemap_remove_name(&bmain, &id, BKE_id_name(id)); + ListBase &lb = *which_libbase(&bmain, GS(id.name)); + IDNewNameResult result = BKE_id_new_name_validate(bmain, lb, id, name.c_str(), mode, true); + if (!ELEM(result.action, + IDNewNameResult::Action::UNCHANGED, + IDNewNameResult::Action::UNCHANGED_COLLISION)) + { + bmain.is_memfile_undo_written = false; } + return result; +} + +IDNewNameResult BKE_id_rename(Main &bmain, + ID &id, + blender::StringRefNull name, + const IDNewNameMode mode) +{ + const IDNewNameResult result = BKE_libblock_rename(bmain, id, name, mode); + + if (!ELEM(result.action, + IDNewNameResult::Action::UNCHANGED, + IDNewNameResult::Action::UNCHANGED_COLLISION)) + { + switch (GS(id.name)) { + case ID_OB: { + Object &ob = reinterpret_cast(id); + if (ob.type == OB_MBALL) { + DEG_id_tag_update(&ob.id, ID_RECALC_GEOMETRY); + } + break; + } + default: + break; + } + } + + return result; } void BKE_id_full_name_get(char name[MAX_ID_FULL_NAME], const ID *id, char separator_char) @@ -2304,11 +2420,16 @@ void BKE_id_tag_clear_atomic(ID *id, int tag) atomic_fetch_and_and_int32(&id->tag, ~tag); } -bool BKE_id_is_in_global_main(ID *id) +bool BKE_id_is_in_main(Main *bmain, ID *id) { /* We do not want to fail when id is nullptr here, even though this is a bit strange behavior... */ - return (id == nullptr || BLI_findindex(which_libbase(G_MAIN, GS(id->name)), id) != -1); + return (id == nullptr || BLI_findindex(which_libbase(bmain, GS(id->name)), id) != -1); +} + +bool BKE_id_is_in_global_main(ID *id) +{ + return BKE_id_is_in_main(G_MAIN, id); } bool BKE_id_can_be_asset(const ID *id) diff --git a/source/blender/blenkernel/intern/lib_id_test.cc b/source/blender/blenkernel/intern/lib_id_test.cc index 2d906c03dd3..cc42cf6460a 100644 --- a/source/blender/blenkernel/intern/lib_id_test.cc +++ b/source/blender/blenkernel/intern/lib_id_test.cc @@ -71,11 +71,9 @@ static void change_lib(Main *bmain, ID *id, Library *lib) BKE_main_namemap_get_name(bmain, id, id->name + 2, false); } -static void change_name(Main *bmain, ID *id, const char *name) +static IDNewNameResult change_name(Main *bmain, ID *id, const char *name, const IDNewNameMode mode) { - BKE_main_namemap_remove_name(bmain, id, id->name + 2); - BLI_strncpy(id->name + 2, name, MAX_NAME); - BKE_id_new_name_validate(bmain, &bmain->objects, id, nullptr, true); + return BKE_libblock_rename(*bmain, *id, name, mode); } TEST(lib_id_main_sort, linked_ids_1) @@ -114,7 +112,7 @@ TEST(lib_id_main_sort, linked_ids_1) EXPECT_EQ(ctx.bmain->name_map_global, nullptr); } -TEST(lib_id_main_unique_name, local_ids_1) +TEST(lib_id_main_unique_name, local_ids_rename_existing_never) { LibIDMainSortTestContext ctx; EXPECT_TRUE(BLI_listbase_is_empty(&ctx.bmain->libraries)); @@ -126,8 +124,28 @@ TEST(lib_id_main_unique_name, local_ids_1) EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); - change_name(ctx.bmain, id_c, "OB_A"); + IDNewNameResult result; + /* Rename to different root name. */ + result = change_name(ctx.bmain, id_c, "OB_A", IDNewNameMode::RenameExistingNever); + + EXPECT_EQ(result.action, IDNewNameResult::Action::RENAMED_COLLISION_ADJUSTED); + // EXPECT_EQ(result.other_id, id_a); /* other_id purposedly not looked-up currently. */ + EXPECT_EQ(result.other_id, nullptr); + EXPECT_STREQ(id_c->name + 2, "OB_A.001"); + EXPECT_STREQ(id_a->name + 2, "OB_A"); + EXPECT_TRUE(ctx.bmain->objects.first == id_a); + EXPECT_TRUE(ctx.bmain->objects.last == id_b); + test_lib_id_main_sort_check_order({id_a, id_c, id_b}); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + /* Rename to same root name. */ + result = change_name(ctx.bmain, id_c, "OB_A", IDNewNameMode::RenameExistingNever); + + EXPECT_EQ(result.action, IDNewNameResult::Action::UNCHANGED_COLLISION); + // EXPECT_EQ(result.other_id, id_a); /* other_id purposedly not looked-up currently. */ + EXPECT_EQ(result.other_id, nullptr); EXPECT_STREQ(id_c->name + 2, "OB_A.001"); EXPECT_STREQ(id_a->name + 2, "OB_A"); EXPECT_TRUE(ctx.bmain->objects.first == id_a); @@ -150,13 +168,100 @@ TEST(lib_id_main_unique_name, local_ids_1) EXPECT_STREQ(future_name, "OB_BBBB"); /* Name too long, needs to be truncated. */ STRNCPY(future_name, "OB_BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"); - change_name(ctx.bmain, id_a, future_name); + change_name(ctx.bmain, id_a, future_name, IDNewNameMode::RenameExistingNever); EXPECT_STREQ(id_a->name + 2, future_name); EXPECT_STREQ(future_name, "OB_BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"); EXPECT_TRUE(BKE_main_namemap_get_name(ctx.bmain, id_c, future_name, false)); EXPECT_STREQ(future_name, "OB_BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB"); } +TEST(lib_id_main_unique_name, local_ids_rename_existing_always) +{ + LibIDMainSortTestContext ctx; + EXPECT_TRUE(BLI_listbase_is_empty(&ctx.bmain->libraries)); + + ID *id_c = static_cast(BKE_id_new(ctx.bmain, ID_OB, "OB_C")); + ID *id_a = static_cast(BKE_id_new(ctx.bmain, ID_OB, "OB_A")); + ID *id_b = static_cast(BKE_id_new(ctx.bmain, ID_OB, "OB_B")); + test_lib_id_main_sort_check_order({id_a, id_b, id_c}); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + IDNewNameResult result; + + /* Rename to different root name. */ + result = change_name(ctx.bmain, id_c, "OB_A", IDNewNameMode::RenameExistingAlways); + + EXPECT_EQ(result.action, IDNewNameResult::Action::RENAMED_COLLISION_FORCED); + EXPECT_EQ(result.other_id, id_a); + EXPECT_STREQ(id_c->name + 2, "OB_A"); + EXPECT_STREQ(id_a->name + 2, "OB_A.001"); + EXPECT_TRUE(ctx.bmain->objects.first == id_c); + EXPECT_TRUE(ctx.bmain->objects.last == id_b); + test_lib_id_main_sort_check_order({id_c, id_a, id_b}); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + /* Rename to same root name. */ + result = change_name(ctx.bmain, id_a, "OB_A", IDNewNameMode::RenameExistingAlways); + + EXPECT_EQ(result.action, IDNewNameResult::Action::RENAMED_COLLISION_FORCED); + EXPECT_EQ(result.other_id, id_c); + EXPECT_STREQ(id_c->name + 2, "OB_A.001"); + EXPECT_STREQ(id_a->name + 2, "OB_A"); + EXPECT_TRUE(ctx.bmain->objects.first == id_a); + EXPECT_TRUE(ctx.bmain->objects.last == id_b); + test_lib_id_main_sort_check_order({id_a, id_c, id_b}); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); +} + +TEST(lib_id_main_unique_name, local_ids_rename_existing_same_root) +{ + LibIDMainSortTestContext ctx; + EXPECT_TRUE(BLI_listbase_is_empty(&ctx.bmain->libraries)); + + ID *id_c = static_cast(BKE_id_new(ctx.bmain, ID_OB, "OB_C")); + ID *id_a = static_cast(BKE_id_new(ctx.bmain, ID_OB, "OB_A")); + ID *id_b = static_cast(BKE_id_new(ctx.bmain, ID_OB, "OB_B")); + test_lib_id_main_sort_check_order({id_a, id_b, id_c}); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + IDNewNameResult result; + + /* Rename to different root name. */ + result = change_name(ctx.bmain, id_c, "OB_A", IDNewNameMode::RenameExistingSameRoot); + + EXPECT_EQ(result.action, IDNewNameResult::Action::RENAMED_COLLISION_ADJUSTED); + // EXPECT_EQ(result.other_id, id_a); /* other_id purposedly not looked-up currently. */ + EXPECT_EQ(result.other_id, nullptr); + EXPECT_STREQ(id_c->name + 2, "OB_A.001"); + EXPECT_STREQ(id_a->name + 2, "OB_A"); + EXPECT_TRUE(ctx.bmain->objects.first == id_a); + EXPECT_TRUE(ctx.bmain->objects.last == id_b); + test_lib_id_main_sort_check_order({id_a, id_c, id_b}); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + /* Rename to same root name. */ + result = change_name(ctx.bmain, id_c, "OB_A", IDNewNameMode::RenameExistingSameRoot); + + EXPECT_EQ(result.action, IDNewNameResult::Action::RENAMED_COLLISION_FORCED); + EXPECT_EQ(result.other_id, id_a); + EXPECT_STREQ(id_c->name + 2, "OB_A"); + EXPECT_STREQ(id_a->name + 2, "OB_A.001"); + EXPECT_TRUE(ctx.bmain->objects.first == id_c); + EXPECT_TRUE(ctx.bmain->objects.last == id_b); + test_lib_id_main_sort_check_order({id_c, id_a, id_b}); + + EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); + + EXPECT_EQ(ctx.bmain->name_map_global, nullptr); +} + TEST(lib_id_main_unique_name, linked_ids_1) { LibIDMainSortTestContext ctx; @@ -175,7 +280,7 @@ TEST(lib_id_main_unique_name, linked_ids_1) change_lib(ctx.bmain, id_b, lib_a); id_sort_by_name(&ctx.bmain->objects, id_b, nullptr); - change_name(ctx.bmain, id_b, "OB_A"); + change_name(ctx.bmain, id_b, "OB_A", IDNewNameMode::RenameExistingNever); EXPECT_STREQ(id_b->name + 2, "OB_A.001"); EXPECT_STREQ(id_a->name + 2, "OB_A"); EXPECT_TRUE(ctx.bmain->objects.first == id_c); @@ -186,7 +291,7 @@ TEST(lib_id_main_unique_name, linked_ids_1) change_lib(ctx.bmain, id_b, lib_b); id_sort_by_name(&ctx.bmain->objects, id_b, nullptr); - change_name(ctx.bmain, id_b, "OB_A"); + change_name(ctx.bmain, id_b, "OB_A", IDNewNameMode::RenameExistingNever); EXPECT_STREQ(id_b->name + 2, "OB_A"); EXPECT_STREQ(id_a->name + 2, "OB_A"); EXPECT_TRUE(ctx.bmain->objects.first == id_c); @@ -252,7 +357,7 @@ TEST(lib_id_main_global_unique_name, linked_ids_1) EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); - change_name(ctx.bmain, id_b, "OB_C"); + change_name(ctx.bmain, id_b, "OB_C", IDNewNameMode::RenameExistingNever); EXPECT_STREQ(id_b->name + 2, "OB_C"); EXPECT_STREQ(id_a->name + 2, "OB_C.002"); EXPECT_STREQ(id_c->name + 2, "OB_C"); @@ -519,13 +624,13 @@ TEST(lib_id_main_unique_name, renames_with_duplicates) EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); - BKE_libblock_rename(ctx.bmain, id_a, "Foo.002"); + BKE_libblock_rename(*ctx.bmain, *id_a, "Foo.002"); EXPECT_STREQ(id_a->name + 2, "Foo.002"); - BKE_libblock_rename(ctx.bmain, id_b, "Bar"); + BKE_libblock_rename(*ctx.bmain, *id_b, "Bar"); EXPECT_STREQ(id_b->name + 2, "Bar.001"); - BKE_libblock_rename(ctx.bmain, id_c, "Foo"); + BKE_libblock_rename(*ctx.bmain, *id_c, "Foo"); EXPECT_STREQ(id_c->name + 2, "Foo"); - BKE_libblock_rename(ctx.bmain, id_b, "Bar"); + BKE_libblock_rename(*ctx.bmain, *id_b, "Bar"); EXPECT_STREQ(id_b->name + 2, "Bar"); EXPECT_TRUE(BKE_main_namemap_validate(ctx.bmain)); diff --git a/source/blender/blenkernel/intern/main_namemap.cc b/source/blender/blenkernel/intern/main_namemap.cc index 07a632c8811..b932e5514ee 100644 --- a/source/blender/blenkernel/intern/main_namemap.cc +++ b/source/blender/blenkernel/intern/main_namemap.cc @@ -494,8 +494,12 @@ static bool main_namemap_validate_and_fix(Main *bmain, const bool do_fix) * to the validated set if it can now be added to `id_names_libs`, and will prevent * further checking (which would fail again, since the new ID name/lib key has already * been added to `id_names_libs`). */ - BKE_id_new_name_validate( - bmain, which_libbase(bmain, GS(id_iter->name)), id_iter, nullptr, true); + BKE_id_new_name_validate(*bmain, + *which_libbase(bmain, GS(id_iter->name)), + *id_iter, + nullptr, + IDNewNameMode::RenameExistingNever, + true); STRNCPY(key.name, id_iter->name); if (!id_names_libs.add(key)) { /* This is a serious error, very likely a bug, keep it as CLOG_ERROR even when doing diff --git a/source/blender/blenloader/intern/versioning_250.cc b/source/blender/blenloader/intern/versioning_250.cc index 033ac94473b..5806beedb3b 100644 --- a/source/blender/blenloader/intern/versioning_250.cc +++ b/source/blender/blenloader/intern/versioning_250.cc @@ -440,7 +440,7 @@ static void versions_gpencil_add_main(Main *bmain, ListBase *lb, ID *id, const c id->flag = ID_FLAG_FAKEUSER; *((short *)id->name) = ID_GD_LEGACY; - BKE_id_new_name_validate(bmain, lb, id, name, false); + BKE_id_new_name_validate(*bmain, *lb, *id, name, IDNewNameMode::RenameExistingNever, false); /* alphabetic insertion: is in BKE_id_new_name_validate */ if ((id->tag & ID_TAG_TEMP_MAIN) == 0) { diff --git a/source/blender/blenloader/intern/versioning_common.cc b/source/blender/blenloader/intern/versioning_common.cc index d516930cf2e..ab0add79842 100644 --- a/source/blender/blenloader/intern/versioning_common.cc +++ b/source/blender/blenloader/intern/versioning_common.cc @@ -108,7 +108,7 @@ ID *do_versions_rename_id(Main *bmain, } } if (id != nullptr) { - BKE_libblock_rename(bmain, id, name_dst); + BKE_libblock_rename(*bmain, *id, name_dst); } return id; } diff --git a/source/blender/blenloader/intern/versioning_defaults.cc b/source/blender/blenloader/intern/versioning_defaults.cc index d7e18cad42f..df1255bebfe 100644 --- a/source/blender/blenloader/intern/versioning_defaults.cc +++ b/source/blender/blenloader/intern/versioning_defaults.cc @@ -545,7 +545,7 @@ void BLO_update_defaults_startup_blend(Main *bmain, const char *app_template) if (layout->screen) { bScreen *screen = layout->screen; if (!STREQ(screen->id.name + 2, workspace->id.name + 2)) { - BKE_libblock_rename(bmain, &screen->id, workspace->id.name + 2); + BKE_libblock_rename(*bmain, screen->id, workspace->id.name + 2); } } diff --git a/source/blender/editors/CMakeLists.txt b/source/blender/editors/CMakeLists.txt index a0f11b6c7bf..bd2eb054c77 100644 --- a/source/blender/editors/CMakeLists.txt +++ b/source/blender/editors/CMakeLists.txt @@ -16,6 +16,7 @@ if(WITH_BLENDER) add_subdirectory(gizmo_library) add_subdirectory(gpencil_legacy) add_subdirectory(grease_pencil) + add_subdirectory(id_management) add_subdirectory(interface) add_subdirectory(io) add_subdirectory(lattice) diff --git a/source/blender/editors/id_management/CMakeLists.txt b/source/blender/editors/id_management/CMakeLists.txt new file mode 100644 index 00000000000..f45a34ed5e5 --- /dev/null +++ b/source/blender/editors/id_management/CMakeLists.txt @@ -0,0 +1,28 @@ +# SPDX-FileCopyrightText: 2024 Blender Authors +# +# SPDX-License-Identifier: GPL-2.0-or-later + +set(INC + ../include + ../../blenkernel + ../../makesrna + ../../windowmanager +) + +set(INC_SYS +) + +set(SRC + ed_id_management.cc +) + +set(LIB + bf_blenkernel + bf_windowmanager + PRIVATE bf::blenlib + PRIVATE bf::dna + PRIVATE bf::intern::clog + PRIVATE bf::intern::guardedalloc +) + +blender_add_lib(bf_editor_id_management "${SRC}" "${INC}" "${INC_SYS}" "${LIB}") diff --git a/source/blender/editors/id_management/ed_id_management.cc b/source/blender/editors/id_management/ed_id_management.cc new file mode 100644 index 00000000000..10c8a608553 --- /dev/null +++ b/source/blender/editors/id_management/ed_id_management.cc @@ -0,0 +1,71 @@ +/* SPDX-FileCopyrightText: 2004 Blender Authors + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup edundo + */ + +#include "MEM_guardedalloc.h" + +#include "CLG_log.h" + +#include "DNA_ID.h" + +#include "BKE_lib_id.hh" +#include "BKE_main.hh" + +#include "WM_api.hh" + +#include "ED_id_management.hh" + +/** We only need this locally. */ +static CLG_LogRef LOG = {"ed.id_management"}; + +bool ED_id_rename(Main &bmain, ID &id, blender::StringRefNull name) +{ + BLI_assert(ID_IS_EDITABLE(&id)); + + const IDNewNameResult result = BKE_id_rename( + bmain, id, name, IDNewNameMode::RenameExistingSameRoot); + + switch (result.action) { + case IDNewNameResult::Action::UNCHANGED: + CLOG_INFO(&LOG, 4, "ID '%s' not renamed, already using the requested name", id.name + 2); + return false; + case IDNewNameResult::Action::UNCHANGED_COLLISION: + CLOG_INFO(&LOG, + 4, + "ID '%s' not renamed, requested new name '%s' would collide with an existing one", + id.name + 2, + name.c_str()); + return false; + case IDNewNameResult::Action::RENAMED_NO_COLLISION: + CLOG_INFO(&LOG, 4, "ID '%s' renamed without any collision", id.name + 2); + return true; + case IDNewNameResult::Action::RENAMED_COLLISION_ADJUSTED: + CLOG_INFO(&LOG, + 4, + "ID '%s' renamed with adjustment from requested name '%s', to avoid name " + "collision with another ID", + id.name + 2, + name.c_str()); + WM_reportf(RPT_INFO, + "Data-block renamed to '%s', try again to force renaming it to '%s'", + id.name + 2, + name.c_str()); + return true; + case IDNewNameResult::Action::RENAMED_COLLISION_FORCED: + CLOG_INFO( + &LOG, + 4, + "ID '%s' forcefully renamed, another ID had to also be renamed to avoid name collision", + id.name + 2); + WM_reportf(RPT_INFO, + "Name in use. The other data-block was renamed to ‘%s’", + result.other_id->name + 2); + return true; + } + + return false; +} diff --git a/source/blender/editors/include/ED_id_management.hh b/source/blender/editors/include/ED_id_management.hh new file mode 100644 index 00000000000..ac328861443 --- /dev/null +++ b/source/blender/editors/include/ED_id_management.hh @@ -0,0 +1,18 @@ +/* SPDX-FileCopyrightText: 2024 Blender Authors + * + * SPDX-License-Identifier: GPL-2.0-or-later */ + +/** \file + * \ingroup editors + */ + +#pragma once + +#include "BLI_compiler_attrs.h" +#include "BLI_string_ref.hh" + +struct ID; +struct Main; + +/** Handle complex user-facing ID renaming behavior, including user feedback (reporting). */ +bool ED_id_rename(Main &bmain, ID &id, blender::StringRefNull name); diff --git a/source/blender/editors/include/UI_interface_c.hh b/source/blender/editors/include/UI_interface_c.hh index 7f2d3d2bf95..7f5363bdff2 100644 --- a/source/blender/editors/include/UI_interface_c.hh +++ b/source/blender/editors/include/UI_interface_c.hh @@ -1790,6 +1790,8 @@ void UI_block_funcN_set(uiBlock *block, uiButArgNCopy func_argN_copy_fn = MEM_dupallocN); void UI_but_func_rename_set(uiBut *but, uiButHandleRenameFunc func, void *arg1); +void UI_but_func_rename_full_set(uiBut *but, + std::function rename_full_func); void UI_but_func_set(uiBut *but, uiButHandleFunc func, void *arg1, void *arg2); void UI_but_funcN_set(uiBut *but, uiButHandleNFunc funcN, diff --git a/source/blender/editors/interface/CMakeLists.txt b/source/blender/editors/interface/CMakeLists.txt index b63d733c166..6275d7ba620 100644 --- a/source/blender/editors/interface/CMakeLists.txt +++ b/source/blender/editors/interface/CMakeLists.txt @@ -106,6 +106,7 @@ set(LIB PRIVATE bf::depsgraph PRIVATE bf::dna bf_editor_datafiles + bf_editor_id_management PRIVATE bf::extern::fmtlib PRIVATE bf::intern::guardedalloc PRIVATE bf::animrig diff --git a/source/blender/editors/interface/interface.cc b/source/blender/editors/interface/interface.cc index b208c80331c..1ed18f38aef 100644 --- a/source/blender/editors/interface/interface.cc +++ b/source/blender/editors/interface/interface.cc @@ -3082,8 +3082,14 @@ bool ui_but_string_set(bContext *C, uiBut *but, const char *str) const PropertyType type = RNA_property_type(but->rnaprop); if (type == PROP_STRING) { - /* RNA string */ - RNA_property_string_set(&but->rnapoin, but->rnaprop, str); + /* RNA string, only set it if full rename callback is not defined, otherwise just store the + * user-defined new name to call the callback later. */ + if (but->rename_full_func) { + but->rename_full_new = str; + } + else { + RNA_property_string_set(&but->rnapoin, but->rnaprop, str); + } return true; } @@ -5945,6 +5951,12 @@ void UI_but_func_rename_set(uiBut *but, uiButHandleRenameFunc func, void *arg1) but->rename_arg1 = arg1; } +void UI_but_func_rename_full_set(uiBut *but, + std::function rename_full_func) +{ + but->rename_full_func = rename_full_func; +} + void UI_but_func_drawextra_set( uiBlock *block, void (*func)(const bContext *C, void *idv, void *arg1, void *arg2, rcti *rect), diff --git a/source/blender/editors/interface/interface_handlers.cc b/source/blender/editors/interface/interface_handlers.cc index c998f315a78..79b21ebe5a7 100644 --- a/source/blender/editors/interface/interface_handlers.cc +++ b/source/blender/editors/interface/interface_handlers.cc @@ -501,6 +501,9 @@ struct uiAfterFunc { void *rename_arg1; void *rename_orig; + std::function rename_full_func = nullptr; + std::string rename_full_new; + uiBlockHandleFunc handle_func; void *handle_func_arg; int retval; @@ -831,8 +834,9 @@ static void popup_check(bContext *C, wmOperator *op) */ static bool ui_afterfunc_check(const uiBlock *block, const uiBut *but) { - return (but->func || but->apply_func || but->funcN || but->rename_func || but->optype || - but->rnaprop || block->handle_func || (block->handle && block->handle->popup_op)); + return (but->func || but->apply_func || but->funcN || but->rename_func || + but->rename_full_func || but->optype || but->rnaprop || block->handle_func || + (block->handle && block->handle->popup_op)); } /** @@ -871,6 +875,10 @@ static void ui_apply_but_func(bContext *C, uiBut *but) after->rename_arg1 = but->rename_arg1; after->rename_orig = but->rename_orig; /* needs free! */ + after->rename_full_func = but->rename_full_func; + after->rename_full_new = std::move(but->rename_full_new); + but->rename_full_new = ""; + after->handle_func = block->handle_func; after->handle_func_arg = block->handle_func_arg; after->retval = but->retval; @@ -1069,6 +1077,11 @@ static void ui_apply_but_funcs_after(bContext *C) CTX_store_set(C, nullptr); } + if (after.rename_full_func) { + BLI_assert(!after.rename_func); + after.rename_full_func(after.rename_full_new); + } + if (after.func) { after.func(C, after.func_arg1, after.func_arg2); } diff --git a/source/blender/editors/interface/interface_intern.hh b/source/blender/editors/interface/interface_intern.hh index c71889f0897..6428d8eaa10 100644 --- a/source/blender/editors/interface/interface_intern.hh +++ b/source/blender/editors/interface/interface_intern.hh @@ -227,6 +227,11 @@ struct uiBut { void *rename_arg1 = nullptr; void *rename_orig = nullptr; + /* When defined, and the button edits a string RNA property, the new name is _not_ set at all, + * instead this function is called with the new name. */ + std::function rename_full_func = nullptr; + std::string rename_full_new = ""; + /** Run an action when holding the button down. */ uiButHandleHoldFunc hold_func = nullptr; void *hold_argN = nullptr; diff --git a/source/blender/editors/interface/interface_layout.cc b/source/blender/editors/interface/interface_layout.cc index adee24fe2d1..5045a5b9152 100644 --- a/source/blender/editors/interface/interface_layout.cc +++ b/source/blender/editors/interface/interface_layout.cc @@ -33,6 +33,7 @@ #include "BKE_context.hh" #include "BKE_global.hh" #include "BKE_idprop.hh" +#include "BKE_lib_id.hh" #include "BKE_screen.hh" #include "RNA_access.hh" @@ -40,6 +41,8 @@ #include "UI_interface.hh" +#include "ED_id_management.hh" + #include "WM_api.hh" #include "WM_types.hh" @@ -2112,6 +2115,8 @@ void uiItemFullR(uiLayout *layout, const PropertyType type = RNA_property_type(prop); const bool is_array = RNA_property_array_check(prop); const int len = (is_array) ? RNA_property_array_length(ptr, prop) : 0; + const bool is_id_name_prop = (ptr->owner_id == ptr->data && type == PROP_STRING && + prop == RNA_struct_name_property(ptr->type)); const bool icon_only = (flag & UI_ITEM_R_ICON_ONLY) != 0; @@ -2395,6 +2400,16 @@ void uiItemFullR(uiLayout *layout, /* property with separate label */ else if (ELEM(type, PROP_ENUM, PROP_STRING, PROP_POINTER)) { but = ui_item_with_label(layout, block, name, icon, ptr, prop, index, 0, 0, w, h, flag); + + if (is_id_name_prop) { + Main *bmain = CTX_data_main(static_cast(block->evil_C)); + ID *id = ptr->owner_id; + UI_but_func_rename_full_set(but, [bmain, id](std::string &new_name) { + ED_id_rename(*bmain, *id, new_name); + WM_main_add_notifier(NC_ID | NA_RENAME, nullptr); + }); + } + bool results_are_suggestions = false; if (type == PROP_STRING) { const eStringPropertySearchFlag search_flag = RNA_property_string_search_flag(prop); diff --git a/source/blender/editors/interface/templates/interface_templates.cc b/source/blender/editors/interface/templates/interface_templates.cc index ba02b27eb2d..68eef6884db 100644 --- a/source/blender/editors/interface/templates/interface_templates.cc +++ b/source/blender/editors/interface/templates/interface_templates.cc @@ -77,6 +77,7 @@ #include "DEG_depsgraph_query.hh" #include "ED_fileselect.hh" +#include "ED_id_management.hh" #include "ED_info.hh" #include "ED_object.hh" #include "ED_render.hh" @@ -977,6 +978,10 @@ static void template_id_cb(bContext *C, void *arg_litem, void *arg_event) * still assign the callback so the button can be identified as part of an ID-template. See * #UI_context_active_but_prop_get_templateID(). */ break; + case UI_ID_RENAME: + /* Only for the undo push. */ + undo_push_label = "Rename Data-Block"; + break; case UI_ID_BROWSE: case UI_ID_PIN: RNA_warning("warning, id event %d shouldn't come here", event); @@ -1393,6 +1398,15 @@ static void template_ID(const bContext *C, 0, 0, RNA_struct_ui_description(type)); + /* Handle undo through the #template_id_cb set below. Default undo handling from the button + * code (see #ui_apply_but_undo) would not work here, as the new name is not yet applied to the + * ID. */ + UI_but_flag_disable(but, UI_BUT_UNDO); + Main *bmain = CTX_data_main(C); + UI_but_func_rename_full_set(but, [bmain, id](std::string &new_name) { + ED_id_rename(*bmain, *id, new_name); + WM_main_add_notifier(NC_ID | NA_RENAME, nullptr); + }); UI_but_funcN_set(but, template_id_cb, MEM_new(__func__, template_ui), diff --git a/source/blender/editors/screen/workspace_edit.cc b/source/blender/editors/screen/workspace_edit.cc index e4741e6e260..bb0b61b8635 100644 --- a/source/blender/editors/screen/workspace_edit.cc +++ b/source/blender/editors/screen/workspace_edit.cc @@ -361,7 +361,7 @@ static int workspace_append_activate_exec(bContext *C, wmOperator *op) if (BLT_translate_new_dataname()) { /* Translate workspace name */ BKE_libblock_rename( - bmain, &appended_workspace->id, CTX_DATA_(BLT_I18NCONTEXT_ID_WORKSPACE, idname)); + *bmain, appended_workspace->id, CTX_DATA_(BLT_I18NCONTEXT_ID_WORKSPACE, idname)); } /* Set defaults. */ diff --git a/source/blender/editors/space_outliner/CMakeLists.txt b/source/blender/editors/space_outliner/CMakeLists.txt index 7fdde6280aa..497f6f05274 100644 --- a/source/blender/editors/space_outliner/CMakeLists.txt +++ b/source/blender/editors/space_outliner/CMakeLists.txt @@ -139,6 +139,7 @@ set(LIB PRIVATE bf::blenlib PRIVATE bf::depsgraph PRIVATE bf::dna + bf_editor_id_management bf_editor_undo PRIVATE bf::intern::clog PRIVATE bf::intern::guardedalloc diff --git a/source/blender/editors/space_outliner/outliner_draw.cc b/source/blender/editors/space_outliner/outliner_draw.cc index aa5d5499a54..b0cfb215fb8 100644 --- a/source/blender/editors/space_outliner/outliner_draw.cc +++ b/source/blender/editors/space_outliner/outliner_draw.cc @@ -51,8 +51,10 @@ #include "ED_armature.hh" #include "ED_fileselect.hh" +#include "ED_id_management.hh" #include "ED_outliner.hh" #include "ED_screen.hh" +#include "ED_undo.hh" #include "WM_api.hh" #include "WM_message.hh" @@ -679,22 +681,26 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname) BLI_mempool *ts = space_outliner->treestore; TreeStoreElem *tselem = static_cast(tsep); + const char *undo_str = nullptr; + /* Unfortunately at this point, the name of the ID has already been set to its new value. Revert * it to its old name, to be able to use the generic 'rename' function for IDs. * * NOTE: While utterly inelegant, performances are not really a concern here, so this is an * acceptable solution for now. */ - auto id_rename_helper = [bmain, tselem, oldname] { + auto id_rename_helper = [bmain, tselem, oldname]() -> bool { std::string new_name = tselem->id->name + 2; BLI_strncpy(tselem->id->name + 2, oldname, sizeof(tselem->id->name) - 2); - BKE_libblock_rename(bmain, tselem->id, new_name.c_str()); + return ED_id_rename(*bmain, *tselem->id, new_name); }; if (ts && tselem) { TreeElement *te = outliner_find_tree_element(&space_outliner->tree, tselem); if (ELEM(tselem->type, TSE_SOME_ID, TSE_LINKED_NODE_TREE)) { - id_rename_helper(); + if (id_rename_helper()) { + undo_str = "Rename Data-Block"; + } WM_msg_publish_rna_prop(mbus, tselem->id, tselem->id, ID, name); @@ -711,13 +717,6 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname) case ID_SCE: WM_event_add_notifier(C, NC_SCENE, nullptr); break; - case ID_OB: { - Object *ob = (Object *)tselem->id; - if (ob->type == OB_MBALL) { - DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY); - } - break; - } default: break; } @@ -757,17 +756,21 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname) BKE_object_defgroup_unique_name(vg, ob); WM_msg_publish_rna_prop(mbus, &ob->id, vg, VertexGroup, name); DEG_id_tag_update(tselem->id, ID_RECALC_SYNC_TO_EVAL); + undo_str = "Rename Vertex Group"; break; } case TSE_NLA_ACTION: { /* The #tselem->id is a #bAction. */ - id_rename_helper(); + if (id_rename_helper()) { + undo_str = "Rename Data-Block"; + } WM_msg_publish_rna_prop(mbus, tselem->id, tselem->id, ID, name); DEG_id_tag_update(tselem->id, ID_RECALC_SYNC_TO_EVAL); break; } case TSE_NLA_TRACK: { WM_event_add_notifier(C, NC_ANIMATION | ND_ANIMCHAN | NA_RENAME, nullptr); + undo_str = "Rename NLA Track"; break; } case TSE_EBONE: { @@ -783,6 +786,7 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname) WM_msg_publish_rna_prop(mbus, &arm->id, ebone, EditBone, name); WM_event_add_notifier(C, NC_OBJECT | ND_POSE, nullptr); DEG_id_tag_update(tselem->id, ID_RECALC_SYNC_TO_EVAL); + undo_str = "Rename Edit Bone"; } break; } @@ -805,6 +809,7 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname) WM_msg_publish_rna_prop(mbus, &arm->id, bone, Bone, name); WM_event_add_notifier(C, NC_OBJECT | ND_POSE, nullptr); DEG_id_tag_update(tselem->id, ID_RECALC_SYNC_TO_EVAL); + undo_str = "Rename Bone"; break; } case TSE_POSE_CHANNEL: { @@ -829,6 +834,7 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname) WM_event_add_notifier(C, NC_OBJECT | ND_POSE, nullptr); DEG_id_tag_update(tselem->id, ID_RECALC_SYNC_TO_EVAL); DEG_id_tag_update(&arm->id, ID_RECALC_SYNC_TO_EVAL); + undo_str = "Rename Pose Bone"; break; } case TSE_GP_LAYER: { @@ -846,6 +852,7 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname) DEG_id_tag_update(&gpd->id, ID_RECALC_GEOMETRY); WM_event_add_notifier(C, NC_GPENCIL | ND_DATA | NA_SELECTED, gpd); DEG_id_tag_update(tselem->id, ID_RECALC_SYNC_TO_EVAL); + undo_str = "Rename Grease Pencil Layer"; break; } case TSE_GREASE_PENCIL_NODE: { @@ -861,6 +868,7 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname) grease_pencil.rename_node(*bmain, node, new_name); DEG_id_tag_update(&grease_pencil.id, ID_RECALC_SYNC_TO_EVAL); WM_event_add_notifier(C, NC_ID | NA_RENAME, nullptr); + undo_str = "Rename Grease Pencil Drawing"; break; } case TSE_R_LAYER: { @@ -877,11 +885,14 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname) WM_msg_publish_rna_prop(mbus, &scene->id, view_layer, ViewLayer, name); WM_event_add_notifier(C, NC_ID | NA_RENAME, nullptr); DEG_id_tag_update(tselem->id, ID_RECALC_SYNC_TO_EVAL); + undo_str = "Rename View Layer"; break; } case TSE_LAYER_COLLECTION: { /* The #tselem->id is a #Collection, not a #LayerCollection */ - id_rename_helper(); + if (id_rename_helper()) { + undo_str = "Rename Data-Block"; + } WM_msg_publish_rna_prop(mbus, tselem->id, tselem->id, ID, name); WM_event_add_notifier(C, NC_ID | NA_RENAME, nullptr); DEG_id_tag_update(tselem->id, ID_RECALC_SYNC_TO_EVAL); @@ -896,12 +907,17 @@ static void namebutton_fn(bContext *C, void *tsep, char *oldname) WM_msg_publish_rna_prop(mbus, &arm->id, bcoll, BoneCollection, name); WM_event_add_notifier(C, NC_OBJECT | ND_BONE_COLLECTION, arm); DEG_id_tag_update(&arm->id, ID_RECALC_SYNC_TO_EVAL); + undo_str = "Rename Bone Collection"; break; } } } tselem->flag &= ~TSE_TEXTBUT; } + + if (undo_str) { + ED_undo_push(C, undo_str); + } } struct RestrictProperties { @@ -2139,6 +2155,10 @@ static void outliner_buttons(const bContext *C, 1.0, float(len), ""); + /* Handle undo through the #template_id_cb set below. Default undo handling from the button + * code (see #ui_apply_but_undo) would not work here, as the new name is not yet applied to the + * ID. */ + UI_but_flag_disable(bt, UI_BUT_UNDO); UI_but_func_rename_set(bt, namebutton_fn, tselem); /* Returns false if button got removed. */ diff --git a/source/blender/editors/space_outliner/outliner_edit.cc b/source/blender/editors/space_outliner/outliner_edit.cc index 55bb13abc93..58ce59eca97 100644 --- a/source/blender/editors/space_outliner/outliner_edit.cc +++ b/source/blender/editors/space_outliner/outliner_edit.cc @@ -449,8 +449,9 @@ void OUTLINER_OT_item_rename(wmOperatorType *ot) ot->poll = ED_operator_region_outliner_active; - /* Flags. */ - ot->flag = OPTYPE_REGISTER | OPTYPE_UNDO; + /* Flags. No undo, since this operator only activate the name editing text field in the Outliner, + * but does not actually change anything. */ + ot->flag = OPTYPE_REGISTER; prop = RNA_def_boolean(ot->srna, "use_active", diff --git a/source/blender/editors/util/CMakeLists.txt b/source/blender/editors/util/CMakeLists.txt index a96b167395b..44afead2f86 100644 --- a/source/blender/editors/util/CMakeLists.txt +++ b/source/blender/editors/util/CMakeLists.txt @@ -51,6 +51,7 @@ set(SRC ../include/ED_gizmo_utils.hh ../include/ED_gpencil_legacy.hh ../include/ED_grease_pencil.hh + ../include/ED_id_management.hh ../include/ED_image.hh ../include/ED_info.hh ../include/ED_keyframes_draw.hh diff --git a/source/blender/io/collada/DocumentImporter.cpp b/source/blender/io/collada/DocumentImporter.cpp index 70f8785ce04..01ee8379226 100644 --- a/source/blender/io/collada/DocumentImporter.cpp +++ b/source/blender/io/collada/DocumentImporter.cpp @@ -638,7 +638,7 @@ std::vector *DocumentImporter::write_node(COLLADAFW::Node *node, for (Object *ob : *objects_done) { std::string nodename = node->getName().empty() ? node->getOriginalId() : node->getName(); - BKE_libblock_rename(bmain, &ob->id, (char *)nodename.c_str()); + BKE_libblock_rename(*bmain, ob->id, (char *)nodename.c_str()); object_map.insert(std::pair(node->getUniqueId(), ob)); node_map[node->getUniqueId()] = node; diff --git a/source/blender/makesrna/intern/rna_ID.cc b/source/blender/makesrna/intern/rna_ID.cc index f12ecb378ce..1bc70dc8f67 100644 --- a/source/blender/makesrna/intern/rna_ID.cc +++ b/source/blender/makesrna/intern/rna_ID.cc @@ -282,20 +282,17 @@ int rna_ID_name_length(PointerRNA *ptr) return strlen(id->name + 2); } +static int rna_ID_rename(ID *self, Main *bmain, const char *new_name, const int mode) +{ + IDNewNameResult result = BKE_id_rename(*bmain, *self, new_name, IDNewNameMode(mode)); + return int(result.action); +} + void rna_ID_name_set(PointerRNA *ptr, const char *value) { ID *id = (ID *)ptr->data; - BLI_assert(BKE_id_is_in_global_main(id)); - BLI_assert(ID_IS_EDITABLE(id)); - BKE_libblock_rename(G_MAIN, id, value); - - if (GS(id->name) == ID_OB) { - Object *ob = (Object *)id; - if (ob->type == OB_MBALL) { - DEG_id_tag_update(&ob->id, ID_RECALC_GEOMETRY); - } - } + rna_ID_rename(id, G_MAIN, value, int(IDNewNameMode::RenameExistingNever)); } static int rna_ID_name_editable(const PointerRNA *ptr, const char **r_info) @@ -2191,6 +2188,58 @@ static void rna_def_ID(BlenderRNA *brna) {0, nullptr, 0, nullptr, nullptr}, }; + static const EnumPropertyItem rename_mode_items[] = { + {int(IDNewNameMode::RenameExistingNever), + "NEVER", + 0, + "Never Rename", + "Never rename an exisitng ID whose name would conflict, the currently renamed ID will get " + "a numeric suffix appended to its new name"}, + {int(IDNewNameMode::RenameExistingAlways), + "ALWAYS", + 0, + "Always Rename", + "Always rename an exisitng ID whose name would conflict, ensuring that the currently " + "renamed ID will get requested name"}, + {int(IDNewNameMode::RenameExistingSameRoot), + "SAME_ROOT", + 0, + "Rename If Same Root", + "Only rename an exisitng ID whose name would conflict if its name root (everything besides " + "the numerical suffix) is the same as the existing name of the currently renamed ID"}, + {0, nullptr, 0, nullptr, nullptr}, + }; + + static const EnumPropertyItem rename_result_items[] = { + {int(IDNewNameResult::Action::UNCHANGED), + "UNCHANGED", + 0, + "Unchanged", + "The ID was not renamed, e.g. because it is already named as requested"}, + {int(IDNewNameResult::Action::UNCHANGED_COLLISION), + "UNCHANGED_COLLISION", + 0, + "Unchanged Due to Collision", + "The ID was not renamed, because requested name would have collided with another existing " + "ID's name, and the automatically adjusted name was the same as the current ID's name"}, + {int(IDNewNameResult::Action::RENAMED_NO_COLLISION), + "RENAMED_NO_COLLISION", + 0, + "Renamed Without Collision", + "The ID was renamed as requested, without creating any name collision"}, + {int(IDNewNameResult::Action::RENAMED_COLLISION_ADJUSTED), + "RENAMED_COLLISION_ADJUSTED", + 0, + "Renamed With Collision", + "The ID was renamed with adjustement of the requested name, to avoid a name collision"}, + {int(IDNewNameResult::Action::RENAMED_COLLISION_FORCED), + "RENAMED_COLLISION_FORCED", + 0, + "Renamed Enforced With Collision", + "The ID was renamed as requested, also renaming another ID to avoid a name collision"}, + {0, nullptr, 0, nullptr, nullptr}, + }; + srna = RNA_def_struct(brna, "ID", nullptr); RNA_def_struct_ui_text( srna, @@ -2358,6 +2407,32 @@ static void rna_def_ID(BlenderRNA *brna) RNA_def_property_pointer_funcs(prop, "rna_IDPreview_get", nullptr, nullptr, nullptr); /* functions */ + func = RNA_def_function(srna, "rename", "rna_ID_rename"); + RNA_def_function_ui_description( + func, "More refined handling in case the new name collides with another ID's name"); + RNA_def_function_flag(func, FUNC_USE_MAIN); + parm = RNA_def_string(func, + "name", + nullptr, + MAX_NAME, + "", + "New name to rename the ID to, if empty will re-use the current ID name"); + RNA_def_parameter_flags(parm, PropertyFlag(0), PARM_REQUIRED); + parm = RNA_def_enum(func, + "mode", + rename_mode_items, + int(IDNewNameMode::RenameExistingNever), + "", + "How to handle name collision, in case the requested new name is already " + "used by another ID of the same type"); + parm = RNA_def_enum(func, + "id_rename_result", + rename_result_items, + int(IDNewNameResult::Action::UNCHANGED), + "", + "How did the renaming of the data-block went on"); + RNA_def_function_return(func, parm); + func = RNA_def_function(srna, "evaluated_get", "rna_ID_evaluated_get"); RNA_def_function_ui_description( func, diff --git a/source/blender/windowmanager/intern/wm_files.cc b/source/blender/windowmanager/intern/wm_files.cc index a3fbdbeb23d..a4763865f3f 100644 --- a/source/blender/windowmanager/intern/wm_files.cc +++ b/source/blender/windowmanager/intern/wm_files.cc @@ -790,7 +790,7 @@ static void wm_file_read_post(bContext *C, /* Translate workspace names. */ LISTBASE_FOREACH_MUTABLE (WorkSpace *, workspace, &bmain->workspaces) { BKE_libblock_rename( - bmain, &workspace->id, CTX_DATA_(BLT_I18NCONTEXT_ID_WORKSPACE, workspace->id.name + 2)); + *bmain, workspace->id, CTX_DATA_(BLT_I18NCONTEXT_ID_WORKSPACE, workspace->id.name + 2)); } } diff --git a/tests/python/bl_id_management.py b/tests/python/bl_id_management.py index 4f57dcb3796..356ae55e232 100644 --- a/tests/python/bl_id_management.py +++ b/tests/python/bl_id_management.py @@ -205,8 +205,57 @@ class TestIdRename(TestHelper, unittest.TestCase): data = dt break data.name = name - # This can fail currently, see #71244. - # ~ self.assertEqual(data.name, self.default_name + ".001") + self.assertEqual(data.name, self.default_name + ".001") + self.ensure_proper_order() + + def test_rename_api(self): + self.clear_container() + self.add_items_with_randomized_names(100) + self.ensure_proper_order() + + name = self.default_name + data = self.data_container[0] + data.name = name + data_other = None + self.assertEqual(data.name, name) + for dt in self.data_container: + if dt is not data: + data_other = dt + break + name_other = "Other_" + name + data_other.name = name_other + + data_other.rename(name, mode='NEVER') + self.assertEqual(data.name, name) + self.assertEqual(data_other.name, name + ".001") + self.ensure_proper_order() + data_other.rename(name, mode='NEVER') + self.assertEqual(data.name, name) + self.assertEqual(data_other.name, name + ".001") + self.ensure_proper_order() + + data_other.name = name_other + data.name = name + + data_other.rename(name, mode='ALWAYS') + self.assertEqual(data.name, name + ".001") + self.assertEqual(data_other.name, name) + self.ensure_proper_order() + data.rename(name, mode='ALWAYS') + self.assertEqual(data.name, name) + self.assertEqual(data_other.name, name + ".001") + self.ensure_proper_order() + + data_other.name = name_other + data.name = name + + data_other.rename(name, mode='SAME_ROOT') + self.assertEqual(data.name, name) + self.assertEqual(data_other.name, name + ".001") + self.ensure_proper_order() + data_other.rename(name, mode='SAME_ROOT') + self.assertEqual(data.name, name + ".001") + self.assertEqual(data_other.name, name) self.ensure_proper_order()