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

Refine criteria that prohibit vehicle modification when moving #64056

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cake-pie
Copy link
Contributor

@cake-pie cake-pie commented Mar 8, 2023

Summary

Bugfixes "Refine criteria that prohibit vehicle modification when moving"

Purpose of change

This is the current check for whether a vehicle is moving before allowing modifications (e.g. adding / removing parts)

if( std::abs( veh->velocity ) > 100 || player_character.controlling_vehicle ) {
return task_reason::MOVING_VEHICLE;
}

It was adapted from the "Dive from moving vehicle?" check, so apparently a small enough non-zero velocity is fine! This should not be the case.

Meanwhile the check for whether the player is controlling the vehicle was intended as a guard against acceleration via cruise control. This isn't sufficient, especially also considering the development of autopilot, autodrive, and eventually looking toward things like NPC driving.

Here are a couple corner cases I can think of that would pass, but shouldn't:

  • Drive a vehicle, get it up to a relatively high speed. Stop controlling the vehicle while it is moving; vehicle will continue to coast uncontrolled while gradually slowing down. Per the criteria, you'd be allowed to remove a tire etc, once it reaches a sufficiently low speed, but before it has come to a complete stop. (The window of opportunity to do this will depend on deceleration rate.)
  • Some self-driving vehicle (auto-farm or auto-patrol) is moving sufficiently slowly, and you are able to get alongside it and examine as it passes by...

Describe the solution

  • require zero velocity
  • player controlling vehicle check replaced by actual check on cruise velocity
  • additional check to catch aircraft in stationary hover, since helicopters are a thing now
  • update the messages displayed to player

Describe alternatives you've considered

One could argue for allowing some simple things such as clock or seatbelt. That would to need special casing to flag what kind of work is allowed, with checks such as you actually being on board the vehicle and will continue to be able to access the mount point while vehicle continues to move. Seems pretty low utility, and out of scope.

Testing

Absolutely no changes allowed while vehicle is in motion, or aircraft in stationary hover.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Mar 8, 2023
@PatrikLundell
Copy link
Contributor

Please check whether you can modify stationary vehicles that have been towed to their current location. Currently* there is a bug that sets the cruise speed of such vehicles to their max speed (or something like that) resulting in heavy acceleration when you start one unless you remember to immediately reduce their speed to something reasonable. It seems that a check for a non zero cruise speed might fail on such vehicles, resulting in them not being possible to be modified for no apparent reason (from the player's perspective).

  • "Currently" = last time I played with towed vehicles, probably half a year ago.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Mar 8, 2023
@cake-pie
Copy link
Contributor Author

cake-pie commented Mar 8, 2023

Oof, yes that's an inconvenient bug.

@cake-pie cake-pie marked this pull request as draft March 8, 2023 09:41
@PatrikLundell
Copy link
Contributor

I suspect you'd also end up with a non zero target speed if the engine is turned off while moving or otherwise dies (e.g. due to running out of fuel or getting damaged). Note that I haven't verified this is the case, though.

@anothersimulacrum anothersimulacrum self-assigned this Apr 18, 2024
@anothersimulacrum anothersimulacrum added the stale Closed for lack of activity, but still valid. label Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions stale Closed for lack of activity, but still valid.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants