Use item::energy to stash sub-kJ power quantities for more precise po…

…wer usage (#75912)

* Use item::energy to stash sub-kJ power quantities for more precise power usage

* address clang warnings

src/item.cpp

@@ -10883,6 +10883,7 @@ units::energy item::energy_remaining( const Character *carrier ) const
 
     // Battery(ammo) contained within
     if( is_magazine() ) {
+        ret += energy;
         for( const item *e : contents.all_items_top( pocket_type::MAGAZINE ) ) {
             if( e->typeId() == itype_battery ) {
                 ret += units::from_kilojoule( static_cast<std::int64_t>( e->charges ) );
@@ -11086,8 +11087,22 @@ units::energy item::energy_consume( units::energy qty, const tripoint &pos, Char
     if( is_battery() || fuel_efficiency >= 0 ) {
         int consumed_kj = contents.ammo_consume( units::to_kilojoule( qty ), pos, fuel_efficiency );
         qty -= units::from_kilojoule( static_cast<std::int64_t>( consumed_kj ) );
-        // fix negative quantity
-        if( qty < 0_J ) {
+        // Either we're out of juice or truncating the value above means we didn't drain quite enough.
+        // In the latter case at least this will bump up energy enough to satisfy the remainder,
+        // if not it will drain the item all the way.
+        // TODO: reconsider what happens with fuel burning, right now this stashes
+        // the remainder of energy from burning the fuel in the item in question,
+        // which potentially allows it to burn less fuel next time.
+        // Do we want an implicit 1kJ battery in the generator to smooth things out?
+        if( qty > energy ) {
+            int64_t residual_drain = contents.ammo_consume( 1, pos, fuel_efficiency );
+            energy += units::from_kilojoule( residual_drain );
+        }
+        if( qty > energy ) {
+            qty -= energy;
+            energy = 0_J;
+        } else {
+            energy -= qty;
             qty = 0_J;
         }
     }
@@ -11109,13 +11124,6 @@ units::energy item::energy_consume( units::energy qty, const tripoint &pos, Char
         qty -= bio_used;
     }
 
-    // If consumption is not integer kJ we need to consume one extra battery charge to "round up".
-    // Should happen only if battery powered and energy per shot is not integer kJ.
-    if( qty > 0_kJ && is_battery() ) {
-        int consumed_kj = contents.ammo_consume( 1, pos );
-        qty -= units::from_kilojoule( static_cast<std::int64_t>( consumed_kj ) );
-    }
-
     return wanted_energy - qty;
 }
 
@@ -14073,6 +14081,18 @@ bool item::process_wet( Character *carrier, const tripoint & /*pos*/ )
     return true;
 }
 
+units::energy item::energy_per_second() const
+{
+    units::energy energy_to_burn;
+    if( type->tool->turns_per_charge > 0 ) {
+        energy_to_burn += units::from_kilojoule( std::max<int64_t>( ammo_required(),
+                          1 ) ) / type->tool->turns_per_charge;
+    } else if( type->tool->power_draw > 0_mW ) {
+        energy_to_burn += type->tool->power_draw * 1_seconds;
+    }
+    return energy_to_burn;
+}
+
 bool item::process_tool( Character *carrier, const tripoint &pos )
 {
     // FIXME: remove this once power armors don't need to be TOOL_ARMOR anymore
@@ -14082,7 +14102,7 @@ bool item::process_tool( Character *carrier, const tripoint &pos )
 
     // if insufficient available charges shutdown the tool
     if( ( type->tool->turns_per_charge > 0 || type->tool->power_draw > 0_W ) &&
-        ammo_remaining( carrier, true ) == 0 ) {
+        energy_remaining( carrier ) < energy_per_second() ) {
         if( carrier && has_flag( flag_USE_UPS ) ) {
             carrier->add_msg_if_player( m_info, _( "You need an UPS to run the %s!" ), tname() );
         }
@@ -14097,20 +14117,19 @@ bool item::process_tool( Character *carrier, const tripoint &pos )
         }
     }
 
-    int energy = 0;
-    if( type->tool->turns_per_charge > 0 &&
-        to_turn<int>( calendar::turn ) % type->tool->turns_per_charge == 0 ) {
-        energy = std::max( ammo_required(), 1 );
-    } else if( type->tool->power_draw > 0_W ) {
-        // kJ (battery unit) per second
-        energy = units::to_kilowatt( type->tool->power_draw );
-        // energy_bat remainder results in chance at additional charge/discharge
-        const int kw_in_mw = units::to_milliwatt( 1_kW );
-        energy += x_in_y( units::to_milliwatt( type->tool->power_draw ) % kw_in_mw, kw_in_mw ) ? 1 : 0;
-    }
+    if( energy_remaining( carrier ) > 0_J ) {
+        energy_consume( energy_per_second(), pos, carrier );
+    } else {
+        // Non-electrical charge consumption.
+        int charges_to_use = 0;
+        if( type->tool->turns_per_charge > 0 &&
+            to_turn<int>( calendar::turn ) % type->tool->turns_per_charge == 0 ) {
+            charges_to_use = std::max( ammo_required(), 1 );
+        }
 
-    if( energy > 0 ) {
-        ammo_consume( energy, pos, carrier );
+        if( charges_to_use > 0 ) {
+            ammo_consume( charges_to_use, pos, carrier );
+        }
     }
 
     type->tick( carrier, *this, pos );

src/item.h

@@ -2458,6 +2458,7 @@ class item : public visitable
 
 
     private:
+        units::energy energy_per_second() const;
         int ammo_remaining( const std::set<ammotype> &ammo, const Character *carrier = nullptr,
                             bool include_linked = false ) const;
     public:

tests/active_item_test.cpp

@@ -6,6 +6,7 @@
 #include "calendar.h"
 #include "cata_catch.h"
 #include "item.h"
+#include "itype.h"
 #include "map.h"
 #include "map_helpers.h"
 #include "player_helpers.h"
@@ -42,3 +43,30 @@ TEST_CASE( "active_items_processed_regularly", "[active_item]" )
     CHECK( player_character.get_wielded_item()->typeId().str() == "chainsaw_off" );
     CHECK( here.i_at( player_character.pos_bub() ).only_item().typeId().str() == "chainsaw_off" );
 }
+
+TEST_CASE( "tool_power_consumption_rate", "[active_item]" )
+{
+    // Give the flashlight a fully charged battery, 56 kJ
+    item test_battery( "medium_battery_cell" );
+    test_battery.ammo_set( test_battery.ammo_default(), 56 );
+    REQUIRE( test_battery.energy_remaining() == 56_kJ );
+
+    item tool( "flashlight_on" );
+    tool.put_in( test_battery, pocket_type::MAGAZINE_WELL );
+    REQUIRE( tool.energy_remaining() == 56_kJ );
+    tool.active = true;
+
+    // Now process the tool until it runs out of battery power, which should be about 10h.
+    int seconds_of_discharge = 0;
+    map &here = get_map();
+    // Capture now because after the loop the tool will be an inactive tool with no power draw.
+    units::energy minimum_energy = tool.type->tool->power_draw * 1_seconds;
+    do {
+        tool.process( here, nullptr, tripoint_zero );
+        seconds_of_discharge++;
+    } while( tool.active );
+    REQUIRE( tool.energy_remaining() < minimum_energy );
+    // Just a loose check, 9 - 10 hours runtime, based on the product page.
+    CHECK( seconds_of_discharge > to_seconds<int>( 9_hours + 30_minutes ) );
+    CHECK( seconds_of_discharge < to_seconds<int>( 10_hours ) );
+}

tests/ammo_test.cpp

@@ -237,12 +237,22 @@ TEST_CASE( "battery_energy_test", "[ammo][energy][item]" )
     }
 
     SECTION( "Non-integer drain from battery" ) {
-        // Battery charge is in chunks of kj. Non integer kj drain is rounded up.
-        // 4.5 kJ drain becomes 5 kJ drain
+        // Battery charge is in mJ now, so check for precise numbers.
+        // 4.5 kJ drain is 4.5 kJ drain
         REQUIRE( test_battery.energy_remaining( nullptr ) == 56_kJ );
         units::energy consumed = test_battery.energy_consume( 4500_J, tripoint_zero, nullptr );
-        CHECK( test_battery.energy_remaining( nullptr ) == 51_kJ );
-        CHECK( consumed == 5_kJ );
+        CHECK( test_battery.energy_remaining( nullptr ) == 51.5_kJ );
+        CHECK( consumed == 4500_J );
+    }
+
+    SECTION( "Tiny Non-integer drain from battery" ) {
+        // Make sure lots of tiny discharges sum up as expected.
+        REQUIRE( test_battery.energy_remaining( nullptr ) == 56_kJ );
+        for( int i = 0; i < 133; ++i ) {
+            units::energy consumed = test_battery.energy_consume( 2_J, tripoint_zero, nullptr );
+            CHECK( consumed == 2_J );
+        }
+        CHECK( test_battery.energy_remaining( nullptr ) == 55734_J );
     }
 
     SECTION( "Non-integer over-drain from battery" ) {