Paths

Table of Contentst

Differential D4268

Fix: use correct units for some force field properties
Needs ReviewPublic
Actions

Authored by Scott Spadea (scottspadea) on Jan 27 2019, 9:58 PM.

Details

Reviewers

Jacques Lucke (JacquesLucke)

Summary

This revision is related to T60824.

In rna_object_force.c,
changed distance_min and distance_max to Distance units.
changed radial_min and radial_max to Angle units, then I realized they should be Distance units instead.

these radial settings are for Tube shaped force fields, setting a radius min and max for falloff calculations.
an Angle property should be created to define cone shaped force fields.

Diff Detail

Repository: rB Blender

Event Timeline

Scott Spadea (scottspadea) created this revision.Jan 27 2019, 9:58 PM

Scott Spadea (scottspadea) retitled this revision from Force Field Units to Fix: use correct units for some force field properties.Jan 28 2019, 1:22 PM

Scott Spadea (scottspadea) edited the summary of this revision. (Show Details)Jan 28 2019, 1:36 PM

Jacques Lucke (JacquesLucke) added a subscriber: William Reynish (billreynish).Jan 31 2019, 12:24 PM

Jacques Lucke (JacquesLucke) added inline comments.

source/blender/makesrna/intern/rna_object_force.c
1286	Unfortunately, we can't do this currently, because it looks like internally the value is stored in degree but the UI expects radians and multiplies the value again. @William Reynish (billreynish), can you confirm that? just a side note (not directly related to this patch), the name `minrad` is soo bad for an angle that is in not in radians ^^ radial force field limit.blend3 MBDownload

Hehe, you might be right. I guess the solution is to:

Change it work in radians
Make use of the units system to display as degrees

As for the internal name, maybe it could be changed to something actually descriptive like minimum_radial_distance or something. I don't maintain any of the underlying code, but I never understood why we use such short and crazy abbreviations for things - it just makes everything really hard to read. Why do we call it 'numbut' and number 'number_field' or something normal and readable? Anyway...

I guess the more sensible way is to change it to radians internally. Then the name actually starts to make more sense, smart haha

A more generic system for conversion of numbers between UI inter internals would be nice as well but is probably more complicated..

Yes, that was what I meant. Change it to radians internally, so that we can use PROP_ANGLE properly.

@Scott Spadea (scottspadea), do you want to try this?

In D4268#96985, @Jacques Lucke (JacquesLucke) wrote:

@Scott Spadea (scottspadea), do you want to try this?

sure

maybe instead of just PROP_ANGLE as a property subtype, we could split
it into PROP_RADIANS and PROP_DEGREES?
both should display as degrees to the user, but one needs conversion.

I found DEG2RADF() and RAD2DEGF()
and I tried to multiply by those in the getter and setter before setting minrad, but it was a generated file... so I need to learn more about editing getters and setters without being overwritten by the DNA/RNA code generation.

also, when looking at the way these properties affect the bounding cylinder of the force field, it seems more like a distance, than an angle. it just scales the radius of the cylinder. so should this property just be a PROP_DISTANCE ?

I believe radial_min and radial_max are distances for the radius of a Tube shaped force field.

Cone shaped force fields don't seem to be implemented yet...
a cone_angle property should be created for Cone shaped force fields.

Scott Spadea (scottspadea) edited the summary of this revision. (Show Details)Jan 31 2019, 10:36 PM

Damn, how stupid is that. The radial_max property ist as distance AND as angle depending on whether the shape is set to Tube or Cone.

Please take the radial_xxx properties out for now so that I can merge the distance properties separately... Not sure how to handle the other properties yet.

How frustrating! These different values really should be different properties altogether so we can use the correct units. Well, this focus on the units reveals some interesting oddities in Blender, that's for sure :)

William Reynish (billreynish) edited the summary of this revision. (Show Details)Feb 19 2019, 1:59 PM

Revision Contents
Changeset List

Path

Size

source/

blender/

makesrna/

intern/

rna_object_force.c

8 lines

Diff 13380

View Options

source/blender/makesrna/intern/rna_object_force.c

Fix: use correct units for some force field propertiesNeeds ReviewPublicActions