diff --git a/src/activity_actor.cpp b/src/activity_actor.cpp index c9ee3e9951a01..4bad4361952e1 100644 --- a/src/activity_actor.cpp +++ b/src/activity_actor.cpp @@ -1761,6 +1761,10 @@ std::unique_ptr unload_activity_actor::deserialize( JsonIn &jsin craft_activity_actor::craft_activity_actor( item_location &it, const bool is_long ) : craft_item( it ), is_long( is_long ) { + cached_crafting_speed = 0; + cached_assistants = 0; + cached_base_total_moves = 1; + cached_cur_total_moves = 1; } bool craft_activity_actor::check_if_craft_okay( item_location &craft_item, Character &crafter ) @@ -1796,6 +1800,7 @@ void craft_activity_actor::start( player_activity &act, Character &crafter ) } act.moves_left = calendar::INDEFINITELY_LONG; activity_override = craft_item.get_item()->get_making().exertion_level(); + cached_crafting_speed = 0; } void craft_activity_actor::do_turn( player_activity &act, Character &crafter ) @@ -1819,18 +1824,26 @@ void craft_activity_actor::do_turn( player_activity &act, Character &crafter ) return; } + if( cached_crafting_speed != crafting_speed || cached_assistants != assistants ) { + cached_crafting_speed = crafting_speed; + cached_assistants = assistants; + + // Base moves for batch size with no speed modifier or assistants + // Must ensure >= 1 so we don't divide by 0; + cached_base_total_moves = std::max( static_cast( 1 ), + rec.batch_time( crafter, craft.charges, 1.0f, 0 ) ); + // Current expected total moves, includes crafting speed modifiers and assistants + cached_cur_total_moves = std::max( static_cast( 1 ), + rec.batch_time( crafter, craft.charges, crafting_speed, + assistants ) ); + } + const double base_total_moves = cached_base_total_moves; + const double cur_total_moves = cached_cur_total_moves; + // item_counter represents the percent progress relative to the base batch time // stored precise to 5 decimal places ( e.g. 67.32 percent would be stored as 6'732'000 ) const int old_counter = craft.item_counter; - // Base moves for batch size with no speed modifier or assistants - // Must ensure >= 1 so we don't divide by 0; - const double base_total_moves = std::max( static_cast( 1 ), rec.batch_time( crafter, - craft.charges, 1.0f, 0 ) ); - // Current expected total moves, includes crafting speed modifiers and assistants - const double cur_total_moves = std::max( static_cast( 1 ), rec.batch_time( crafter, - craft.charges, crafting_speed, - assistants ) ); // Delta progress in moves adjusted for current crafting speed / //crafter.exertion_adjusted_move_multiplier( exertion_level() ) int spent_moves = crafter.get_moves() * crafter.exertion_adjusted_move_multiplier( @@ -1863,6 +1876,8 @@ void craft_activity_actor::do_turn( player_activity &act, Character &crafter ) // Divide by 100 for seconds, 20 for 5% const time_duration pct_time = time_duration::from_seconds( base_total_moves / 2000 ); crafter.craft_proficiency_gain( craft, pct_time * five_percent_steps ); + // Invalidate the crafting time cache because proficiencies may have changed + cached_crafting_speed = 0; } // Unlike skill, tools are consumed once at the start and should not be consumed at the end diff --git a/src/activity_actor_definitions.h b/src/activity_actor_definitions.h index b3fd236b518b2..a245d29180781 100644 --- a/src/activity_actor_definitions.h +++ b/src/activity_actor_definitions.h @@ -698,6 +698,10 @@ class craft_activity_actor : public activity_actor float activity_override = NO_EXERCISE; cata::optional cached_continuation_requirements; + float cached_crafting_speed; + int cached_assistants; + double cached_base_total_moves; + double cached_cur_total_moves; bool check_if_craft_okay( item_location &craft_item, Character &crafter ); public: diff --git a/src/character.cpp b/src/character.cpp index 8d43e6870d7b8..4736b79553499 100644 --- a/src/character.cpp +++ b/src/character.cpp @@ -437,7 +437,6 @@ std::string enum_to_string( blood_type data ) // *INDENT-OFF* Character::Character() : - cached_time( calendar::before_time_starts ), id( -1 ), next_climate_control_check( calendar::before_time_starts ), last_climate_control_ret( false ) @@ -481,6 +480,7 @@ Character::Character() : move_mode = move_mode_id( "walk" ); next_expected_position = cata::nullopt; + crafting_cache.time = calendar::before_time_starts; } // *INDENT-ON* diff --git a/src/character.h b/src/character.h index c751e2fcf0b50..bc33ba153f83e 100644 --- a/src/character.h +++ b/src/character.h @@ -2011,8 +2011,6 @@ class Character : public Creature, public visitable // Save favorite ammo location item_location ammo_location; std::set camps; - /* crafting inventory cached time */ - time_point cached_time; std::vector addictions; /** Adds an addiction to the player */ @@ -2462,7 +2460,7 @@ class Character : public Creature, public visitable void clear_morale(); bool has_morale_to_read() const; bool has_morale_to_craft() const; - const inventory &crafting_inventory( bool clear_path ); + const inventory &crafting_inventory( bool clear_path ) const; /** * Returns items that can be used to craft with. Always includes character inventory. * @param src_pos Character position. @@ -2471,7 +2469,7 @@ class Character : public Creature, public visitable * @returns Craftable inventory items found. * */ const inventory &crafting_inventory( const tripoint &src_pos = tripoint_zero, - int radius = PICKUP_RANGE, bool clear_path = true ); + int radius = PICKUP_RANGE, bool clear_path = true ) const; void invalidate_crafting_inventory(); /** Returns a value from 1.0 to 11.0 that acts as a multiplier @@ -2530,15 +2528,10 @@ class Character : public Creature, public visitable /** For use with in progress crafts */ float crafting_speed_multiplier( const item &craft, const cata::optional &loc ) const; int available_assistant_count( const recipe &rec ) const; - /** - * Time to craft not including speed multiplier - */ - int64_t base_time_to_craft( const recipe &rec, int batch_size = 1 ) const; /** * Expected time to craft a recipe, with assumption that multipliers stay constant. */ - int64_t expected_time_to_craft( const recipe &rec, int batch_size = 1, - bool in_progress = false ) const; + int64_t expected_time_to_craft( const recipe &rec, int batch_size = 1 ) const; std::vector get_eligible_containers_for_crafting() const; bool check_eligible_containers_for_crafting( const recipe &rec, int batch_size = 1 ) const; bool can_make( const recipe *r, int batch_size = 1 ); // have components? @@ -2945,9 +2938,14 @@ class Character : public Creature, public visitable struct weighted_int_list melee_miss_reasons; - int cached_moves; - tripoint cached_position; - pimpl cached_crafting_inventory; + struct crafting_cache_type { + time_point time; + int moves; + tripoint position; + int radius; + pimpl crafting_inventory; + }; + mutable crafting_cache_type crafting_cache; time_point melee_warning_turn = calendar::turn_zero; protected: diff --git a/src/crafting.cpp b/src/crafting.cpp index 6b2c8ff1c2f47..980130372a99c 100644 --- a/src/crafting.cpp +++ b/src/crafting.cpp @@ -375,17 +375,10 @@ int Character::available_assistant_count( const recipe &rec ) const } ); } -int64_t Character::base_time_to_craft( const recipe &rec, int batch_size ) const +int64_t Character::expected_time_to_craft( const recipe &rec, int batch_size ) const { const size_t assistants = available_assistant_count( rec ); - return rec.batch_time( *this, batch_size, 1.0f, assistants ); -} - -int64_t Character::expected_time_to_craft( const recipe &rec, int batch_size, - bool in_progress ) const -{ - const size_t assistants = available_assistant_count( rec ); - float modifier = crafting_speed_multiplier( rec, in_progress ); + float modifier = crafting_speed_multiplier( rec ); return rec.batch_time( *this, batch_size, modifier, assistants ); } @@ -536,61 +529,62 @@ bool Character::can_start_craft( const recipe *rec, recipe_filter_flags flags, i inv, rec->get_component_filter( flags ), batch_size, craft_flags::start_only ); } -const inventory &Character::crafting_inventory( bool clear_path ) +const inventory &Character::crafting_inventory( bool clear_path ) const { return crafting_inventory( tripoint_zero, PICKUP_RANGE, clear_path ); } const inventory &Character::crafting_inventory( const tripoint &src_pos, int radius, - bool clear_path ) + bool clear_path ) const { tripoint inv_pos = src_pos; if( src_pos == tripoint_zero ) { inv_pos = pos(); } - static int radius_mem = radius; - if( cached_moves == moves - && radius_mem == radius - && cached_time == calendar::turn - && cached_position == inv_pos ) { - return *cached_crafting_inventory; + if( moves == crafting_cache.moves + && radius == crafting_cache.radius + && calendar::turn == crafting_cache.time + && inv_pos == crafting_cache.position ) { + return *crafting_cache.crafting_inventory; } - radius_mem = radius; - cached_crafting_inventory->clear(); + crafting_cache.crafting_inventory->clear(); if( radius >= 0 ) { - cached_crafting_inventory->form_from_map( inv_pos, radius, this, false, clear_path ); + crafting_cache.crafting_inventory->form_from_map( inv_pos, radius, this, false, clear_path ); } - for( const item_location &it : all_items_loc() ) { + // TODO: Add a const overload of all_items_loc() that returns something like + // vector in order to get rid of the const_cast here. + for( const item_location &it : const_cast( this )->all_items_loc() ) { // can't craft with containers that have items in them if( !it->contents.empty_container() ) { continue; } - cached_crafting_inventory->add_item( *it ); + crafting_cache.crafting_inventory->add_item( *it ); } for( const bionic &bio : *my_bionics ) { const bionic_data &bio_data = bio.info(); if( ( !bio_data.activated || bio.powered ) && !bio_data.fake_item.is_empty() ) { - *cached_crafting_inventory += item( bio.info().fake_item, - calendar::turn, units::to_kilojoule( get_power_level() ) ); + *crafting_cache.crafting_inventory += item( bio.info().fake_item, + calendar::turn, units::to_kilojoule( get_power_level() ) ); } } if( has_trait( trait_BURROW ) ) { - *cached_crafting_inventory += item( "pickaxe", calendar::turn ); - *cached_crafting_inventory += item( "shovel", calendar::turn ); + *crafting_cache.crafting_inventory += item( "pickaxe", calendar::turn ); + *crafting_cache.crafting_inventory += item( "shovel", calendar::turn ); } - cached_moves = moves; - cached_time = calendar::turn; - cached_position = inv_pos; - return *cached_crafting_inventory; + crafting_cache.moves = moves; + crafting_cache.time = calendar::turn; + crafting_cache.position = inv_pos; + crafting_cache.radius = radius; + return *crafting_cache.crafting_inventory; } void Character::invalidate_crafting_inventory() { - cached_time = calendar::before_time_starts; + crafting_cache.time = calendar::before_time_starts; } void Character::make_craft( const recipe_id &id_to_make, int batch_size, @@ -1020,14 +1014,7 @@ double Character::crafting_success_roll( const recipe &making ) const return 2; } - float prof_multiplier = 1.0f; - for( const recipe_proficiency &recip : making.proficiencies ) { - if( !recip.required && !has_proficiency( recip.id ) ) { - prof_multiplier *= recip.fail_multiplier; - } - } - - return ( skill_roll / diff_roll ) / prof_multiplier; + return ( skill_roll / diff_roll ) / making.proficiency_failure_maluses( *this ); } int item::get_next_failure_point() const diff --git a/src/recipe.cpp b/src/recipe.cpp index f6542a1bd863b..e73a45c07a6c9 100644 --- a/src/recipe.cpp +++ b/src/recipe.cpp @@ -67,16 +67,7 @@ int64_t recipe::time_to_craft_moves( const Character &guy, recipe_time_flag flag if( flags == recipe_time_flag::ignore_proficiencies ) { return time; } - int64_t ret = time; - for( const recipe_proficiency &prof : proficiencies ) { - if( !prof.required ) { - if( !guy.has_proficiency( prof.id ) && - !helpers_have_proficiencies( guy, prof.id ) ) { - ret *= prof.time_multiplier; - } - } - } - return ret; + return time * proficiency_time_maluses( guy ); } int64_t recipe::batch_time( const Character &guy, int batch, float multiplier, @@ -658,7 +649,7 @@ std::string recipe::used_proficiencies_string( const Character *c ) const return used; } -std::string recipe::missing_proficiencies_string( Character *c ) const +std::string recipe::missing_proficiencies_string( const Character *c ) const { if( c == nullptr ) { return { }; @@ -738,7 +729,7 @@ std::set recipe::assist_proficiencies() const return ret; } -float recipe::proficiency_time_maluses( Character &guy ) const +float recipe::proficiency_time_maluses( const Character &guy ) const { float total_malus = 1.0f; for( const recipe_proficiency &prof : proficiencies ) { @@ -752,7 +743,7 @@ float recipe::proficiency_time_maluses( Character &guy ) const return total_malus; } -float recipe::proficiency_failure_maluses( Character &guy ) const +float recipe::proficiency_failure_maluses( const Character &guy ) const { float total_malus = 1.0f; for( const recipe_proficiency &prof : proficiencies ) { diff --git a/src/recipe.h b/src/recipe.h index a08eab3389b17..5799fe6d27a20 100644 --- a/src/recipe.h +++ b/src/recipe.h @@ -174,7 +174,7 @@ class recipe // Format the proficiencies string. std::string required_proficiencies_string( const Character *c ) const; std::string used_proficiencies_string( const Character *c ) const; - std::string missing_proficiencies_string( Character *c ) const; + std::string missing_proficiencies_string( const Character *c ) const; // Proficiencies for search std::string recipe_proficiencies_string() const; // Required proficiencies @@ -184,9 +184,9 @@ class recipe // Helpful proficiencies std::set assist_proficiencies() const; // The time malus due to proficiencies lacking - float proficiency_time_maluses( Character &guy ) const; + float proficiency_time_maluses( const Character &guy ) const; // The failure malus due to proficiencies lacking - float proficiency_failure_maluses( Character &guy ) const; + float proficiency_failure_maluses( const Character &guy ) const; // How active of exercise this recipe is float exertion_level() const; diff --git a/tests/crafting_test.cpp b/tests/crafting_test.cpp index d61d4c1b1a8a0..9a0a6c99cedfc 100644 --- a/tests/crafting_test.cpp +++ b/tests/crafting_test.cpp @@ -311,6 +311,17 @@ static void give_tools( const std::vector &tools ) } } +static void grant_skills_to_character( Character &you, const recipe &r ) +{ + // Ensure adequate skill for all "required" skills + for( const std::pair &skl : r.required_skills ) { + you.set_skill_level( skl.first, skl.second ); + } + // and just in case "used" skill difficulty is higher, set that too + you.set_skill_level( r.skill_used, std::max( r.difficulty, + you.get_skill_level( r.skill_used ) ) ); +} + static void prep_craft( const recipe_id &rid, const std::vector &tools, bool expect_craftable ) { @@ -321,17 +332,10 @@ static void prep_craft( const recipe_id &rid, const std::vector &tools, Character &player_character = get_player_character(); player_character.toggle_trait( trait_id( "DEBUG_CNF" ) ); player_character.setpos( test_origin ); - give_tools( tools ); - const recipe &r = rid.obj(); + grant_skills_to_character( player_character, r ); - // Ensure adequate skill for all "required" skills - for( const std::pair &skl : r.required_skills ) { - player_character.set_skill_level( skl.first, skl.second ); - } - // and just in case "used" skill difficulty is higher, set that too - player_character.set_skill_level( r.skill_used, std::max( r.difficulty, - player_character.get_skill_level( r.skill_used ) ) ); + give_tools( tools ); player_character.moves--; const inventory &crafting_inv = player_character.crafting_inventory(); @@ -923,3 +927,33 @@ TEST_CASE( "check-tool_qualities" ) CHECK( tool_with_ammo( "survivor_mess_kit", 20 ).has_quality( quality_id( "BOIL" ), 2, 1 ) ); CHECK( tool_with_ammo( "survivor_mess_kit", 20 ).get_quality( quality_id( "BOIL" ) ) > 0 ); } + +TEST_CASE( "book_proficiency_mitigation", "[crafting][proficiency]" ) +{ + GIVEN( "a recipe with required proficiencies" ) { + clear_avatar(); + clear_map(); + const recipe &test_recipe = recipe_id( "leather_belt" ).obj(); + + grant_skills_to_character( get_player_character(), test_recipe ); + int unmitigated_time_taken = test_recipe.batch_time( get_player_character(), 1, 1, 0 ); + + WHEN( "player has a book mitigating lack of proficiency" ) { + std::vector books; + books.emplace_back( "manual_tailor" ); + give_tools( books ); + get_player_character().invalidate_crafting_inventory(); + int mitigated_time_taken = test_recipe.batch_time( get_player_character(), 1, 1, 0 ); + THEN( "it takes less time to craft the recipe" ) { + CHECK( mitigated_time_taken < unmitigated_time_taken ); + } + AND_WHEN( "player acquires missing proficiencies" ) { + grant_proficiencies_to_character( get_player_character(), test_recipe, true ); + int proficient_time_taken = test_recipe.batch_time( get_player_character(), 1, 1, 0 ); + THEN( "it takes even less time to craft the recipe" ) { + CHECK( proficient_time_taken < mitigated_time_taken ); + } + } + } + } +}