Fix #125426: Update paths in animation when renaming IDProperty #125474
No reviewers
Labels
No Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset System
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Viewport & EEVEE
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Interest: X11
Legacy
Asset Browser Project
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Module
Viewport & EEVEE
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Severity
High
Severity
Low
Severity
Normal
Severity
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: blender/blender#125474
Loading…
Reference in New Issue
Block a user
No description provided.
Delete Branch "ChengduLittleA/blender:fix-125426"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Previously when renaming an IDProperty the existing paths in the
animation/driver system isn't updated, this leads to missing animation
after renaming the property. Now
BKE_animdata_fix_paths_rename_all
will be called so the animation system records the updated name.
It works, thanks!
Just a few more or less cosmetic notes.
@ -1882,3 +1882,3 @@
del item[name_old]
item[name] = new_value
Configure your IDE to do auto-formatting on save ;-)
@ -1888,1 +1888,4 @@
if name_old != name:
item.id_data.fix_paths_rename_all(prefix="",old_name=name_old,
new_name=bpy.utils.escape_identifier(name))
It's not necessary to call
bpy.utils.escape_identifier()
here. EventuallyBKE_animdata_fix_paths_rename()
is called, which already handles escaping of the name when necessary.@ -656,0 +659,4 @@
const char *oldName,
const char *newName)
{
BKE_animdata_fix_paths_rename_all(id, prefix, oldName, newName);
In this case it's better to avoid
BKE_animdata_fix_paths_rename_all
as it has this little snippet:Instead, call
BKE_animdata_fix_paths_rename_all_ex()
and pass it abmain
. You can get thatbmain
from RNA by addingRNA_def_function_flag(func, FUNC_USE_MAIN);
below where you define the RNA function.@ -1553,6 +1562,7 @@ static void rna_def_ID_properties(BlenderRNA *brna)
{
StructRNA *srna;
PropertyRNA *prop;
FunctionRNA *func;
This variable is unused.
@ -2512,6 +2522,12 @@ static void rna_def_ID(BlenderRNA *brna)
func, "preview_image", "ImagePreview", "", "The existing or created preview");
RNA_def_function_return(func, parm);
func = RNA_def_function(srna, "fix_paths_rename_all", "rna_id_animdata_fix_paths_rename_all");
I think it's better to define this function on the
AnimData
struct directly.Hummm we kinda need that
ref_id
when calling rename. (WillFUNC_USE_SELF_ID
do the job / will that be the ID we wanted?)Re:
FUNC_USE_SELF_ID
I'm not 100% sure (RNA is still something I'm learning). But poking around for other examples I found
rna_Driver_new()
andrna_Driver_remove()
inrna_animation.cc
. Both are defined on animdata, but one of them usesFUNC_USE_SELF_ID
and the other doesn't. From looking at those, it looks to me likeFUNC_USE_SELF_ID
does indeed provide access to the appropriate ID. And the doc comment onFUNC_USE_SELF_ID
itself also suggests that that would be the case.So I'd say give it a shot and see if it works.
@ -2514,1 +2524,4 @@
func = RNA_def_function(srna, "fix_paths_rename_all", "rna_id_animdata_fix_paths_rename_all");
RNA_def_string(func, "prefix", 0, 128, "Prefix", "Name prefix");
RNA_def_string(func, "old_name", 0, 128, "Old Name", "Old name");
0
for aconst char *default_value
parameter, usenullptr
instead.128
come from?I have no idea how long these should be 😅 (I guess I'll put
MAX_ID_NAME
here then)It looks to me like
MAX_IDPROP_NAME
is probably the right one to use here.Not sure if I accidentally marked this as resolved or if you did, but it looks like you haven't changed
MAX_ID_NAME
toMAX_IDPROP_NAME
yet, so unresolving.(No big deal or anything, just want to make sure this doesn't get lost.)
Oooh, wait, it looks like the issue is that the version of this function on ID is still left over (after you added it to AnimData). And that's what I was looking at. I guess you forgot to delete it?
The version on AnimData is correct.
Ah I guess it's due to comment reference line being outdated 😅 it's kinda confusing sometimes indeed. But yeah it's changed to
MAX_IDPROP_NAME
already :D👍
It looks like you forgot to delete this (now duplicate) version of
rna_id_animdata_fix_paths_rename_all()
that these comments are still referencing, though?In fact, I think(?) all the changes to rna_ID.cc can now be discarded, since they've been moved into rna_animation_api.cc now.
Weird... Actually I have no idea why it's still there. I'll double check
@ -2515,0 +2526,4 @@
RNA_def_string(func, "prefix", 0, 128, "Prefix", "Name prefix");
RNA_def_string(func, "old_name", 0, 128, "Old Name", "Old name");
RNA_def_string(func, "new_name", 0, 128, "New Name", "New name");
RNA_def_function_ui_description(func, "Rename animation properties");
This description could use some work, "animation properties" is too vague. This will become the Python API documentation, and thus should be understandable without knowing which internal Blender function it calls.
Testing the latest version of this PR, it appears that it no longer fixes the issue. Maybe
FUNC_USE_SELF_ID
doesn't provide the right ID after all? Or maybe it's something else.@nathanvegdahl wait it doesn't? seems to be working here, let me try again 😅 (tested again, seems to be still working?)
Gah! My bad. I had the the new animation system experimental flag enabled, so it was using new-style actions. With that flag disabled, everything works. Sorry about that!
@dr.sybren can we merge this? :D
@dr.sybren is away from the office for a while. The code looks good to me and works in testing, and you've addressed his comments from his initial review. So let's merge.
@nathanvegdahl ah great. Thanks!