BGE: Sensor.frequency is badly named #42341

New Issue

inactive account · 2014-10-22T09:57:02+02:00

inactive account commented

2014-10-22 09:57:02 +02:00

The BGE class Sensor has a property frequency , that determines the frequency of the pulses it emits.

Expected behaviour: increase the property to increase the frequency of the pulses.
Observed behaviour: increasing the property decreases the frequency of the pulses.

The tooltip notes that it's actually the "Delay between repeated pulses". This matches the observed behaviour.

Based on these observations, the name "frequency" seems wrong, and "period" would be a better name, as this is the reciprocal of frequency .

Note that if only the name is changed to "period", it would still also require a change in values in order to be correct. Right now "frequency = 0" indicates a period of 1; "frequency = 1" indicates a period of 2, and so on.

I understand that changing a property name is not something trivial, as it introduces incompatibility with existing code/files. However, I do feel that this issue should be addressed, for two reasons:

The name "frequency" doesn't match the behaviour.
The property name and its tooltip are in conflict.

PS: If I'm really nitpicky about this, even the word "delay" in the tooltip is a misnomer. If somebody ships 100 DVDs to a customer, and they are delayed, in the end all 100 DVDs still get delivered, or people wouldn't speak of "delaying" but "losing" or "misplacing". However, with 100 pulses and "frequency = 3", only 25 pulses would be "delivered".

The BGE class Sensor has a property [frequency ](http://www.blender.org/documentation/blender_python_api_2_72_release/bpy.types.Sensor.html#bpy.types.Sensor.frequency), that determines the frequency of the pulses it emits. **Expected behaviour:** increase the property to *increase* the frequency of the pulses. **Observed behaviour:** increasing the property *decreases* the frequency of the pulses. The tooltip notes that it's actually the "Delay between repeated pulses". This matches the observed behaviour. Based on these observations, **the name "frequency" seems wrong**, and "period" would be a better name, as [this is the reciprocal of frequency ](http://en.wikipedia.org/wiki/Frequency). Note that if only the name is changed to "period", it would still **also require a change in values** in order to be correct. Right now "frequency = 0" indicates a period of 1; "frequency = 1" indicates a period of 2, and so on. I understand that changing a property name is not something trivial, as it introduces incompatibility with existing code/files. However, I do feel that this issue should be addressed, for two reasons: - The name "frequency" doesn't match the behaviour. - The property name and its tooltip are in conflict. PS: If I'm really nitpicky about this, even the word "delay" in the tooltip is a misnomer. If somebody ships 100 DVDs to a customer, and they are delayed, in the end all 100 DVDs still get delivered, or people wouldn't speak of "delaying" but "losing" or "misplacing". However, with 100 pulses and "frequency = 3", only 25 pulses would be "delivered".

inactive account commented

2014-10-22 09:57:02 +02:00

Changed status to: 'Open'

inactive account commented

2014-10-22 09:57:02 +02:00

Added subscriber: @sybrenstuvel

Lukas Tönne commented

2014-10-22 11:15:41 +02:00

Added subscriber: @LukasTonne

Jorge Bernal commented

2015-04-09 15:29:37 +02:00

Added subscriber: @JorgeBernalMartinez

Jorge Bernal commented

2015-04-09 15:29:37 +02:00

Maybe we should change the tooltip to "Number of pulses omitted between 2 active pulses (in logic tics, 0 = no pulses omitted)" and the "Freq:" by "Omit:".

We could also change "freq" or "m_pulse_frequency" internal variables to a name more appropiated but I'm not totally convince of this one.

Maybe we should change the tooltip to "Number of pulses omitted between 2 active pulses (in logic tics, 0 = no pulses omitted)" and the "Freq:" by "Omit:". We could also change "freq" or "m_pulse_frequency" internal variables to a name more appropiated but I'm not totally convince of this one.

Sybren A. Stüvel commented

2015-04-09 18:07:41 +02:00

Added subscriber: @dr.sybren

Sybren A. Stüvel commented

2015-04-09 18:07:41 +02:00

In #42341#301495, @JorgeBernalMartinez wrote:
Maybe we should change the tooltip to "Number of pulses omitted between 2 active pulses (in logic tics, 0 = no pulses omitted)"

For me "Number of pulses omitted (in logic ticks)" is hard to understand, as it implies that you can count pulses in logic ticks, whereas this setting is exactly there to change the relation between pulses and logic ticks.

"Number of logic ticks between 2 active pulses (0 = pulse every logic tick, 1 = skip 1 logic tick between pulses, etc.)" may be clearer.

and the "Freq:" by "Omit:".

I'm partial to either "period" or "skip" rather than "omit".

We could also change "freq" or "m_pulse_frequency" internal variables to a name more appropiated but I'm not totally convince of this one.

IMO that would certainly be an improvement. "period" would stop my head from hurting every time I see that variable name. Naming something the exact opposite of what it represents, which is the current situation, is Not Good.

> In #42341#301495, @JorgeBernalMartinez wrote: > Maybe we should change the tooltip to "Number of pulses omitted between 2 active pulses (in logic tics, 0 = no pulses omitted)" For me "Number of pulses omitted (in logic ticks)" is hard to understand, as it implies that you can count pulses in logic ticks, whereas this setting is exactly there to change the relation between pulses and logic ticks. "Number of logic ticks between 2 active pulses (0 = pulse every logic tick, 1 = skip 1 logic tick between pulses, etc.)" may be clearer. > and the "Freq:" by "Omit:". I'm partial to either "period" or "skip" rather than "omit". > We could also change "freq" or "m_pulse_frequency" internal variables to a name more appropiated but I'm not totally convince of this one. IMO that would certainly be an improvement. "period" would stop my head from hurting every time I see that variable name. Naming something the exact opposite of what it represents, which is the current situation, is Not Good.

Lukas Tönne commented

2015-04-09 18:58:19 +02:00

Removed subscriber: @LukasTonne

Jorge Bernal commented

2015-04-09 20:49:10 +02:00

In #42341#301561, @dr.sybren wrote:

For me "Number of pulses omitted (in logic ticks)" is hard to understand, as it implies that you can count pulses in logic ticks, whereas this setting is exactly there to change the relation between pulses and logic ticks.

"Number of logic ticks between 2 active pulses (0 = pulse every logic tick, 1 = skip 1 logic tick between pulses, etc.)" may be clearer.

Ok. Much better.

I'm partial to either "period" or "skip" rather than "omit".

I'm for "skip" as it will avoid a do_versions to convert from 0 to 1 as default value (and also avoid internal logic coversion)

IMO that would certainly be an improvement. "period" would stop my head from hurting every time I see that variable name. Naming something the exact opposite of what it represents, which is the current situation, is Not Good.

Following above election then i would use: tics_skipped, m_tics_skipped, ticskip, etc

If you are agree, I have a patch ready to upload to differential.

> In #42341#301561, @dr.sybren wrote: > > For me "Number of pulses omitted (in logic ticks)" is hard to understand, as it implies that you can count pulses in logic ticks, whereas this setting is exactly there to change the relation between pulses and logic ticks. > > "Number of logic ticks between 2 active pulses (0 = pulse every logic tick, 1 = skip 1 logic tick between pulses, etc.)" may be clearer. Ok. Much better. > > I'm partial to either "period" or "skip" rather than "omit". > I'm for "skip" as it will avoid a do_versions to convert from 0 to 1 as default value (and also avoid internal logic coversion) > > IMO that would certainly be an improvement. "period" would stop my head from hurting every time I see that variable name. Naming something the exact opposite of what it represents, which is the current situation, is Not Good. Following above election then i would use: tics_skipped, m_tics_skipped, ticskip, etc If you are agree, I have a patch ready to upload to differential.

Sybren A. Stüvel commented

2015-04-10 08:53:53 +02:00

In #42341#301584, @JorgeBernalMartinez wrote:
If you are agree, I have a patch ready to upload to differential.

Fire it up! :)

> In #42341#301584, @JorgeBernalMartinez wrote: > If you are agree, I have a patch ready to upload to differential. Fire it up! :)

Jorge Bernal commented

2015-04-16 06:54:26 +02:00

Changed status from 'Open' to: 'Resolved'

Jorge Bernal closed this issue

2015-04-16 06:54:26 +02:00

Jorge Bernal self-assigned this 2015-04-16 06:54:26 +02:00

Jorge Bernal commented

2015-04-16 06:54:26 +02:00

Closed by commit 8c98b1649d

Closed by commit 8c98b1649d

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

BGE: Sensor.frequency is badly named #42341