Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vehicles - Utilize setCruiseControl for speed limiter #8273

Merged
merged 5 commits into from
Oct 13, 2021

Conversation

AndreasBrostrom
Copy link
Contributor

@AndreasBrostrom AndreasBrostrom commented Jun 1, 2021

When merged this pull request will:

Adjustments and code of note:

  • Kept setVelocity with a +2 to speed check to make vehicle slow down downhill and to slow it down when enabled.
  • Still use the per frame due to ctrl+scroll adjustment check and active breake.

REQUIRE ARMA Version v2.06 >

Kept setVelocity with a +2 to speed check to make vehicle slow down downhill and to slow it down when enabled.
@jonpas jonpas added kind/enhancement Release Notes: **IMPROVED:** target/next-arma Requires something in arma dev branch labels Jun 1, 2021
@jonpas jonpas changed the title Speed limiter utalizes setCruiseControl Vehicles - Utilize setCruiseControl for speed limiter Jun 1, 2021
@Drofseh
Copy link
Contributor

Drofseh commented Jun 1, 2021

I don't know if you want to do it in this PR, but someway to have autoThrust as true to do actual cruise control would be nice to have at some point

@AndreasBrostrom
Copy link
Contributor Author

I don't know if you want to do it in this PR, but someway to have autoThrust as true to do actual cruise control would be nice to have at some point

I can experiement with it in a nother pr.

@jonpas
Copy link
Member

jonpas commented Jun 2, 2021

@AndreasBrostrom
Copy link
Contributor Author

AndreasBrostrom commented Jun 3, 2021

Tweaked: Cruise control now aborts the speed limit when you brake manually

the script reapply it if i am not mistaken. will check effects.

Tweaked: Cruise control now lets you drive faster than the target speed when you apply manual thrust

should not effect this pr

@dedmen
Copy link
Contributor

dedmen commented Jun 4, 2021

Kept setVelocity with a +2 to speed check to make vehicle slow down downhill and to slow it down when enabled.

speed limiter will apply brakes if you are too fast.
setVelocity is unrealistic and will stop you immediately, thats bad

@AndreasBrostrom
Copy link
Contributor Author

The cruise control do not activly slow you down when your going down hill wich is the resoning why i kept the setVelocity as mentioned.
In normal cases you enable the speed limiter with delete key and a sudden slow down in speed whould never apply.

I did not have the setVelocity breaker in the begining. But after testing were i manage go over ~20km/h of the speed limit.
With the setVelocity, to no suprce never reached, over set speed +2.

@dedmen
Copy link
Contributor

dedmen commented Aug 24, 2021

The cruise control do not activly slow you down when your going down hill wich is the resoning

It should actively brake though.
I just looked at the code again
if (abs(curSpeed-setpoint) > setpoint*0.2f && speed>setpoint)
brakeWanted = 0.5;

The first check is to limit the PID controller to a smaller range, outside of the range its manually controlled, inside its controlled by the PID.

so if your limit is 50kmh if your speed delta is more than 20% (10kmh) above setpoint it will apply 50% brakes and force thrust to 0.
Then when you come within 20% of setpoint the brakes will be removed and thrust will be controlled by the PID (which will immediately apply low thrust as you are above setpoint)

I only did 50% brakes to make it a little smoother, maybe thats not sufficient for your downhill test.

My main concern is the "exploit" where you just quickly mouse-wheel your speed down if you need to quickly stop to apply the "magic" brakes.

Maybe the "magic" brake can be disabled if you just reduced speed via mouse wheel in the last 2-5 seconds?

@dedmen
Copy link
Contributor

dedmen commented Aug 24, 2021

Okey I just tested, because the PID controller only controls thrust and not brakes (while you are within the 10kmh delta) you will go 60kmh downhill when speedlimit is set to 50 (testing on malden mountain)
The PID controller can go negative so I will adjust the code to apply brakes too.

@AndreasBrostrom
Copy link
Contributor Author

Okey I just tested, because the PID controller only controls thrust and not brakes (while you are within the 10kmh delta) you will go 60kmh downhill when speedlimit is set to 50 (testing on malden mountain)
The PID controller can go negative so I will adjust the code to apply brakes too.

feel free to adjust

@dedmen
Copy link
Contributor

dedmen commented Aug 24, 2021

Okey I fixed it.
Looks good to me. of course theres a little over/undershoot but looks good enough.
Also found another bug, speed limiter only applied when forward thrust was applied, meaning you could just roll downhill and not be stopped. Fixed that too.

https://youtu.be/36od_7hFr7w

log bottom right

Thrust, Brake, PID output

Will be in next RC build this week. Maybe profiling branch if I do one
The wiki page was also updated with more details about how it works https://community.bistudio.com/wiki/setCruiseControl

addons/vehicles/functions/fnc_speedLimiter.sqf Outdated Show resolved Hide resolved
addons/vehicles/functions/fnc_speedLimiter.sqf Outdated Show resolved Hide resolved
@jonpas jonpas added this to the 3.14.0 milestone Oct 5, 2021
@jonpas jonpas removed the target/next-arma Requires something in arma dev branch label Oct 5, 2021
@veteran29
Copy link
Member

Mission makers might want to limit the speed of certain vehicles for whatever reason, I think we should limit the max limiter speed to the value of the limit at the time of Speedlimiter activation.

@Zakant
Copy link
Contributor

Zakant commented Oct 6, 2021

Mission makers might want to limit the speed of certain vehicles for whatever reason, I think we should limit the max limiter speed to the value of the limit at the time of Speedlimiter activation.

I'm against this. We often activate the speed limiter when not moving at all to have clear 5 km/h steps. Otherwise vehicles end up driving 43 and 47 km/h. And for players i dont see the benefit. One could just disable it, speed up and reenable it?

AndreasBrostrom and others added 2 commits October 7, 2021 18:45
Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
@jonpas
Copy link
Member

jonpas commented Oct 10, 2021

I am confused, are @veteran29's and @Zakant's comments even related? One seems to speak about upper-most value that can be restricted from a perspective of a mission maker, and another speaks of value when you activate.

If I am not mistaken, what @Zakant is saying is actually how it works before this PR. You can still increase it by step. Someone explain?

@veteran29
Copy link
Member

I think Zakant does not understand what I meant. His comment does not make sense to me.

@jonpas jonpas requested a review from veteran29 October 10, 2021 14:16
@veteran29
Copy link
Member

veteran29 commented Oct 10, 2021

What I meant about respecting mission maker set limit is basically this...

when speed limiter is engaged, outside of PFH:

GVAR(maxSpeedLimit) = getCruiseControl _vehicle select 0;
if (GVAR(maxSpeedLimit) == 0) then {
    GVAR(maxSpeedLimit) = 1e10;
};

inside the PFH:

    getCruiseControl _vehicle params ["_currentSpeedLimit"];
    if (_currentSpeedLimit != GVAR(speedLimit)) then {
        _vehicle setCruiseControl [GVAR(speedLimit) min GVAR(maxSpeedLimit), false];
    };

I guess the limit should be enforced in the handler of the "increase limit" function instead of the PFH but this should explain what I've meant.

Co-authored-by: PabstMirror <pabstmirror@gmail.com>
Co-authored-by: PabstMirror <pabstmirror@gmail.com>
@PabstMirror
Copy link
Contributor

@veteran29 Does that last sound ok?

jonpas added a commit to Theseus-Aegis/ACE3 that referenced this pull request Oct 13, 2021
@veteran29
Copy link
Member

@veteran29 Does that last sound ok?

I guess it's ok for now, allowing to use the feature and restoring the mission set limit would be better but can be done in another PR.

@PabstMirror PabstMirror merged commit 0c85f5f into acemod:master Oct 13, 2021
commy2 added a commit that referenced this pull request Oct 24, 2021
AndreasBrostrom added a commit to AndreasBrostrom/ACE3 that referenced this pull request Dec 3, 2021
* Change speed limiter to utalize setCruiseControl
Kept setVelocity with a +2 to speed check to make vehicle slow down downhill and to slow it down when enabled.

* Update addons/vehicles/functions/fnc_speedLimiter.sqf

Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>

* Update addons/vehicles/functions/fnc_speedLimiter.sqf

Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>

* Update addons/vehicles/functions/fnc_speedLimiter.sqf

Co-authored-by: PabstMirror <pabstmirror@gmail.com>

* Update addons/vehicles/functions/fnc_speedLimiter.sqf

Co-authored-by: PabstMirror <pabstmirror@gmail.com>

Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
Co-authored-by: PabstMirror <pabstmirror@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants