RNA: unify and improve RNA property flag descriptions #118036

Sybren A. Stüvel · 2024-02-09T13:02:16+01:00

Sybren A. Stüvel commented

2024-02-09 13:02:16 +01:00

This PR consists of two commits, and I intend to land them as separate commits:

Ensure the flag descriptions are used for both non-enum and enum properties. Before only the former were updated, but the latter still had empty descriptions.
Write new descriptions for HIDDEN, SKIP_SAVE, and LIB_EXCEPTION flags.

Most of these I already wrote before 0d7282e69b landed. I adjusted the text to account for that commit as well.

Personally I don't think "Do not use ghost values" was particularly helpful, as the concept of "ghost values" isn't actually that well known (compared to generally how many people use bpy). Granted, it's described at bpy_struct, but IMO if the term "ghost values" is to be kept here, it should include a link to that page.

I've also changed Inherits 'SKIP_PRESET' to a description of what actually happens, so that each flag can be read by itself.

This PR consists of two commits, and I intend to land them as separate commits: 1. Ensure the flag descriptions are used for both non-enum and enum properties. Before only the former were updated, but the latter still had empty descriptions. 2. Write new descriptions for `HIDDEN`, `SKIP_SAVE`, and `LIB_EXCEPTION` flags. Most of these I already wrote before 0d7282e69b35d1cc7c67629ecddbd397b85ea727 landed. I adjusted the text to account for that commit as well. Personally I don't think "Do not use ghost values" was particularly helpful, as the concept of "ghost values" isn't actually that well known (compared to generally how many people use `bpy`). Granted, it's described at [bpy_struct](https://docs.blender.org/api/current/bpy.types.bpy_struct.html), but IMO if the term "ghost values" is to be kept here, it should include a link to that page. I've also changed `Inherits 'SKIP_PRESET'` to a description of what actually happens, so that each flag can be read by itself.

Sybren A. Stüvel added 2 commits 2024-02-09 13:02:26 +01:00

8a51c3ef7a RNA: unify property flag descriptions between enum and non-enum flags

These flags (like `'HIDDEN', 'SKIP_SAVE', etc.) mean the same thing when
used on enums and non-enum properties. Now they actually use the same
description definition in the source code, ensuring that any
improvements will apply to both.

No changes to the actual messages. Just using them for both enums and
non-enum property flags.

d079474f1e RNA: update property flag descriptions

Add new descriptions to the `PROP_HIDDEN`, `PROP_SKIP_SAVE`, and
`PROP_LIB_EXCEPTION` flags. For the former two, these are more
explanatory than the previous descriptions. For the latter it's the
first ever description.

Most of these I already wrote before
0d7282e69b landed. I adjusted the text to
account for that commit as well.

Personally I don't think "Do not use ghost values" was particularly
helpful, as the concept of "ghost values" isn't actually that well known
(compared to generally how many people use `bpy`). Granted, it's
described at [1], but IMO if the term "ghost values" is to be kept here,
it should include a link to that page.

[1]: https://docs.blender.org/api/current/bpy.types.bpy_struct.html

Sybren A. Stüvel requested review from Brecht Van Lommel 2024-02-09 13:07:36 +01:00

Steff Kempink reviewed 2024-02-09 13:31:24 +01:00

source/blender/makesrna/intern/rna_rna.cc Outdated

						
				@ -156,0 +156,4 @@

				/* Descriptions for rna_enum_property_flag_items and rna_enum_property_flag_enum_items. */

				constexpr auto PROP_HIDDEN_DESCR =

				    "For operators: exclude from the 'Redo' panel. The property will still be visible when "

				    "explicitly drawn in UI code. Also this property is not written to presents";

Steff Kempink commented

2024-02-09 13:29:45 +01:00

First-time contributor

presents->presets? (also happened below in SKIP_SAVE_DESCR)

Sybren A. Stüvel commented

2024-02-09 14:13:03 +01:00

It's my dad's b'day, can you tell? :)

🎉 1

dr.sybren marked this conversation as resolved

Brecht Van Lommel requested changes 2024-02-09 13:52:42 +01:00

source/blender/makesrna/intern/rna_rna.cc Outdated

						
				@ -154,2 +154,4 @@

				};

				/* Descriptions for rna_enum_property_flag_items and rna_enum_property_flag_enum_items. */

				constexpr auto PROP_HIDDEN_DESCR =

Brecht Van Lommel commented

2024-02-09 13:45:13 +01:00

constexpr -> static constepxr, this should not be in the global namespace.

Also not sure about the use of auto here, seems like deviating from existing conventions.

`constexpr` -> `static constepxr`, this should not be in the global namespace. Also not sure about the use of `auto` here, seems like deviating from existing conventions.

Sybren A. Stüvel commented

2024-02-09 14:34:26 +01:00

I'll add the static. I tried to remove auto, but then the full declaration would be static constexpr const char *.

Without the const in there, I get this compiler warning:

warning: ISO C++11 does not allow conversion from string literal to 'char *const' [-Wwritable-strings]

I think the auto is preferrable here, as it's also not such complex code that it becomes hard to understand what the type is.

I'll add the `static`. I tried to remove `auto`, but then the full declaration would be `static constexpr const char *`. Without the `const` in there, I get this compiler warning: ``` warning: ISO C++11 does not allow conversion from string literal to 'char *const' [-Wwritable-strings] ``` I think the `auto` is preferrable here, as it's also not such complex code that it becomes hard to understand what the type is.

dr.sybren marked this conversation as resolved

source/blender/makesrna/intern/rna_rna.cc Outdated

						
				@ -155,1 +155,4 @@

				/* Descriptions for rna_enum_property_flag_items and rna_enum_property_flag_enum_items. */

				constexpr auto PROP_HIDDEN_DESCR =

				    "For operators: exclude from the 'Redo' panel. The property will still be visible when "

Brecht Van Lommel commented

2024-02-09 13:51:08 +01:00

Do not use quotes around 'Redo' (guideline for referring to Blender terms in general). Also I think we don't call it the Redo panel in the UI, and this affects also panels should in properties dialogs.

Brecht Van Lommel commented

2024-02-09 13:56:48 +01:00

Maybe better would be to say something like, "hide from the user interface (unless manually used in the operator draw callback)`

Sybren A. Stüvel commented

2024-02-09 14:27:21 +01:00

You're right, in the user manual it's called the Adjust Last Operation region. Not sure how many people would refer this as a "region" though, but that's a discussion for another time.

hide from the user interface (unless manually used in the operator draw callback)

The fact that it's hidden is quite clear from the flag's name. I wanted to have some information in there about where it would be shown if this flag isn't used. You're right in that the redo panel isn't the only place, and that it can pop up in other areas as well. I'll go for a combo, like "hide from places in the user interface where Blender would add the property automatically, like Adjust Last Operation". This hints that the flag is ineffective when you explicitly draw the property, and I think it's clear enough without going into too many details.

You're right, in the user manual it's called the [Adjust Last Operation region](https://docs.blender.org/manual/en/latest/interface/operators.html#operator-properties). Not sure how many people would refer this as a "region" though, but that's a discussion for another time. > hide from the user interface (unless manually used in the operator draw callback) The fact that it's hidden is quite clear from the flag's name. I wanted to have some information in there about where it *would* be shown if this flag isn't used. You're right in that the redo panel isn't the only place, and that it can pop up in other areas as well. I'll go for a combo, like "hide from places in the user interface where Blender would add the property automatically, like Adjust Last Operation". This hints that the flag is ineffective when you explicitly draw the property, and I think it's clear enough without going into too many details.

dr.sybren marked this conversation as resolved

source/blender/makesrna/intern/rna_rna.cc Outdated

						
				@ -156,0 +158,4 @@

				    "For operators: exclude from the 'Redo' panel. The property will still be visible when "

				    "explicitly drawn in UI code. Also this property is not written to presents";

				constexpr auto PROP_SKIP_SAVE_DESCR =

				    "For operators: the value of this property will not be remembered between invocations to the "