UI: Remove incorrect RNA percentage property definitions
ClosedPublic
Actions

Authored by Hans Goudey (HooglyBoogly) on Oct 25 2020, 10:25 PM.

Details

Reviewers

Campbell Barton (campbellbarton)

Group Reviewers

User Interface

Maniphest Tasks

T82070: Ratio displayed with a % sign in the Decimate Keyframes Adjust Last Operation panel.

Commits

rBc71a8e837616: UI: Fix incorrect RNA percentage property definitions

Summary

We now have a nice preference to control whether to display factors or
percentages, so it doesn't make sense to manually define properties with
percentages anymore.

In two cases the percentage property was actually used incorrectly, as
pointed out in T82070. The range was [0, 1], but the properties were still
displayed as percentages:

Given the the existance of RNA_def_float_factor, this patch completely
removes RNA_def_float_percentage to prevent further confusion. I would
be fine leaving this change out, but it's worth considering.

Fixes T82070

Diff Detail

Repository: rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) created this revision.Oct 25 2020, 10:25 PM

Harbormaster completed remote builds in B10908: Diff 30349.Oct 25 2020, 10:25 PM

Hans Goudey (HooglyBoogly) requested review of this revision.Oct 25 2020, 10:25 PM

Hans Goudey (HooglyBoogly) added a reviewer: User Interface.Oct 25 2020, 10:26 PM

Hans Goudey (HooglyBoogly) added a project: User Interface.

Hans Goudey (HooglyBoogly) mentioned this in T82070: Ratio displayed with a % sign in the Decimate Keyframes Adjust Last Operation panel..

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)

When I search for "PROP_FLOAT, PROP_PERCENTAGE" I get a few matches like eraser_strength_factor, I guess they are cases where the developer wanted to force display of percents, or just old code?

Changes missed in previous commit

Harbormaster completed remote builds in B10911: Diff 30356.Oct 26 2020, 12:17 AM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Oct 26 2020, 12:18 AM

Hans Goudey (HooglyBoogly) retitled this revision from UI: Remove RNA percentage property definitions to UI: Remove incorrect RNA percentage property definitions.

In D9344#231257, @Erik Abrahamsson (erik85) wrote:

When I search for "PROP_FLOAT, PROP_PERCENTAGE" I get a few matches like eraser_strength_factor, I guess they are cases where the developer wanted to force display of percents, or just old code?

Yeah, I'm not quite sure about that. I would like to discuss with the UI team whether there are circumstances where it's better to force the display of percentages with PROP_PERCENTAGE. If so there should be some guideline for when it makes sense.

Generally seems fine, calling this percent no longer makes any sense though.

Suggest to rename to factor, the UI label as well.

This revision now requires changes to proceed.Oct 28 2020, 11:13 AM

Right, good point. I think I prefer "Ratio" here since it's more specific to this action.
If you have a strong preference though, I can use "Factor".

Harbormaster completed remote builds in B11024: Diff 30579.Oct 31 2020, 6:48 AM

Campbell Barton (campbellbarton) accepted this revision.Jan 27 2021, 5:35 AM

This revision is now accepted and ready to land.Jan 27 2021, 5:35 AM

Note regarding ratio / factor, currently in Blender factor is normally used as a multiplier.

Ratio's are often represented as two values (16:9, 4:3 ... etc).

In this case for selection, ratio is already used in some areas, so I don't have a strong opinion against using ratio in this patch.

Although, I rather not try to push this change into new areas seeing as we have approx 100 use of the term factor in RNA, in the interest of not breaking API compatibility or having two different conventions depending on who writes the code, better leave these as-is.

Closed by commit rBc71a8e837616: UI: Fix incorrect RNA percentage property definitions (authored by Hans Goudey (HooglyBoogly)). · Explain WhyJan 27 2021, 6:20 PM

This revision was automatically updated to reflect the committed changes.

Hans Goudey (HooglyBoogly) added a commit: rBc71a8e837616: UI: Fix incorrect RNA percentage property definitions.