Modifiers: Add idname field to ModifierTypeInfo #110468

Falk David · 2023-07-25T18:23:47+02:00

Falk David commented

2023-07-25 18:23:47 +02:00

Previously the panel type name of a modifier (e.g. "MOD_PT_Smooth") was
created by copying from the ModifierTypeInfos name.
This meant that modifiers with the same default name would use
the same identifier for the panels.

Since different object types (e.g. OB_GREASE_PENCIL and OB_MESH)
might want to use the same default modifier name, this PR introduces
an idname field in the ModifierTypeInfo struct. This is then used to
generate the panel type name.

For compatibility reasons, the idname is the same as the name for now.

Note: Because the name was used previously, this means that some
modifiers have spaces in their panel type name.
E.g. "MOD_PT_Volume to Mesh".

Previously the panel type name of a modifier (e.g. "MOD_PT_Smooth") was created by copying from the `ModifierTypeInfo`s `name`. This meant that modifiers with the same default name would use the same identifier for the panels. Since different object types (e.g. `OB_GREASE_PENCIL` and `OB_MESH`) might want to use the same default modifier name, this PR introduces an `idname` field in the `ModifierTypeInfo` struct. This is then used to generate the panel type name. For compatibility reasons, the `idname` is the same as the `name` for now. Note: Because the `name` was used previously, this means that some modifiers have spaces in their panel type name. E.g. "MOD_PT_Volume to Mesh".

Falk David requested review from Sergey Sharybin 2023-07-25 18:24:20 +02:00

Hans Goudey commented

2023-07-25 18:33:51 +02:00

If I understand correctly, this wasn't a problem before because grease pencil modifiers were totally separate.
I think that context is sort of essential to understand the patch.

Maybe use structName then? The panel identifiers don't really matter, since the panels are created and destroyed at runtime and just need to be unique between modifiers in a single object.

If I understand correctly, this wasn't a problem before because grease pencil modifiers were totally separate. I think that context is sort of essential to understand the patch. Maybe use `structName` then? The panel identifiers don't really matter, since the panels are created and destroyed at runtime and just need to be unique between modifiers in a single object.

Falk David commented

2023-07-25 19:11:37 +02:00

AFAIK using the structName breaks compatibility, because addons could rely on the panel names. The names are specifically duplicated from the name so that the panel are still called the same.
Should have also added that to the description.

AFAIK using the `structName` breaks compatibility, because addons could rely on the panel names. The names are specifically duplicated from the `name` so that the panel are still called the same. Should have also added that to the description.

Hans Goudey commented

2023-07-25 19:17:31 +02:00

I'm fairly sure panel types defined in C/C++ aren't accessible in Python anyway. And then, just finding any unique name when registering the panel should be fine.

Thomas Barlow commented

2023-07-25 22:43:55 +02:00

Addons can set their custom Panel's bl_parent_id to a C/C++ defined Panel's id name to add their custom Panel as a sub-panel of that C/C++ defined Panel. I think it's about the only way to extend C/C++ Panels from addons.

Addons can set their custom Panel's `bl_parent_id` to a C/C++ defined Panel's id name to add their custom Panel as a sub-panel of that C/C++ defined Panel. I think it's about the only way to extend C/C++ Panels from addons.

Sergey Sharybin commented

2023-07-26 09:47:35 +02:00

We should really be careful with the API, and if it is simple to avoid any API change we should follow this path.

The only thing which seems to make more sense is to add an id field, which then can be used to generate panel ID. Having it a panel ID in the type info seems a bit too specific.

We should really be careful with the API, and if it is simple to avoid any API change we should follow this path. The only thing which seems to make more sense is to add an `id` field, which then can be used to generate panel ID. Having it a panel ID in the type info seems a bit too specific.

Falk David commented

2023-07-26 10:36:54 +02:00

I can confirm what @Mysteryem said.

I can confirm what @Mysteryem said. ![image](/attachments/e9e8f3fd-d88d-4424-9be9-f08b0ea5003b) ![image](/attachments/0b06dfe6-d5e5-4a7b-a4f8-b415f164f66d)

image.png

30 KiB

image.png

34 KiB

❤️ 1

Falk David commented

2023-07-26 11:05:08 +02:00

Another weird thing about using name in the panel id that I found is that the name can contain spaces.
Which means that using bl_parent_id = "MOD_PT_Volume to Mesh" in a panel actually works.
So yea this should probably be changed.

Another weird thing about using `name` in the panel id that I found is that the `name` can contain spaces. Which means that using `bl_parent_id = "MOD_PT_Volume to Mesh"` in a panel actually works. So yea this should probably be changed. ![image](/attachments/a5fb3087-cefe-44cd-80e3-1f308bcd1dac)

image.png

122 KiB