Fixed Anti Gravity Gain value #2005
Conversation
|
what about this sonar issue??And maybe we can rename |
|
The Sonar issue only tells you to change the |
src/js/tabs/pid_tuning.js
Outdated
| @@ -182,8 +182,12 @@ TABS.pid_tuning.initialize = function (callback) { | |||
| var checked = $(this).is(':checked'); | |||
| if (checked) { | |||
| const MAX_ACCELERATOR_GAIN = semver.gte(CONFIG.apiVersion, API_VERSION_1_43) ? 3.5 : 1.1; | |||
| const itermAcceleratorGain = Math.max(ADVANCED_TUNING.itermAcceleratorGain / 1000, MAX_ACCELERATOR_GAIN); | |||
| $('.antigravity input[name="itermAcceleratorGain"]').val(itermAcceleratorGain); | |||
| const itermAcceleratorGain = Math.max(ADVANCED_TUNING.itermAcceleratorGain / 1000); | |||
There was a problem hiding this comment.
Maybe you forgot the second parameter to the Math.max function? Or you forgot to remove it completely? I suppose the second one...
There was a problem hiding this comment.
yes sorry, I forgot to remove the math, fixed all issues now :)
| $('.antigravity input[name="itermAcceleratorGain"]').val(MAX_ACCELERATOR_GAIN); | ||
| const DEFAULT_ACCELERATOR_GAIN = semver.gte(CONFIG.apiVersion, API_VERSION_1_43) ? 3.5 : 1.1; | ||
| const itermAcceleratorGain = (ADVANCED_TUNING.itermAcceleratorGain / 1000); | ||
| if (ADVANCED_TUNING.itermAcceleratorGain === 1000) { |
There was a problem hiding this comment.
Sorry to not see this in the first review ;)
After removing the Math.max, the DEFAULT_ACCELERATOR_GAIN is only used inside the if, so better to move inside it:
if (ADVANCED_TUNING.itermAcceleratorGain === 1000) {
const DEFAULT_ACCELERATOR_GAIN = semver.gte(CONFIG.apiVersion, API_VERSION_1_43) ? 3.5 : 1.1;
$('.antigravity input[name="itermAcceleratorGain"]').val(DEFAULT_ACCELERATOR_GAIN);
} else {Another solution is to change the complete if-else by a:
$('.antigravity input[name="itermAcceleratorGain"]').val(ADVANCED_TUNING.itermAcceleratorGain === 1000 ? DEFAULT_ACCELERATOR_GAIN : itermAcceleratorGain);But maybe in this case is clearer with the if-else.
Take the form that you prefer...
There was a problem hiding this comment.
dont worry,fixed now, i think that I prefer if/else in this case,as you said looks more clearer.
There was a problem hiding this comment.
It happens the same with the itermAcceleratorGain :( It is used only in the else part...
I think this confinement has made me slower in reactions...
There was a problem hiding this comment.
I was thinking exactly that for itermAcceleratorGain
There was a problem hiding this comment.
Yes, and it is modified too some lines up:
so we end assigning it twice. I think this line can be removed but is outside the scope of this PR. To me is OK now.
There was a problem hiding this comment.
@McGiverGim all is done now!
| $('.antigravity input[name="itermAcceleratorGain"]').val(MAX_ACCELERATOR_GAIN); | ||
| const DEFAULT_ACCELERATOR_GAIN = semver.gte(CONFIG.apiVersion, API_VERSION_1_43) ? 3.5 : 1.1; | ||
| const itermAcceleratorGain = (ADVANCED_TUNING.itermAcceleratorGain / 1000); | ||
| if (ADVANCED_TUNING.itermAcceleratorGain === 1000) { |
There was a problem hiding this comment.
It happens the same with the itermAcceleratorGain :( It is used only in the else part...
I think this confinement has made me slower in reactions...
asizon
force-pushed
the
fix_antigravity_gain
branch
from
12dfc4f
to
d32fe20
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
sonar issue,remove unneded math,const renamed Moved const inside if Moved const inside else
Fixes #2007
Kudos to @selimmch for finding it
This is a bug in the code presented since this feature was added, also before, if you unchek antigravity without saving,you lost you configured value. Now we can note this bug when we aplies new 3.5 default value recently, before, we had 1.1,the limit to enable disable this feature.