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

Lower minimum speed limiter speed to 5 km/h #5065

Merged
merged 4 commits into from
May 31, 2017
Merged

Lower minimum speed limiter speed to 5 km/h #5065

merged 4 commits into from
May 31, 2017

Conversation

Schmitt-DE
Copy link
Contributor

@Schmitt-DE Schmitt-DE commented Apr 8, 2017

Minimum speed limit of 10 km/h was needed in the past due to engine limitations. Multiple user tests have shown that the minimum speed is not needed anymore. The new minimum of 0 km/h allows for example setting walking speed for vehicles (<10 km/h).

Minimum speed limit of 10 km/h was needed in the past due to engine limitations. Multiple user tests have shown that the minimum speed is not needed anymore. The new minimum of 0 km/h allows for example setting walking speed for vehicles (<10 km/h).
@jonpas
Copy link
Member

jonpas commented Apr 8, 2017

dedmen: What is the reason for the 10kmh limit?
commy2: Handbrake
commy2: PhysX Handbrake stops vehicles abruptly below 10 kmph

@derbuur
Copy link

derbuur commented Apr 8, 2017

Please see also discussion on armaworld forum https://armaworld.de/index.php?thread/2892-tempomat-f%C3%BCr-schrittgeschwindigkeit/&pageNo=1.
It seems that the original reason (Handbrake) is solved by BI so that the 10km/h limit is not longer necessary.

@commy2
Copy link
Contributor

commy2 commented Apr 8, 2017

What happens when you drive backwards and use the limiter after this PR?

@commy2 commy2 self-assigned this Apr 8, 2017
@derbuur
Copy link

derbuur commented Apr 8, 2017

Maybe change the minimum speed limit from 10 km/h to 5 km/h or 3 km/h to avoid any problems with driving backwards.

@jonpas
Copy link
Member

jonpas commented Apr 8, 2017

Don't speak German, so that forum is useless to me (and apparently everyone speaks badly enough for Google Translate to not make any sense of it). 😆

@derbuur
Copy link

derbuur commented Apr 8, 2017

sorry jonpas, let me give you a brief result of the forum post.
We tested the minimal speed with following function:
oneachframe { _vehicle = vehicle player; _maxSpeed = 1; _speed = speed _vehicle; if (_speed > _maxSpeed) then { _vehicle setVelocity ((velocity _vehicle) vectorMultiply ((_maxSpeed / _speed) - 0.00001)); };};
Six month ago the result of this function with slow speed was very bad and not playable, it was like a driver without knowledge of coupling.
But tested again a couple of days ago the result was very good. It seams that BI has changed the behavior of coupling and handbrake.
This is all written in the forum.

@jonpas
Copy link
Member

jonpas commented Apr 8, 2017

Ah, nice! Thank you.

@Schmitt-DE
Copy link
Contributor Author

If you feel more comfortable with min speed of 3 km/h you can of course change from 10 to 3. This also solves the problem I want to avoid with the pull request. With minimum 3 km/h it is easily possible to limit to walking speed (~6 km/h). Do you want me to generate a new pull request with minimum speed 3?

@jonpas
Copy link
Member

jonpas commented Apr 10, 2017

3 sounds good, you can just commit to the same branch, it will show on PR automatically.

To avoid problems with negative speeds (driving backwards) and zero speed, the current change switches from 10 km/h minimum speed to 3 km/h minimum speed. This seems to be the optimal solution to allow all relevant speeds including walking speed.
@Schmitt-DE
Copy link
Contributor Author

Done. So what are the next steps? Who needs to approve this change?

@commy2
Copy link
Contributor

commy2 commented Apr 10, 2017

One of the maintainers of this component and the author of the original function, who incidentally assigned this pull request to himself - me.

@commy2 commy2 added this to the 3.10.0 milestone Apr 10, 2017
@commy2 commy2 added the kind/enhancement Release Notes: **IMPROVED:** label Apr 10, 2017
@commy2
Copy link
Contributor

commy2 commented Apr 10, 2017

This pull request breaks the speed limiter for cars.
For example, enter a quadbike and set the speed limiter to 3 kmph. The driving is extremely choppy in curves and the worst thing is, that the engine triggers the brake sound constantly. Sounds like you're driving with the handbrake on.

@Schmitt-DE
Copy link
Contributor Author

Who ever wants to set 3 km/h speed limit for a quadbike?! Nobody in history has ever tried and nobody will ever try. Much more important is to support those players who want to synchronize tank and soldier speed when approaching cities. And if someone really tries to set it for quad bikes, he will notice that 3 km/h does not work really fine and this guy will raise his speed.

@commy2
Copy link
Contributor

commy2 commented Apr 11, 2017

Disagreed. The feature has to work in any circumstance. And as for tanks, the vanilla ones might not use brake sounds, but I know of certain ones that do.

@Schmitt-DE
Copy link
Contributor Author

so what about a new keyboard combination like SHIFT+DEL for limiting to any speed limit. this new keyboard combination will be hidden in the code and not being officially supported?

@commy2
Copy link
Contributor

commy2 commented Apr 11, 2017

Sounds like something you should put in your mission or community addon packet and not in ACE.

@bux
Copy link
Member

bux commented Apr 11, 2017

The feature has to work in any circumstance.

I agree with @commy2.

@Schmitt-DE
Copy link
Contributor Author

Ok. But could you at least transform the current value 10 to a global variable, so that others have the chance to influence the script from outside? Hard coded numbers in a script are not the nicest way of coding so that you could solve two issues at the same time: 1) High quality code 2) Chance for others to influence minimum required speed in their environment

@bux
Copy link
Member

bux commented Apr 11, 2017

transform the current value 10 to a global variable

Could be a solution.

Hard coded numbers in a script are not the nicest way of coding

Though, generally speaking this might be true, but in this case it's utterly wrong.

The reason why 10 has been chosen is explained above.

@Schmitt-DE
Copy link
Contributor Author

As I said. Fine for me when 10 needs to be the official ACE value. But there should be some way to configure this minimum speed value.

@PabstMirror
Copy link
Contributor

What about something like this?

// Set `ace_vehicles_speedLimiterMin` in mission to change limit (may cause problems if set too low)
private _limit = missionNamespace getVariable [QGVAR(speedLimiterMin), 10];
private _maxSpeed = speed _vehicle max _limit;

Would let people change it via mission or a little addon.

@jonpas
Copy link
Member

jonpas commented Apr 11, 2017

In that case I'd just make it a setting to be honest.

@PabstMirror
Copy link
Contributor

If we make it a setting, then people can set it to values that could break things.
I would rather just make it a semi-hidden mission var.
If you need to change it, you still can.

@Schmitt-DE
Copy link
Contributor Author

By the way: I asked again what is the absolute minimum speed needed for tank/soldier groups. Minimum of 5 km/h would still be fine. So if the car issue that came up with 3 km/h is no problem with 5 km/h, we can easily solve the current issue by setting 5 km/h. Otherwise a semi-hidden mission var could do the job like @PabstMirror depicted.

Officially the minimum required speed is 10 km/h in the master. Lower minimum needed to set car speed to walking speed of accompanying soldiers. Problems have been reported with 3 kmh/ using cars like ATVs.  Thus the new commit is set to 5 km/h minimum speed. Not tested with ATVs yet.
@kymckay
Copy link
Member

kymckay commented Apr 16, 2017

Is there no minimum speed value that can be read from the vehicle's config?

@commy2
Copy link
Contributor

commy2 commented Apr 16, 2017

No, but I guess it's hard coded for the different simulation types.

@jonpas jonpas changed the title No minimum speed for speed limiter anymore Lower minimum speed limiter speed to 5 km/h May 31, 2017
Copy link
Contributor

@PabstMirror PabstMirror left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Limiting to 5kph, tanks sound rough (like they're stuck in first gear at high rpm)
But didn't seem to be any more likely to flip 😄

Hopefully this doesn't cause any issues and people can still always use 10kph if there is.

@PabstMirror PabstMirror merged commit d90f15a into acemod:master May 31, 2017
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