Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.
Sign up[WIP] Add physics calculations for vehicles. #10666
Conversation
This comment has been minimized.
This comment has been minimized.
|
This looks... impressive. |
BevapDin
reviewed
Dec 29, 2014
| to running. With the canon values a human uses 6 seconds to run 1 | ||
| meter, which is ridiculously slow. To compensate, using the values | ||
| below slows down vehicles by a factor of 18. */ | ||
| /* The size (width) of a single game tile in mm, 1 meter is canon */ |
This comment has been minimized.
This comment has been minimized.
BevapDin
Dec 29, 2014
Contributor
"mm" as in Millimeter? Why is it named _M, and the value 3 does not match either.
This comment has been minimized.
This comment has been minimized.
vidarsk
Dec 30, 2014
Author
Contributor
Thanks for the catch. mm is incorrect, the comment should read "meters",
I'll update it.
6 seconds/1 meter is canonical, but I can't use those values for the
reasons I tried to explain in the comments above. 10mph = 4.4704
meters/second. With turn_time at 6 seconds, you'd drive 27 meters per turn,
or 27 tiles. 27 tiles per game turn at 10mph does not fit into the current
game setup. Walking/running speed in cataclysm has no basis in reality, and
therefore I need to fudge the values to make proper physics fit.
On Mon, Dec 29, 2014 at 4:42 PM, BevapDin notifications@github.com wrote:
In src/vehicle.h
#10666 (diff)
:@@ -22,6 +22,16 @@ float get_collision_factor(float delta_v);
//How far to scatter parts from a vehicle when the part is destroyed (+/-)
#define SCATTER_DISTANCE 3+/* These values are changed since otherwise vehicle movement (which uses
- them as the base for calculating speed) becomes too fast compared
- to running. With the canon values a human uses 6 seconds to run 1
- meter, which is ridiculously slow. To compensate, using the values
- below slows down vehicles by a factor of 18. /
+/ The size (width) of a single game tile in mm, 1 meter is canon */"mm" as in Millimeter? Why is it named _M, and the value 3 does not match
either.—
Reply to this email directly or view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/pull/10666/files#r22315454.
- Vidar
BevapDin
reviewed
Dec 29, 2014
| @@ -0,0 +1,105 @@ | |||
| #ifndef _VEH_PHYSICS_H_ | |||
| #define _VEH_PHYSICS_H_ | |||
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
vidarsk
Dec 30, 2014
Author
Contributor
Thanks, fixed. I copied it from vehicles.h 9 months ago and didn't know
about the change.
On Mon, Dec 29, 2014 at 4:46 PM, BevapDin notifications@github.com wrote:
In src/veh_physics.h
#10666 (diff)
:@@ -0,0 +1,105 @@
+#ifndef VEH_PHYSICS_H
+#define VEH_PHYSICS_HNo leading underscore please, see #8415
#8415—
Reply to this email directly or view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/pull/10666/files#r22315630.
- Vidar
Coolthulhu
reviewed
Dec 29, 2014
| velocity_end = 0.0; | ||
| if( distance_traveled < 0.001 || t >= TURN_TIME_S || t <= 0.001 || velocity_end == 0.0 ) { | ||
| time_taken = TURN_TIME_S; | ||
| break; /* For gameplay purposes you can always travel 1 tile in a turn */ |
This comment has been minimized.
This comment has been minimized.
Coolthulhu
Dec 29, 2014
Contributor
I'm not sure if I get it, but I hope I just don't.
1 tile in a turn is (about?) 10 mph. Are you rounding up speed below 10 mph to 10 mph?
This comment has been minimized.
This comment has been minimized.
vidarsk
Dec 30, 2014
Author
Contributor
Yes, I am rounding it up. This should be easy to change. The main reason
for having a minimum speed is that I wasn't sure how all possible vehicle
situations would work. What if you drive a heavy vehicle onto broken
terrain and become unable to drive it away from the terrain again? Having
realistic physics here might make the game very unfun. This was not a
problem with the old system since it didn't consider friction or terrain,
so you couldn't become stuck in the same way. I'll add a TODO to see if I
can remove the rounding up (or at least use a lower value) once the whole
vehicle physics rework is done, before I make a proper pull request. Due to
the way the other vehicle systems work (gas usage, traps under wheels,
movement system) it'll be easier to have 1 tile/turn as a minimum until
everything else is completed.
On Mon, Dec 29, 2014 at 8:32 PM, Coolthulhu notifications@github.com
wrote:
In src/veh_physics.cpp
#10666 (diff)
:
if ( !((v_dir > 0.0) ^ (velocity_end < 0.0)) ) { /\* XAND *//\* Don't go directly from forward to reverse */kinetic_end = 0.0;velocity_end = 0.0;}/\* Update some other values as well now that we know speed etc */velocity_average = (velocity_start + velocity_end) / 2;engine_watt_average = newton_average \* velocity_average;distance_traveled = fabs(velocity_average \* t); /\* No negatives *//\* More closely match t to actual time taken, for use in next iteration */t = t \* TILE_SIZE_M / distance_traveled;if( fabs(velocity_end) < 0.1 )velocity_end = 0.0;if( distance_traveled < 0.001 || t >= TURN_TIME_S || t <= 0.001 || velocity_end == 0.0 ) {time_taken = TURN_TIME_S;break; /\* For gameplay purposes you can always travel 1 tile in a turn */I'm not sure if I get it, but I hope I just don't.
1 tile in a turn is (about?) 10 mph. Are you rounding up speed below 10
mph to 10 mph?—
Reply to this email directly or view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/pull/10666/files#r22324914.
- Vidar
This comment has been minimized.
This comment has been minimized.
kevingranade
Dec 30, 2014
Member
IMO the situation is the opposite, if there's a super coarse override you need to make the system work, it indicates a problem with the system, and if you're making compromises based on 'this wouldn't be fun if it were realistic', why bother to simulate it in such detail in the first place?
IMO getting stuck because your traction/horsepower is too low is definitely something that should happen sometimes.
This comment has been minimized.
This comment has been minimized.
vidarsk
Dec 30, 2014
Author
Contributor
Oki, I'll look into it before I submit a proper pull request.
On Tue, Dec 30, 2014 at 1:55 AM, Kevin Granade notifications@github.com
wrote:
In src/veh_physics.cpp
#10666 (diff)
:
if ( !((v_dir > 0.0) ^ (velocity_end < 0.0)) ) { /\* XAND *//\* Don't go directly from forward to reverse */kinetic_end = 0.0;velocity_end = 0.0;}/\* Update some other values as well now that we know speed etc */velocity_average = (velocity_start + velocity_end) / 2;engine_watt_average = newton_average \* velocity_average;distance_traveled = fabs(velocity_average \* t); /\* No negatives *//\* More closely match t to actual time taken, for use in next iteration */t = t \* TILE_SIZE_M / distance_traveled;if( fabs(velocity_end) < 0.1 )velocity_end = 0.0;if( distance_traveled < 0.001 || t >= TURN_TIME_S || t <= 0.001 || velocity_end == 0.0 ) {time_taken = TURN_TIME_S;break; /\* For gameplay purposes you can always travel 1 tile in a turn */IMO the situation is the opposite, if there's a super coarse override you
need to make the system work, it indicates a problem with the system, and
if you're making compromises based on 'this wouldn't be fun if it were
realistic', why bother to simulate it in such detail in the first place?
IMO getting stuck because your traction/horsepower is too low is
definitely something that should happen sometimes.—
Reply to this email directly or view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/pull/10666/files#r22335501.
- Vidar
kevingranade
reviewed
Dec 30, 2014
| #include "vehicle.h" | ||
| #include <vector> | ||
| #include <string> | ||
|
|
This comment has been minimized.
This comment has been minimized.
kevingranade
Dec 30, 2014
Member
what is a 'vehicle physics'? If you're encapsulating something, you need to make it very clear what it is you're encapsulating, and this doesn't do that.
This comment has been minimized.
This comment has been minimized.
vidarsk
Dec 30, 2014
Author
Contributor
It's all the physics variables that are required to calculate vehicle
movement and collision. They are transient and are also used speculative
(such as how fast would this car drive if we accelerated as fast as we
could for 10 seconds, or does the physics match up if we estimate that it
takes 2 seconds at 60% engine speed to drive 3 meters). How do I make it
clear what I'm encapsulating, are you thinking of adding a comment block to
veh_physics.h or something else?
On Tue, Dec 30, 2014 at 1:49 AM, Kevin Granade notifications@github.com
wrote:
In src/veh_physics.h
#10666 (diff)
:@@ -0,0 +1,105 @@
+#ifndef VEH_PHYSICS_H
+#define VEH_PHYSICS_H
+
+#include "vehicle.h"
+#include
+#include
+what is a 'vehicle physics'? If you're encapsulating something, you need
to make it very clear what it is you're encapsulating, and this doesn't do
that.—
Reply to this email directly or view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/pull/10666/files#r22335382.
- Vidar
kevingranade
reviewed
Dec 30, 2014
| /** | ||
| * These values are used when accelerating/braking and in the examine screen | ||
| * The first letters of the comments shows where the value is used: | ||
| * i=internal, basic properties of vehicle, copied from vehicle, setup in constructor |
This comment has been minimized.
This comment has been minimized.
kevingranade
Dec 30, 2014
Member
why ever copy values from the vehicle? just use them from the vehicle.
This comment has been minimized.
This comment has been minimized.
vidarsk
Dec 30, 2014
Author
Contributor
total_mass() loops through every part and item stored in that part, I don't
want to do that 20 times while calculating stuff. The others are less
intensive, but still worth caching.
On Tue, Dec 30, 2014 at 1:51 AM, Kevin Granade notifications@github.com
wrote:
In src/veh_physics.h
#10666 (diff)
:
+public:
- veh_physics(vehicle *v) : veh(v) {
mass = veh->total_mass();eng_pwr_cur = veh->total_power();eng_pwr_max = veh->total_power(false);engine_watt_avail = veh->engine_on ? double(veh->power_to_epower(eng_pwr_cur)) : 0.0;valid_wheel_config = veh->valid_wheel_config(); /\* Cache this */tire_friction_coeff = 0.7;ground_res_coeff = 0.01;- }
- /**
\* These values are used when accelerating/braking and in the examine screen\* The first letters of the comments shows where the value is used:\* i=internal, basic properties of vehicle, copied from vehicle, setup in constructorwhy ever copy values from the vehicle? just use them from the vehicle.
—
Reply to this email directly or view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/pull/10666/files#r22335432.
- Vidar
kevingranade
reviewed
Dec 30, 2014
| to running. With the canon values a human uses 6 seconds to run 1 | ||
| meter, which is ridiculously slow. To compensate, using the values | ||
| below slows down vehicles by a factor of 18. */ | ||
| /* The size (width) of a single game tile in meters, 1 meter is canon */ |
This comment has been minimized.
This comment has been minimized.
kevingranade
Dec 30, 2014
Member
Please remove these comments, there is no cannonical width for a tile, and I don't want people to start thinking there is.
This comment has been minimized.
This comment has been minimized.
vidarsk
Dec 30, 2014
Author
Contributor
Oki. Rewrote the comments, maybe I succeeded in making it clearer now.
Sorry for all the confusion surrounding this, explanations are not my thing.
On Tue, Dec 30, 2014 at 1:57 AM, Kevin Granade notifications@github.com
wrote:
In src/vehicle.h
#10666 (diff)
:@@ -22,6 +22,16 @@ float get_collision_factor(float delta_v);
//How far to scatter parts from a vehicle when the part is destroyed (+/-)
#define SCATTER_DISTANCE 3+/* These values are changed since otherwise vehicle movement (which uses
- them as the base for calculating speed) becomes too fast compared
- to running. With the canon values a human uses 6 seconds to run 1
- meter, which is ridiculously slow. To compensate, using the values
- below slows down vehicles by a factor of 18. /
+/ The size (width) of a single game tile in meters, 1 meter is canon */Please remove these comments, there is no cannonical width for a tile, and
I don't want people to start thinking there is.—
Reply to this email directly or view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/pull/10666/files#r22335527.
- Vidar
kevingranade
reviewed
Dec 30, 2014
| if( engine_watt_avail < 1 ) { | ||
| return 0.0; | ||
| } | ||
| double total_time = 0.0, tile_time; |
This comment has been minimized.
This comment has been minimized.
kevingranade
Dec 30, 2014
Member
one variable declaration per line please, and initialize variables to something sensible.
This comment has been minimized.
This comment has been minimized.
vidarsk
Dec 30, 2014
Author
Contributor
ack, fixed.
On Tue, Dec 30, 2014 at 2:13 AM, Kevin Granade notifications@github.com
wrote:
In src/veh_physics.cpp
#10666 (diff)
:@@ -0,0 +1,191 @@
+#include "veh_physics.h"
+#include "game.h"
+#include "vehicle.h"
+#include "translations.h"
+
+double veh_physics::calculate_acceleration( double target_speed, bool max_power )
+{
- engine_watt_avail = double(veh->power_to_epower(eng_pwr_cur));
- if( max_power ) {
engine_watt_avail = double(veh->power_to_epower(eng_pwr_max));- }
- if( engine_watt_avail < 1 ) {
return 0.0;- }
- double total_time = 0.0, tile_time;
one variable declaration per line please, and initialize variables to
something sensible.—
Reply to this email directly or view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/pull/10666/files#r22335803.
- Vidar
kevingranade
reviewed
Dec 30, 2014
| return 0.0; | ||
| } | ||
| total_time += tile_time; | ||
| if( total_time >= 10.0 ) { |
This comment has been minimized.
This comment has been minimized.
kevingranade
Dec 30, 2014
Member
Why 10 sec? one turn is 6 sec.
also, please keep the interleaving of vehicle movement, it's problematic to do all of one vehicle's movement followed by the next.
This comment has been minimized.
This comment has been minimized.
vidarsk
Dec 30, 2014
Author
Contributor
That function is used to calculate acceleration 0-60mph (or 0-100kph). Some
cars will not be able to reach those speeds ever, so I need a fallback. In
this case I chose the speed it is able to reach after 10 second. I could
use anything here, 100 seconds, 1 second, 0.1 millisecond, it's just used
as a car performance indicator and will be displayed in veh_interact
instead of safe/top speed.
On Tue, Dec 30, 2014 at 2:15 AM, Kevin Granade notifications@github.com
wrote:
In src/veh_physics.cpp
#10666 (diff)
:
- if( engine_watt_avail < 1 ) {
return 0.0;- }
- double total_time = 0.0, tile_time;
- int nr_calcs = 0;
- int old_velocity = veh->velocity;
- veh->velocity = 0;
- do {
tile_time = calculate_time_to_move_one_tile( 1.0 );veh->velocity = int( velocity_end \* 223.6 + 0.5 );if( veh->velocity == 0 ) {veh->velocity = old_velocity;return 0.0;}total_time += tile_time;if( total_time >= 10.0 ) {Why 10 sec? one turn is 6 sec.
also, please keep the interleaving of vehicle movement, it's problematic
to do all of one vehicle's movement followed by the next.—
Reply to this email directly or view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/pull/10666/files#r22335832.
- Vidar
This comment has been minimized.
This comment has been minimized.
kevingranade
Dec 30, 2014
Member
To verify that I'm understanding you, when you bring up the vehicle
interaction screen, you're planning on simulating accelerating from the
current location for 10 seconds in order to determine max speed?
Why not just use a formula?
This comment has been minimized.
This comment has been minimized.
vidarsk
Dec 30, 2014
Author
Contributor
The description from veh_physics.h is:
/**
* Max acceleration mode assumes flat ground and accelerates (from a
full stop) as fast as
* possible across multiple tiles (max 1000 tiles or 10 secs) until
target_speed is reached.
* Used by veh_interact to display vehicle stats.
* @param target_speed Target speed to accelerate to in m/s
* @param max_power Assume all engines are 100% health and fuelled
* @return Time taken to reach target_speed, if negative it's speed
reached after 10 secs instead
*/
double calculate_acceleration( double target_speed, bool max_power =
false );
So it doesn't use the current location but a perfect road, sorta like the
0-100 times you'd see on an info page about a car.
I don't use a formula because trying to integrate all the variables
correctly was beyond my skills and would anyways have resulted in an
incomprehensible monstrosity that no mere mortal would be able to debug or
improve (I could be wrong here, maybe some physics wizard could actually
make a simple formula, but I have my doubts).
On Tue, Dec 30, 2014 at 3:50 AM, Kevin Granade notifications@github.com
wrote:
In src/veh_physics.cpp
#10666 (diff)
:
- if( engine_watt_avail < 1 ) {
return 0.0;- }
- double total_time = 0.0, tile_time;
- int nr_calcs = 0;
- int old_velocity = veh->velocity;
- veh->velocity = 0;
- do {
tile_time = calculate_time_to_move_one_tile( 1.0 );veh->velocity = int( velocity_end \* 223.6 + 0.5 );if( veh->velocity == 0 ) {veh->velocity = old_velocity;return 0.0;}total_time += tile_time;if( total_time >= 10.0 ) {To verify that I'm understanding you, when you bring up the vehicle
interaction screen, you're planning on simulating accelerating from the
current location for 10 seconds in order to determine max speed? Why not
just use a formula?—
Reply to this email directly or view it on GitHub
https://github.com/CleverRaven/Cataclysm-DDA/pull/10666/files#r22337439.
- Vidar
vidarsk
changed the title
[WIP] Add physics calculations for vehicles. Not actually used anywhere yet.
[WIP] Add physics calculations for vehicles.
Dec 30, 2014
This comment has been minimized.
This comment has been minimized.
|
This PR will probably not become mergeable. Closing. |
vidarsk commentedDec 29, 2014
No description provided.