Page MenuHome

RNA Default Values as Defines
Closed, ResolvedPublicPATCH

Description

This patch can replace #32862, #32859, and #32854.

On almost all UI elements we can right-click and select "Reset to Default Value".
But this almost never resets it to default but instead sets it to the lowest value.
This is because almost no properties have default values set.

This patch adds more than 200 defaults to properties so that you can change many int,
float, boolean, or enum and then restore the default values with the right-click option.

As per Campbell Barton's request all defaults are defines within a single file, DNA_defaults.c

Event Timeline

typo: All the defines are in a single file, DNA_defaults.h

I think the defines are a good idea, but I would just place them in the DNA file where the relevant structs are defined. And they should be used in blenkernel/ as well when the structs are created, as I understand it the whole point of have these defined in DNA is to ensure there is no duplication with that code.

Thanks Brecht! Will take a look and see how much work is required
to do it that way, but it might be more than I can find. Will see...

The attached new patch, DefaultsAsDefines2.diff, should be as Brecht asked for it:

There is no longer a DNA_Defaults.h file. Instead the defines are in the DNA files where the structs are defined.

Blenkernel files are also modified so the defaults are used when the structs are created.

this makes for a large diff file, at 3649 lines, affecting a lot of files. This is not ALL the defaults, as that would take until the end of time, just the most visible ones on all the major panels except for material and textures.

Although I *could* break this patch into smaller, but related, pieces.
If that helps the review process....

It's not necessary to break up the patch, would not help much. It might take a few days until I get to reviewing and committing it.

It would be nice to have this reviewed fairly soon as it is a large patch. I will have time over the next few weeks to make changes if they are necessary, but will have less time starting in January.

I'll do this after the release, didn't have enough time to do before. Probably won't require further work from you anyway, the way it's implemented seems fine.

Hi,
is this implementation really the best one?
I mean, I see the reason for it, this way we only have 1 default variable per property, which can be used in RNA and on the lower level like camera.c etc.

Imho this is polluting DNA files too much. It's not entirely clear to me which default Value belongs to which property, while only looking at the DNA files.

The "Reset to Default" value uses the defaults in RNA, so I'd rather only add the defaults in RNA and keep the rest as is.

Thomas,

That is exactly as I did it originally and you can find that in patches #32862, #32859, and #32854.
Those just used constants like "RNA_def_property_int_default(prop, 44100)".

But then Campbell asked that all those constants be replaced with defines in a single file, and
that is "DefaultsAsDefines.diff"

After that Brecht asked that these instead go into the appropriate DNA file and be used to initialize
there as well. That is the "DefaultsAsDefines2.diff" version.

I've made a smaller patch against the current SVN (54976) to help with testing this out.
"DefaultsAsDefinesScene.diff" adds defaults to just the "scene" tab of the properties editor.
My hope is that you try this to see whether this feature is useful or if we should just remove
the "Reset to Default Values" item.

Can see the use of such place but implementation could be simplified i think. For example, we don't really need dedicated defines for the defaults only used by RNA. Unless the value is actually expected to be re-used by DNA struct allocation/initialization and RNA i wouldn't bother with dedicated define.

On the other hand, those values are never changing. @Campbell Barton (campbellbarton) has a nice idea, he is typing it now :)

Adding 1000's of defines is also giving us work to manage... (though it does make some sense).

What we could do instead:

  • Add a test file (in tests/python/rna_defaults.py)
  • The script can create new data scene, object... etc
  • The script compare initial values with the RNA defaults and raise errors when they don't match.

This way we have an automated way of keeping the values in sync without adding defines all over.

Made a script to check defaults are OK, (as described above) P219

Campbell Barton (campbellbarton) changed the task status from Unknown Status to Resolved.Apr 30 2015, 5:26 PM