Make electric motors go in reverse better (#38212)

* Make electric motors go in reverse better

Due to the downscaling of reverse power/speed for vehicle engines, it
can happen that a vehicle is so heavy, it can move forward but not
backward.

Because battery-powered electric motors can turn in either direction
with equal speed and torque, vehicles with electric motors should be
exempt from the power-reduction for reverse speeds.

This commit adds a new `vehicle::max_reverse_velocity` function as the
counterpart to `vehicle::max_velocity`, where an exception is made for
battery-powered motors. Here I suppose is where the maximum reverse
speed multiplier for horses, bicycles, and jet turbines would eventually
be made; for now it just checks for battery-powered motors.

Note, how *much* the motor is contributing to overall torque is not
calculated; the simple presence of any electric motor in the drivetrain
(no matter how small) ought to negate the reverse-power penalty.

Fixed #38165

* Refactor vehicle_power_test.cpp for readability

This is mainly preliminary work for adding new test cases to cover the
changes I have made to maximum reverse speed; this vehicle power test
seemed like a suitable place to put them.

I had trouble understanding what this test was doing; refactoring it
into BDD-style expressions helped me grok it while making it a whole lot
easier for others to read in the future.

* Add vehicle reverse velocity tests

To minimally test the new `max_reverse_velocity` function, this commit
adds two test vehicles (scooter and electric scooter), and checks that:

- Combustion engine reverse velocity is 1/4 of forward
- Electric engine reverse velocity is equivalent to forward

Some additional refactoring of the helper functions is also done.

data/json/vehicles/test.json

@@ -182,5 +182,32 @@
       { "x": 1, "y": 3, "parts": [ "frame_vertical", "solar_panel" ] },
       { "x": 0, "y": -1, "parts": [ "frame_vertical", "seat", "battery_car" ] }
     ]
+  },
+  {
+    "id": "scooter_test",
+    "type": "vehicle",
+    "name": "TEST Scooter",
+    "blueprint": [ "o>o" ],
+    "parts": [
+      { "x": 0, "y": 0, "parts": [ "frame_handle", "headlight", "saddle", "controls", "controls_electronic" ] },
+      { "x": 0, "y": 0, "parts": [ "horn_car", "motorcycle_kickstand", "engine_1cyl", "alternator_motorbike" ] },
+      { "x": 0, "y": 0, "parts": [ "battery_motorbike", { "part": "tank_small", "fuel": "gasoline" } ] },
+      { "x": 1, "y": 0, "parts": [ "xlframe_vertical", "wheel_mount_light_steerable", "wheel_small" ] },
+      { "x": -1, "y": 0, "parts": [ "xlframe_vertical", "wheel_mount_light", "wheel_small" ] },
+      { "x": -1, "y": 0, "part": "muffler" }
+    ]
+  },
+  {
+    "id": "scooter_electric_test",
+    "type": "vehicle",
+    "name": "TEST Electric Scooter",
+    "blueprint": [ "o>o" ],
+    "parts": [
+      { "x": 0, "y": 0, "parts": [ "frame_handle", "headlight", "saddle", "controls" ] },
+      { "x": 0, "y": 0, "parts": [ "controls_electronic", "horn_car", "motorcycle_kickstand" ] },
+      { "x": 0, "y": 0, "parts": [ "medium_storage_battery", "engine_electric_small" ] },
+      { "x": 1, "y": 0, "parts": [ "xlframe_vertical", "wheel_mount_light_steerable", "wheel_small" ] },
+      { "x": -1, "y": 0, "parts": [ "xlframe_vertical", "wheel_mount_light", "wheel_small" ] }
+    ]
   }
 ]

src/vehicle.cpp

@@ -3543,6 +3543,18 @@ int vehicle::max_velocity( const bool fueled ) const
     return is_watercraft() ? max_water_velocity( fueled ) : max_ground_velocity( fueled );
 }
 
+int vehicle::max_reverse_velocity( const bool fueled ) const
+{
+    int max_vel = max_velocity( fueled );
+    if( has_engine_type( fuel_type_battery, true ) ) {
+        // Electric motors can go in reverse as well as forward
+        return -max_vel;
+    } else {
+        // All other motive powers do poorly in reverse
+        return -max_vel / 4;
+    }
+}
+
 // the same physics as max_ground_velocity, but with a smaller engine power
 int vehicle::safe_ground_velocity( const bool fueled ) const
 {

src/vehicle.h

@@ -1198,6 +1198,8 @@ class vehicle
         int max_water_velocity( bool fueled = true ) const;
         // Get maximum velocity for the current movement mode
         int max_velocity( bool fueled = true ) const;
+        // Get maximum reverse velocity for the current movement mode
+        int max_reverse_velocity( bool fueled = true ) const;
 
         // Get safe ground velocity gained by combined power of all engines.
         // If fueled == true, then only the engines which the vehicle has fuel for are included

src/vehicle_move.cpp

@@ -159,8 +159,8 @@ void vehicle::thrust( int thd )
 
     //pos or neg if accelerator or brake
     int vel_inc = ( ( thrusting ) ? accel : brk ) * thd;
-    if( thd == -1 && thrusting ) {
-        //accelerate 60% if going backward
+    // Reverse is only 60% acceleration, unless an electric motor is in use
+    if( thd == -1 && thrusting && !has_engine_type( fuel_type_battery, true ) ) {
         vel_inc = .6 * vel_inc;
     }
 
@@ -225,7 +225,7 @@ void vehicle::thrust( int thd )
         stop();
     } else {
         // Increase velocity up to max_vel or min_vel, but not above.
-        const int min_vel = -max_vel / 4;
+        const int min_vel = max_reverse_velocity();
         if( vel_inc > 0 ) {
             // Don't allow braking by accelerating (could happen with damaged engines)
             velocity = std::max( velocity, std::min( velocity + vel_inc, max_vel ) );
@@ -257,7 +257,7 @@ void vehicle::cruise_thrust( int amount )
     }
     int safe_vel = safe_velocity();
     int max_vel = max_velocity();
-    int max_rev_vel = -max_vel / 4;
+    int max_rev_vel = max_reverse_velocity();
 
     //if the safe velocity is between the cruise velocity and its next value, set to safe velocity
     if( ( cruise_velocity < safe_vel && safe_vel < ( cruise_velocity + amount ) ) ||

tests/vehicle_power_test.cpp

@@ -16,75 +16,193 @@
 
 static const itype_id fuel_type_battery( "battery" );
 static const itype_id fuel_type_plut_cell( "plut_cell" );
+static const efftype_id effect_blind( "blind" );
 
-TEST_CASE( "vehicle_power" )
+static void reset_player()
 {
-    GIVEN( "Reactor and solar panels" ) {
-        for( const tripoint &p : g->m.points_in_rectangle( tripoint_zero,
-                tripoint( MAPSIZE * SEEX, MAPSIZE * SEEY, 0 ) ) ) {
-            g->m.furn_set( p, furn_id( "f_null" ) );
-            g->m.ter_set( p, ter_id( "t_pavement" ) );
-            g->m.trap_set( p, trap_id( "tr_null" ) );
-            g->m.i_clear( p );
-        }
+    // Move player somewhere safe
+    REQUIRE( !g->u.in_vehicle );
+    g->u.setpos( tripoint_zero );
+    // Blind the player to avoid needless drawing-related overhead
+    g->u.add_effect( effect_blind, 1_turns, num_bp, true );
+}
 
-        g->m.invalidate_map_cache( 0 );
-        g->m.build_map_cache( 0, true );
-
-        CHECK( !g->u.in_vehicle );
-        const tripoint test_origin( 15, 15, 0 );
-        g->u.setpos( test_origin );
-        const tripoint vehicle_origin = tripoint( 10, 10, 0 );
-        VehicleList vehs = g->m.get_vehicles();
-        vehicle *veh_ptr;
-        for( auto &vehs_v : vehs ) {
-            veh_ptr = vehs_v.v;
-            g->m.destroy_vehicle( veh_ptr );
-        }
-        g->refresh_all();
-        REQUIRE( g->m.get_vehicles().empty() );
-        veh_ptr = g->m.add_vehicle( vproto_id( "reactor_test" ), vehicle_origin, 0, 0, 0 );
+// Build a map of size MAPSIZE_X x MAPSIZE_Y around tripoint_zero with a given
+// terrain, and no furniture, traps, or items.
+static void build_test_map( const ter_id &terrain )
+{
+    for( const tripoint &p : g->m.points_in_rectangle( tripoint_zero,
+            tripoint( MAPSIZE * SEEX, MAPSIZE * SEEY, 0 ) ) ) {
+        g->m.furn_set( p, furn_id( "f_null" ) );
+        g->m.ter_set( p, terrain );
+        g->m.trap_set( p, trap_id( "tr_null" ) );
+        g->m.i_clear( p );
+    }
+
+    g->m.invalidate_map_cache( 0 );
+    g->m.build_map_cache( 0, true );
+}
+
+static void remove_all_vehicles()
+{
+    VehicleList vehs = g->m.get_vehicles();
+    vehicle *veh_ptr;
+    for( auto &vehs_v : vehs ) {
+        veh_ptr = vehs_v.v;
+        g->m.destroy_vehicle( veh_ptr );
+    }
+    REQUIRE( g->m.get_vehicles().empty() );
+}
+
+TEST_CASE( "vehicle power with reactor and solar panels", "[vehicle][power]" )
+{
+    reset_player();
+    build_test_map( ter_id( "t_pavement" ) );
+    remove_all_vehicles();
+
+    SECTION( "vehicle with reactor" ) {
+        const tripoint reactor_origin = tripoint( 10, 10, 0 );
+        vehicle *veh_ptr = g->m.add_vehicle( vproto_id( "reactor_test" ), reactor_origin, 0, 0, 0 );
         REQUIRE( veh_ptr != nullptr );
         g->refresh_all();
+
         REQUIRE( !veh_ptr->reactors.empty() );
         vehicle_part &reactor = veh_ptr->parts[ veh_ptr->reactors.front() ];
-        reactor.ammo_unset();
-        veh_ptr->discharge_battery( veh_ptr->fuel_left( fuel_type_battery ) );
-        REQUIRE( veh_ptr->fuel_left( fuel_type_battery ) == 0 );
-        reactor.ammo_set( fuel_type_plut_cell, 1 );
-        REQUIRE( reactor.ammo_remaining() == 1 );
-        veh_ptr->power_parts();
-        CHECK( reactor.ammo_remaining() == 0 );
-        CHECK( veh_ptr->fuel_left( fuel_type_battery ) == 100 );
-        g->m.destroy_vehicle( veh_ptr );
-        g->refresh_all();
-        REQUIRE( g->m.get_vehicles().empty() );
+
+        GIVEN( "the reactor is empty" ) {
+            reactor.ammo_unset();
+            veh_ptr->discharge_battery( veh_ptr->fuel_left( fuel_type_battery ) );
+            REQUIRE( veh_ptr->fuel_left( fuel_type_battery ) == 0 );
+
+            WHEN( "the reactor is loaded with plutonium fuel" ) {
+                reactor.ammo_set( fuel_type_plut_cell, 1 );
+                REQUIRE( reactor.ammo_remaining() == 1 );
+
+                AND_WHEN( "the reactor is used to power and charge the battery" ) {
+                    veh_ptr->power_parts();
+                    THEN( "the reactor should be empty, and the battery should be charged" ) {
+                        CHECK( reactor.ammo_remaining() == 0 );
+                        CHECK( veh_ptr->fuel_left( fuel_type_battery ) == 100 );
+                    }
+                }
+            }
+        }
+    }
+
+    SECTION( "vehicle with solar panels" ) {
         const tripoint solar_origin = tripoint( 5, 5, 0 );
-        veh_ptr = g->m.add_vehicle( vproto_id( "solar_panel_test" ), solar_origin, 0, 0, 0 );
+        vehicle *veh_ptr = g->m.add_vehicle( vproto_id( "solar_panel_test" ), solar_origin, 0, 0, 0 );
+        REQUIRE( veh_ptr != nullptr );
+        g->refresh_all();
+
+        GIVEN( "it is 3 hours after sunrise, with sunny weather" ) {
+            calendar::turn = calendar::turn_zero + calendar::season_length() + 1_days;
+            const time_point start_time = sunrise( calendar::turn ) + 3_hours;
+            veh_ptr->update_time( start_time );
+            g->weather.weather_override = WEATHER_SUNNY;
+
+            AND_GIVEN( "the battery has no charge" ) {
+                veh_ptr->discharge_battery( veh_ptr->fuel_left( fuel_type_battery ) );
+                REQUIRE( veh_ptr->fuel_left( fuel_type_battery ) == 0 );
+
+                WHEN( "30 minutes elapse" ) {
+                    veh_ptr->update_time( start_time + 30_minutes );
+
+                    THEN( "the battery should be partially charged" ) {
+                        int charge = veh_ptr->fuel_left( fuel_type_battery ) / 100;
+                        CHECK( 10 <= charge );
+                        CHECK( charge <= 15 );
+
+                        AND_WHEN( "another 30 minutes elapse" ) {
+                            veh_ptr->update_time( start_time + 2 * 30_minutes );
+
+                            THEN( "the battery should be further charged" ) {
+                                charge = veh_ptr->fuel_left( fuel_type_battery ) / 100;
+                                CHECK( 20 <= charge );
+                                CHECK( charge <= 30 );
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        GIVEN( "it is 3 hours after sunset, with clear weather" ) {
+            const time_point at_night = sunset( calendar::turn ) + 3_hours;
+            g->weather.weather_override = WEATHER_CLEAR;
+            veh_ptr->update_time( at_night );
+
+            AND_GIVEN( "the battery has no charge" ) {
+                veh_ptr->discharge_battery( veh_ptr->fuel_left( fuel_type_battery ) );
+                REQUIRE( veh_ptr->fuel_left( fuel_type_battery ) == 0 );
+
+                WHEN( "60 minutes elapse" ) {
+                    veh_ptr->update_time( at_night + 2 * 30_minutes );
+
+                    THEN( "the battery should still have no charge" ) {
+                        CHECK( veh_ptr->fuel_left( fuel_type_battery ) == 0 );
+                    }
+                }
+            }
+        }
+    }
+}
+
+TEST_CASE( "maximum reverse velocity", "[vehicle][power][reverse]" )
+{
+    reset_player();
+    build_test_map( ter_id( "t_pavement" ) );
+    remove_all_vehicles();
+
+    GIVEN( "a scooter with combustion engine and charged battery" ) {
+        const tripoint origin = tripoint( 10, 0, 0 );
+        vehicle *veh_ptr = g->m.add_vehicle( vproto_id( "scooter_test" ), origin, 0, 0, 0 );
+        REQUIRE( veh_ptr != nullptr );
+        g->refresh_all();
+        veh_ptr->charge_battery( 500 );
+        REQUIRE( veh_ptr->fuel_left( fuel_type_battery ) == 500 );
+
+        WHEN( "the engine is started" ) {
+            veh_ptr->start_engines();
+
+            THEN( "it can go in both forward and reverse" ) {
+                int max_fwd = veh_ptr->max_velocity( false );
+                int max_rev = veh_ptr->max_reverse_velocity( false );
+
+                CHECK( max_rev < 0 );
+                CHECK( max_fwd > 0 );
+
+                AND_THEN( "its maximum reverse velocity is 1/4 of the maximum forward velocity" ) {
+                    CHECK( std::abs( max_fwd / max_rev ) == 4 );
+                }
+            }
+
+        }
+    }
+
+    GIVEN( "a scooter with an electric motor and charged battery" ) {
+        const tripoint origin = tripoint( 15, 0, 0 );
+        vehicle *veh_ptr = g->m.add_vehicle( vproto_id( "scooter_electric_test" ), origin, 0, 0, 0 );
         REQUIRE( veh_ptr != nullptr );
         g->refresh_all();
-        calendar::turn = calendar::turn_zero + calendar::season_length() + 1_days;
-        const time_point start_time = sunrise( calendar::turn ) + 3_hours;
-        veh_ptr->update_time( start_time );
-        veh_ptr->discharge_battery( veh_ptr->fuel_left( fuel_type_battery ) );
-        REQUIRE( veh_ptr->fuel_left( fuel_type_battery ) == 0 );
-        g->weather.weather_override = WEATHER_SUNNY;
-        veh_ptr->update_time( start_time + 30_minutes );
-        int approx_battery1 = veh_ptr->fuel_left( fuel_type_battery ) / 100;
-        const int exp_min = 10;
-        const int exp_max = 15;
-        CHECK( approx_battery1 >= exp_min );
-        CHECK( approx_battery1 <= exp_max );
-        veh_ptr->update_time( start_time + 2 * 30_minutes );
-        int approx_battery2 = veh_ptr->fuel_left( fuel_type_battery ) / 100;
-        CHECK( approx_battery2 >= approx_battery1 + exp_min );
-        CHECK( approx_battery2 <= approx_battery1 + exp_max );
-        const time_point at_night = sunset( calendar::turn ) + 3_hours;
-        g->weather.weather_override = WEATHER_CLEAR;
-        veh_ptr->update_time( at_night );
-        veh_ptr->discharge_battery( veh_ptr->fuel_left( fuel_type_battery ) );
-        REQUIRE( veh_ptr->fuel_left( fuel_type_battery ) == 0 );
-        veh_ptr->update_time( at_night + 30_minutes );
-        CHECK( veh_ptr->fuel_left( fuel_type_battery ) == 0 );
+        veh_ptr->charge_battery( 5000 );
+        REQUIRE( veh_ptr->fuel_left( fuel_type_battery ) == 5000 );
+
+        WHEN( "the engine is started" ) {
+            veh_ptr->start_engines();
+
+            THEN( "it can go in both forward and reverse" ) {
+                int max_fwd = veh_ptr->max_velocity( false );
+                int max_rev = veh_ptr->max_reverse_velocity( false );
+
+                CHECK( max_rev < 0 );
+                CHECK( max_fwd > 0 );
+
+                AND_THEN( "its maximum reverse velocity is equal to maximum forward velocity" ) {
+                    CHECK( std::abs( max_rev ) == std::abs( max_fwd ) );
+                }
+            }
+        }
     }
 }
+