Page MenuHome

Fix animating nodes modifier properties doesn't work
ClosedPublic

Authored by Hans Goudey (HooglyBoogly) on Jan 22 2021, 7:12 AM.

Details

Summary

The RNA path used for animating the settings passed to the node tree
is incorrect. Currently it's just settings.property_name, but it's
the path from the ID, not the modifier, so it should be
modifiers[modifier_name].settings.property_name.

However, the "Settings" struct is separated in RNA and DNA, which means
that the callback to get the RNA path does not know about the modifier's
name in order to fill the above path. So we would have to add some reference
to the modifier in the "Settings" struct, which creates a convoluted
layout in the ModifierData struct:

NodesModifierData {
  ModifierData md;
  bNodeTree *node_group;
  settings {
    IDProperty *properties;
    ModifierData *md; /* Pointer back to the modifier to get the name, icky! */
  };
};

(For the record, that fix looks like this: P1904)

Instead, this patch simply removes the "Settings" struct from RNA,
which might look a bit messier from the point of view of the Python API,
but otherwise it's an easy simplification. Note that we can't remove the
settings struct from DNA if we want to be able to read old files.

Diff Detail

Repository
rB Blender
Branch
geometry-nodes-fix-animate-v2 (branched from master)
Build Status
Buildable 12319
Build 12319: arc lint + arc unit

Event Timeline

Hans Goudey (HooglyBoogly) requested review of this revision.Jan 22 2021, 7:12 AM
Hans Goudey (HooglyBoogly) created this revision.
  • Remove commented out code, fix build

This approach might mean we have some limitations in the identifiers we can use for node modifier properties.
It should be fine as long as the properties names coming from the node tree start with input_ though.

Too bad, I liked the .settings["..."] :D
Unfortunately, I don't really have any better idea on how to solve that.
Overall, this patch looks like a good improvement, given the constraints we have.

I wonder why we have to be careful about the id property names here. It shouldn't conflict with other properties in the Python API, because the settings can only be accessed using ["prop_name"], right?

I wonder why we have to be careful about the id property names here. It shouldn't conflict with other properties in the Python API, because the settings can only be accessed using ["prop_name"], right?

Oh, true, that's what I get for commenting after midnight..

This revision is now accepted and ready to land.Jan 25 2021, 5:00 PM