UI: Add initial UI support for ID pointers custom properties #110458

Bastien Montagne · 2023-07-25T12:34:56+02:00

Bastien Montagne commented

2023-07-25 12:34:56 +02:00

Customprops to IDs are supported since years through code, but were
never exposed directly in the UI of customporperties.

This commit mainly:

Adds a new DATABLOCK type to UI customprops types.
Exposes the exiting id_type settings to python API.

Customprops to IDs are supported since years through code, but were never exposed directly in the UI of customporperties. This commit mainly: * Adds a new `DATABLOCK` type to UI customprops types. * Exposes the exiting `id_type` settings to python API. ![image](/attachments/4eaa0edf-75c5-4079-9451-86a60d59e62e)

image.png

47 KiB

image.png

40 KiB

❤️ 5

Bastien Montagne referenced this pull request

2023-07-25 12:36:33 +02:00

How to handle preservations of linked IDs when they are effectively unused. #106321

Bastien Montagne commented

2023-07-25 12:37:09 +02:00

@JulianEisel will need your help to finalize and polish this I think.

RedMser commented

2023-07-25 13:52:04 +02:00

The "list all IDs from main" idea could work well for selecting a custom property ID in the drivers editor as well, since it currently requires type selection first:

But it would probably need better UX, like optionally grouping/filtering the IDs by type.

The "list all IDs from main" idea could work well for selecting a custom property ID in the drivers editor as well, since it currently requires type selection first: ![image](/attachments/58b76e46-9748-4c70-9f14-2e31ce64375c) But it would probably need better UX, like optionally grouping/filtering the IDs by type.

image.png

63 KiB

Hans Goudey commented

2023-07-25 14:01:39 +02:00

Nice! This should probably use the IDPropertyUIDataID::id_type value to allow limiting the choice to a specific type. Also I think that type should probably be editable with the custom property edit operator.

Nice! This should probably use the `IDPropertyUIDataID::id_type` value to allow limiting the choice to a specific type. Also I think that type should probably be editable with the custom property edit operator.

Demeter Dzadik commented

2023-07-25 17:11:39 +02:00

Some points:

If drivers were to get a "Pick one from every ID in the entire .blend" drop-down, I don't think that should be default behaviour. Selecting type first, then ID, is much more intuitive imo.
In the same vein, I agree that having a type selector for ID custom properties would also be desirable.
Are ID custom properties allowed to have a default value other than a null pointer? I don't personally need this functionality, but I can see it being both useful and dangerous. (default values are usually quite hidden, and hidden ID pointers are icky).
Since object names aren't unique, as soon as linked IDs get involved, the drop-down list should ideally also display the library path or .blend file name of linked IDs, in order to allow users to uniquely identify them by just looking through the list.

Some points: - If drivers were to get a "Pick one from every ID in the entire .blend" drop-down, I don't think that should be default behaviour. Selecting type first, then ID, is much more intuitive imo. - In the same vein, I agree that having a type selector for ID custom properties would also be desirable. - Are ID custom properties allowed to have a default value other than a null pointer? I don't personally need this functionality, but I can see it being both useful and dangerous. (default values are usually quite hidden, and hidden ID pointers are icky). - Since object names aren't unique, as soon as linked IDs get involved, the drop-down list should _ideally_ also display the library path or .blend file name of linked IDs, in order to allow users to uniquely identify them by just looking through the list.

Bastien Montagne force-pushed ui-for-datablock-customprops from 4bacf23c3b to 5acad9adce

2023-08-10 17:09:57 +02:00

Compare

Bastien Montagne force-pushed ui-for-datablock-customprops from 5acad9adce to 34bd74e51d

2023-08-10 17:32:40 +02:00

Compare

Bastien Montagne commented

2023-08-10 17:37:47 +02:00

Updated the diff, removed the 'all ID types' changes/possibility for now (this would require quite some UI work in the ID template), added handling of the ID property id_type uidata.

Updated the diff, removed the 'all ID types' changes/possibility for now (this would require quite some UI work in the ID template), added handling of the ID property `id_type` uidata.

Bastien Montagne commented

2023-08-10 17:38:02 +02:00

@blender-bot build

Bastien Montagne changed title from ~~WIP: UI: Add initial UI support for ID pointers custom properties.~~ to UI: Add initial UI support for ID pointers custom properties.

2023-08-10 17:38:13 +02:00

Bastien Montagne requested review from Pablo Vazquez 2023-08-10 17:38:51 +02:00

Bastien Montagne requested review from Campbell Barton 2023-08-10 17:38:51 +02:00

Bastien Montagne requested review from Julian Eisel 2023-08-10 17:38:51 +02:00

Hans Goudey changed title from ~~UI: Add initial UI support for ID pointers custom properties.~~ to UI: Add initial UI support for ID pointers custom properties

2023-08-11 02:30:25 +02:00

Bastien Montagne referenced this pull request

2023-08-11 12:39:02 +02:00

Always clear 'fake user' flag from linked data. #111042

Julian Eisel approved these changes 2023-08-11 15:07:46 +02:00

Julian Eisel left a comment

Don't have much to say about this, the little UI code that's here looks fine.

Dalai Felinto commented

2023-08-15 15:29:52 +02:00

I would expect this to use a simple ID "search" instead of a template (without allowing for Fake User or Duplicate/Make Unique). Similar to how we refer to points elsewhere such as the UV Projection modifier, the set Material nodes, the objects we set in constraints, ...

Basically I don't think we have the option anywhere else in Blender to make a new datablock outside the context where the new data-block can be edited. And I don't see a strong reason to break that rule for the custom properties.

I would expect this to use a simple ID "search" instead of a template (without allowing for Fake User or Duplicate/Make Unique). Similar to how we refer to points elsewhere such as the UV Projection modifier, the set Material nodes, the objects we set in constraints, ... Basically I don't think we have the option anywhere else in Blender to make a new datablock outside the context where the new data-block can be edited. And I don't see a strong reason to break that rule for the custom properties.

Brecht Van Lommel commented

2023-08-15 15:45:08 +02:00

I think ID search is normally used for cases where the user count is not increased, like objects which exist only by being in the scene. For the general case I think showing the user count and fake user buttons is important.

I don't think we need a button to create a new datablock here though.

I think ID search is normally used for cases where the user count is not increased, like objects which exist only by being in the scene. For the general case I think showing the user count and fake user buttons is important. I don't think we need a button to create a new datablock here though.

Bastien Montagne commented

2023-08-15 15:52:13 +02:00

@brecht The only way to create new IDs here is by the implicit behavior of the user count button, which creates a single-user copy of the selected ID when clicked... AFAIK there is no way to prevent this behavior?

Brecht Van Lommel commented

2023-08-15 15:54:06 +02:00

From @dfelinto's reply I got the impression it was possible to create a completely new datablock, but creating a single user copy seems fine to me.

Campbell Barton commented

2023-08-18 13:52:05 +02:00

In general this patch looks good, only two points.

Editing an ID property always resets it to None, even when the description is changed, attached patch that resolves this issue.
I'd prefer DATA_BLOCK over DATABLOCK for the enum.

I don't think another review iteration is needed, accepting.

In general this patch looks good, only two points. - Editing an ID property always resets it to None, even when the description is changed, attached patch that resolves this issue. - I'd prefer `DATA_BLOCK` over `DATABLOCK` for the enum. I don't think another review iteration is needed, accepting.

110458_patch_on_PR.diff

2.6 KiB

110458_patch_on_main.diff

9.9 KiB

Campbell Barton approved these changes 2023-08-18 13:52:53 +02:00

Bastien Montagne force-pushed ui-for-datablock-customprops from 34bd74e51d to e4ce221172

2023-08-21 12:07:06 +02:00

Compare

Bastien Montagne commented

2023-08-21 12:07:13 +02:00

@blender-bot build

Bastien Montagne force-pushed ui-for-datablock-customprops from e4ce221172 to f15e1c2a3a