From 9ee199e034937917a94a591ba36b2fe8453ab3f5 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Wed, 1 Jan 2020 17:23:44 -0500 Subject: [PATCH 01/10] Sort alternatives in requirement_data::list_all Want to use this output in tests, and it's helpful for it to be more consistent. --- src/requirements.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/requirements.cpp b/src/requirements.cpp index 3563635cbca89..f63aa8008faf0 100644 --- a/src/requirements.cpp +++ b/src/requirements.cpp @@ -311,12 +311,13 @@ std::string requirement_data::print_all_objs( const std::string &header, if( !buffer.empty() ) { buffer += std::string( "\n" ) + _( "and " ); } - for( auto it = list.begin(); it != list.end(); ++it ) { - if( it != list.begin() ) { - buffer += _( " or " ); - } - buffer += it->to_string(); - } + std::vector alternatives; + std::transform( list.begin(), list.end(), std::back_inserter( alternatives ), + []( const T & t ) { + return t.to_string(); + } ); + std::sort( alternatives.begin(), alternatives.end() ); + buffer += join( alternatives, _( " or " ) ); } if( buffer.empty() ) { return std::string(); From 10aad0a269a1ef1aa8554d2a3abfee44e2551b0b Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Wed, 1 Jan 2020 17:25:14 -0500 Subject: [PATCH 02/10] Typo --- src/requirements.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/requirements.h b/src/requirements.h index 7fbf2a84c84ac..b61a1710dc159 100644 --- a/src/requirements.h +++ b/src/requirements.h @@ -28,7 +28,7 @@ using itype_id = std::string; enum available_status { a_true = +1, // yes, it's available a_false = -1, // no, it's not available - a_insufficent = 0, // neraly, bt not enough for tool+component + a_insufficent = 0, // nearly, but not enough for tool+component }; enum component_type : int { From c572ecd9cf004dd69a5782c8c519d6fbba0558f1 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Wed, 1 Jan 2020 17:32:59 -0500 Subject: [PATCH 03/10] Add deduped_requirement_data This is a new class intended to permit refactoring requirement_data to avoid duplicated items which cause various issues, such as difficulty in determining whether an item is possible to craft. This just ads the class and some tests related to it. As yet, it is not being put to use. --- src/basecamp.cpp | 7 +- src/requirements.cpp | 148 ++++++++++++++++++++++++++++++++++++ src/requirements.h | 37 +++++++++ tests/requirements_test.cpp | 117 ++++++++++++++++++++++++++++ 4 files changed, 303 insertions(+), 6 deletions(-) create mode 100644 tests/requirements_test.cpp diff --git a/src/basecamp.cpp b/src/basecamp.cpp index 1cddbd75f418b..0c12c7170276e 100644 --- a/src/basecamp.cpp +++ b/src/basecamp.cpp @@ -690,7 +690,7 @@ basecamp_action_components::basecamp_action_components( bool basecamp_action_components::choose_components() { const auto filter = is_crafting_component; - const requirement_data &req = making_.requirements(); + const requirement_data &req = making_.deduped_requirements().select_alternative( g->u, filter ); if( !item_selections_.empty() || !tool_selections_.empty() ) { debugmsg( "Reused basecamp_action_components" ); return false; @@ -726,11 +726,6 @@ void basecamp_action_components::consume_components() target_map = map_.get(); } const tripoint &origin = target_map->getlocal( base_.get_dumping_spot() ); - const auto &req = making_.requirements(); - if( item_selections_.size() != req.get_components().size() || - tool_selections_.size() != req.get_tools().size() ) { - debugmsg( "Not all selections have been made for basecamp_action_components" ); - } for( const comp_selection &sel : item_selections_ ) { g->u.consume_items( *target_map, sel, batch_size_, is_crafting_component, origin, base_.inv_range ); diff --git a/src/requirements.cpp b/src/requirements.cpp index f63aa8008faf0..8b3937b71bc14 100644 --- a/src/requirements.cpp +++ b/src/requirements.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "avatar.h" @@ -1088,3 +1089,150 @@ void requirement_data::consolidate() } components = std::move( all_comps ); } + +/// Helper function for deduped_requirement_data constructor below. +/// +/// The goal of this function is to consolidate a particular item_comp that +/// would otherwise be duplicated between two requirements. +/// +/// It operates recursively (increasing @p index with the depth of recursion), +/// searching for another item_comp to marge @p leftover with. For each +/// compatible item_comp found it performs that merger and writes out a +/// suitably updated form of the overall requirements to @p result. +/// +/// If it chooses *not* to merge with any particular item_comp, then it deletes +/// that item_comp from the options, to avoid the duplication. +/// +/// Lastly, it also outputs a version of the requirements where @p leftover +/// remains where it was, and all other compatible item_comp entries have been +/// deleted. +/// +/// @param leftover The item_comp needing to be dealt with. +/// @param req_prefix The requirements considered so far; more will be appended +/// to this. +/// @param to_expand The original requirements we are working through to look +/// for a duplicate. +/// @param orig_index The index into the alter_item_comp_vector where @p +/// leftover was originally to be found. If it isn't merged with another item, +/// then it will be re-inserted at this position. +/// @param index The position within @p to_expand where we will next look for +/// duplicates of @p leftover to merge with. +/// @param result The finished requirements should be appended to this. +static void expand_item_in_reqs( + const item_comp &leftover, requirement_data::alter_item_comp_vector req_prefix, + const requirement_data::alter_item_comp_vector &to_expand, size_t orig_index, size_t index, + std::vector &result ) +{ + assert( req_prefix.size() >= orig_index ); + assert( orig_index < index ); + + if( index == to_expand.size() ) { + // We reached the end without using the leftovers. So need to add them + // as their own requirement, separate from everything else. + req_prefix.insert( req_prefix.begin() + orig_index, { leftover } ); + result.push_back( req_prefix ); + return; + } + + std::vector this_requirement = to_expand[index]; + auto duplicate = std::find_if( this_requirement.begin(), this_requirement.end(), + [&]( const item_comp & c ) { + return c.type == leftover.type; + } ); + if( duplicate == this_requirement.end() ) { + // No match in this one; proceed to next + req_prefix.push_back( this_requirement ); + expand_item_in_reqs( leftover, req_prefix, to_expand, orig_index, index + 1, result ); + return; + } + // First option: amalgamate the leftovers into this requirement, which + // forces us to pick that specific option: + requirement_data::alter_item_comp_vector req = req_prefix; + req.push_back( { item_comp( leftover.type, leftover.count + duplicate->count ) } ); + req.insert( req.end(), to_expand.begin() + index + 1, to_expand.end() ); + result.push_back( req ); + + // Second option: use a separate option for this requirement, which means + // we need to recurse further to find something into which to amalgamate + // the original requirement + this_requirement.erase( duplicate ); + if( !this_requirement.empty() ) { + req_prefix.push_back( this_requirement ); + expand_item_in_reqs( leftover, req_prefix, to_expand, orig_index, index + 1, result ); + } +} + +deduped_requirement_data::deduped_requirement_data( const requirement_data &in ) +{ + // This constructor works through a requirement_data, converting it into an + // equivalent set of requirement_data alternatives, where each alternative + // has the property that no item type appears more than once. + // + // We only deal with item requirements. Tool requirements could be handled + // similarly, but no examples where they are a problem have yet been + // raised. + // + // We maintain a queue of requirement_data component info to be split. + // Each to_check struct has a vector of component requirements, and an + // index. The index is the position within the vector to be checked next. + struct to_check { + alter_item_comp_vector components; + size_t index; + }; + std::stack> pending; + pending.push( { in.get_components(), 0 } ); + + while( !pending.empty() ) { + to_check next = pending.top(); + pending.pop(); + + if( next.index == next.components.size() ) { + alternatives_.emplace_back( in.get_tools(), in.get_qualities(), next.components ); + continue; + } + + // Build a set of all the itypes used in later stages of this set of + // requirements. + std::unordered_set later_itypes; + for( size_t i = next.index + 1; i != next.components.size(); ++i ) { + std::transform( next.components[i].begin(), next.components[i].end(), + std::inserter( later_itypes, later_itypes.end() ), + []( const item_comp & c ) { + return c.type; + } ); + } + + std::vector this_requirement = next.components[next.index]; + + auto first_duplicated = std::stable_partition( + this_requirement.begin(), this_requirement.end(), + [&]( const item_comp & c ) { + return !later_itypes.count( c.type ); + } + ); + + for( auto comp_it = first_duplicated; comp_it != this_requirement.end(); ++comp_it ) { + // Factor this requirement out into its own separate case + + alter_item_comp_vector req_prefix( next.components.begin(), + next.components.begin() + next.index ); + std::vector result; + expand_item_in_reqs( *comp_it, req_prefix, next.components, next.index, next.index + 1, + result ); + for( const alter_item_comp_vector &v : result ) { + // When v is smaller, that means the current requirement was + // deleted, in which case we don't advance index. + size_t index_inc = v.size() == next.components.size() ? 1 : 0; + pending.push( { v, next.index + index_inc } ); + } + } + + // Deal with all the remaining, non-duplicated ones + this_requirement.erase( first_duplicated, this_requirement.end() ); + if( !this_requirement.empty() ) { + alter_item_comp_vector without_dupes = next.components; + without_dupes[next.index] = this_requirement; + pending.push( { without_dupes, next.index + 1 } ); + } + } +} diff --git a/src/requirements.h b/src/requirements.h index b61a1710dc159..f169579cabda3 100644 --- a/src/requirements.h +++ b/src/requirements.h @@ -332,4 +332,41 @@ struct requirement_data { static const T *find_by_type( const std::vector< std::vector > &vec, const ID &type ); }; +// Sometimes the requirement_data is problematic, because it has overlapping +// requirements. For example, a survivor telescope requires +// 1 high-quality lens +// 1 high-quality lens OR 1 small high-quality lens +// If there is just one high-quality lens in the available inventory then it's +// hard to correctly detect that these requirements are impossible to satisfy. +// In general, determining craftability is equivalent to boolean +// satisfiability, and thus NP-hard. +// +// In practice, we don't expect recipes to have too much overlap, so this issue +// should be tractable. +// +// However, to avoid keeping additional state during the process of searching +// for components, we don't make the component search more complex, instead we +// make the requirements more complex. We replace each requirement_data with a +// deduped_requirement_data, which contains a selection of alternative +// requirement_data objects, each of which contains no overlapping +// requirements. +// +// For the majority of recipes, there are no overlaps, and this will be +// essentially equivalent to just a requirement_data. However, for the few +// problematic recipes this allows us to calculate crafting requirements more +// accurately. +class deduped_requirement_data +{ + public: + using alter_item_comp_vector = requirement_data::alter_item_comp_vector; + + deduped_requirement_data( const requirement_data & ); + + std::vector const &alternatives() const { + return alternatives_; + } + private: + std::vector alternatives_; +}; + #endif diff --git a/tests/requirements_test.cpp b/tests/requirements_test.cpp new file mode 100644 index 0000000000000..fc75bb7a8d2e3 --- /dev/null +++ b/tests/requirements_test.cpp @@ -0,0 +1,117 @@ +#include "requirements.h" + +#include "catch/catch.hpp" + +static void test_requirement_deduplication( + const requirement_data::alter_item_comp_vector &before, + std::vector after +) +{ + requirement_data in( {}, {}, before ); + deduped_requirement_data out( in ); + CHECK( out.alternatives().size() == after.size() ); + while( after.size() < out.alternatives().size() ) { + after.emplace_back(); + } + + for( size_t i = 0; i < out.alternatives().size(); ++i ) { + CAPTURE( i ); + requirement_data this_expected( {}, {}, after[i] ); + CHECK( out.alternatives()[i].list_all() == this_expected.list_all() ); + } +} + +TEST_CASE( "simple_requirements_dont_multiply", "[requirement]" ) +{ + test_requirement_deduplication( { { { "rock", 1 } } }, { { { { "rock", 1 } } } } ); +} + +TEST_CASE( "survivor_telescope_inspired_example", "[requirement]" ) +{ + requirement_data::alter_item_comp_vector before; + test_requirement_deduplication( + { { { "rock", 1 }, { "soap", 1 } }, { { "rock", 1 } } }, { + { { { "soap", 1 } }, { { "rock", 1 } } }, + { { { "rock", 2 } } } + } ); +} + +TEST_CASE( "survivor_telescope_inspired_example_2", "[requirement]" ) +{ + requirement_data::alter_item_comp_vector before; + test_requirement_deduplication( + { { { "ash", 1 } }, { { "rock", 1 }, { "soap", 1 } }, { { "rock", 1 } }, { { "lye", 1 } } }, { + { { { "ash", 1 } }, { { "soap", 1 } }, { { "rock", 1 } }, { { "lye", 1 } } }, + { { { "ash", 1 } }, { { "rock", 2 } }, { { "lye", 1 } } } + } ); +} + +TEST_CASE( "woods_soup_inspired_example", "[requirement]" ) +{ + requirement_data::alter_item_comp_vector before; + test_requirement_deduplication( + { { { "rock", 1 }, { "soap", 1 } }, { { "rock", 1 }, { "yarn", 1 } } }, { + { { { "soap", 1 } }, { { "rock", 1 }, { "yarn", 1 } } }, + { { { "rock", 1 } }, { { "yarn", 1 } } }, + { { { "rock", 2 } } } + } ); +} + +TEST_CASE( "triple_overlap_1", "[requirement]" ) +{ + requirement_data::alter_item_comp_vector before; + test_requirement_deduplication( { + { { "rock", 1 }, { "soap", 1 } }, + { { "rock", 1 } }, + { { "soap", 1 } } + }, { + { { { "rock", 1 } }, { { "soap", 2 } } }, + { { { "rock", 2 } }, { { "soap", 1 } } }, + } ); +} + +TEST_CASE( "triple_overlap_2", "[requirement]" ) +{ + requirement_data::alter_item_comp_vector before; + test_requirement_deduplication( { + { { "rock", 1 }, { "soap", 1 } }, + { { "rock", 1 }, { "yarn", 1 } }, + { { "soap", 1 }, { "acid", 1 } } + }, { + { { { "soap", 1 } }, { { "rock", 1 }, { "yarn", 1 } }, { { "acid", 1 } } }, + { { { "rock", 1 }, { "yarn", 1 } }, { { "soap", 2 } } }, + { { { "rock", 1 } }, { { "yarn", 1 } }, { { "acid", 1 }, { "soap", 1 } } }, + { { { "rock", 2 } }, { { "acid", 1 }, { "soap", 1 } } }, + } ); +} + +TEST_CASE( "triple_overlap_3", "[requirement]" ) +{ + requirement_data::alter_item_comp_vector before; + test_requirement_deduplication( { + { { "rock", 1 }, { "soap", 1 } }, + { { "rock", 1 }, { "yarn", 1 } }, + { { "soap", 1 }, { "yarn", 1 } } + }, { + // These results are not ideal. Two of them are equivalent and + // another two could be merged. But they are correct, and that + // seems good enough for now. I don't anticipate any real recipes + // being as complicated to resolve as this one. + { { { "soap", 1 } }, { { "rock", 1 } }, { { "yarn", 1 } } }, + { { { "soap", 1 } }, { { "yarn", 2 } } }, + { { { "rock", 1 }, { "yarn", 1 } }, { { "soap", 2 } } }, + { { { "rock", 1 } }, { { "yarn", 1 } }, { { "soap", 1 } } }, + { { { "rock", 1 } }, { { "yarn", 2 } } }, + { { { "rock", 2 } }, { { "yarn", 1 }, { "soap", 1 } } }, + } ); +} + +TEST_CASE( "deduplicate_repeated_requirements", "[requirement]" ) +{ + requirement_data::alter_item_comp_vector before; + test_requirement_deduplication( { + { { "rock", 1 } }, { { "yarn", 1 } }, { { "rock", 1 } }, { { "yarn", 1 } } + }, { + { { { "rock", 2 } }, { { "yarn", 2 } } }, + } ); +} From 2224d2b0f010b5ea2a87ebcfd59767a17c6ecf47 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Wed, 1 Jan 2020 18:05:15 -0500 Subject: [PATCH 04/10] Add sanity check to requirement deduplication To prevent consuming all time or memory in the event this hits a bad case (which is possible for maliciously crafted recipes). --- src/requirements.cpp | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/requirements.cpp b/src/requirements.cpp index 8b3937b71bc14..18a2d83b51728 100644 --- a/src/requirements.cpp +++ b/src/requirements.cpp @@ -1234,5 +1234,14 @@ deduped_requirement_data::deduped_requirement_data( const requirement_data &in ) without_dupes[next.index] = this_requirement; pending.push( { without_dupes, next.index + 1 } ); } + + // Because this algorithm is super-exponential in the worst case, add a + // sanity check to prevent things getting too far out of control. + static constexpr size_t max_alternatives = 20; + if( alternatives_.size() + pending.size() > max_alternatives ) { + debugmsg( "Construction of deduped_requirement_data generated too many alternatives. " + "The recipe at fault should be simplified." ); + abort(); + } } } From d8490e07de7d5f7851b5189be17cf98dd33d2b4c Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Wed, 1 Jan 2020 23:28:33 -0500 Subject: [PATCH 05/10] Use new deduped_requirements_data in recipes Fixes #32311. Fixes #32554. Each recipe now has a deduped_requirements() in addition to its requirements, now accessed via simple_requirements(). Each caller has been ported to one or the other as appropriate. Refactor craftability checks so that the "only needs 5% of charge" property can now be specified via a flag, to obviate the need to reconstruct an entire requirements object just for that change. Limited in-game testing is promising so far. --- src/basecamp.cpp | 6 +-- src/consumption.cpp | 2 +- src/craft_command.cpp | 3 +- src/crafting.cpp | 60 +++++++------------------- src/crafting.h | 10 +++++ src/crafting_gui.cpp | 4 +- src/faction_camp.cpp | 22 +++++----- src/item.cpp | 3 +- src/iuse.cpp | 10 +++-- src/player.h | 3 ++ src/recipe.cpp | 4 +- src/recipe.h | 21 +++++++-- src/recipe_dictionary.cpp | 10 ++--- src/requirements.cpp | 86 ++++++++++++++++++++++++++++++------- src/requirements.h | 38 +++++++++++++--- tests/comestible_tests.cpp | 5 ++- tests/crafting_test.cpp | 5 +-- tests/requirements_test.cpp | 2 +- 18 files changed, 190 insertions(+), 104 deletions(-) diff --git a/src/basecamp.cpp b/src/basecamp.cpp index 0c12c7170276e..ac77a3719ef99 100644 --- a/src/basecamp.cpp +++ b/src/basecamp.cpp @@ -208,8 +208,8 @@ std::string basecamp::om_upgrade_description( const std::string &bldg, bool trun std::vector component_print_buffer; const int pane = FULL_SCREEN_WIDTH; - const auto tools = making.requirements().get_folded_tools_list( pane, c_white, _inv, 1 ); - const auto comps = making.requirements().get_folded_components_list( pane, c_white, _inv, + const auto tools = making.simple_requirements().get_folded_tools_list( pane, c_white, _inv, 1 ); + const auto comps = making.simple_requirements().get_folded_components_list( pane, c_white, _inv, making.get_component_filter(), 1 ); component_print_buffer.insert( component_print_buffer.end(), tools.begin(), tools.end() ); component_print_buffer.insert( component_print_buffer.end(), comps.begin(), comps.end() ); @@ -345,7 +345,7 @@ std::vector basecamp::available_upgrades( const point &dir ) basecamp_upgrade data; data.bldg = bldg; data.name = recp.blueprint_name(); - const auto &reqs = recp.requirements(); + const auto &reqs = recp.deduped_requirements(); data.avail = reqs.can_make_with_inventory( _inv, recp.get_component_filter(), 1 ); data.in_progress = in_progress; ret_data.emplace_back( data ); diff --git a/src/consumption.cpp b/src/consumption.cpp index e26111abc7ff9..dff13a88c0cc2 100644 --- a/src/consumption.cpp +++ b/src/consumption.cpp @@ -242,7 +242,7 @@ std::pair player::compute_nutrient_range( our_extra_flags.insert( "COOKED" ); } - const requirement_data requirements = rec.requirements(); + const requirement_data requirements = rec.simple_requirements(); const requirement_data::alter_item_comp_vector &component_requirements = requirements.get_components(); diff --git a/src/craft_command.cpp b/src/craft_command.cpp index 0800bf519e065..dc112e91bc733 100644 --- a/src/craft_command.cpp +++ b/src/craft_command.cpp @@ -144,8 +144,9 @@ void craft_command::execute( const tripoint &new_loc ) } item_selections.clear(); - const auto needs = rec->requirements(); const auto filter = rec->get_component_filter( flags ); + const requirement_data &needs = rec->deduped_requirements().select_alternative( + *crafter, filter, batch_size, craft_flags::start_only ); for( const auto &it : needs.get_components() ) { comp_selection is = diff --git a/src/crafting.cpp b/src/crafting.cpp index 2f9839872f76c..d01944d7a78b7 100644 --- a/src/crafting.cpp +++ b/src/crafting.cpp @@ -323,7 +323,7 @@ bool player::making_would_work( const recipe_id &id_to_make, int batch_size ) if( !can_make( &making, batch_size ) ) { std::string buffer = _( "You can no longer make that craft!" ); buffer += "\n"; - buffer += making.requirements().list_missing(); + buffer += making.simple_requirements().list_missing(); popup( buffer, PF_NONE ); return false; } @@ -472,14 +472,14 @@ std::vector player::get_eligible_containers_for_crafting() const bool player::can_make( const recipe *r, int batch_size ) { - const inventory &crafting_inv = crafting_inventory(); + inventory crafting_inv = crafting_inventory(); if( has_recipe( r, crafting_inv, get_crafting_helpers() ) < 0 ) { return false; } - return r->requirements().can_make_with_inventory( crafting_inv, r->get_component_filter(), - batch_size ); + return r->deduped_requirements().can_make_with_inventory( + crafting_inv, r->get_component_filter(), batch_size ); } bool player::can_start_craft( const recipe *rec, recipe_filter_flags flags, int batch_size ) @@ -488,45 +488,9 @@ bool player::can_start_craft( const recipe *rec, recipe_filter_flags flags, int return false; } - const std::vector> &tool_reqs = rec->requirements().get_tools(); - - // For tools adjust the reqired charges - std::vector> adjusted_tool_reqs; - for( const std::vector &alternatives : tool_reqs ) { - std::vector adjusted_alternatives; - for( const tool_comp &alternative : alternatives ) { - tool_comp adjusted_alternative = alternative; - if( adjusted_alternative.count > 0 ) { - adjusted_alternative.count *= batch_size; - // Only for the first 5% progress - adjusted_alternative.count = std::max( adjusted_alternative.count / 20, 1 ); - } - adjusted_alternatives.push_back( adjusted_alternative ); - } - adjusted_tool_reqs.push_back( adjusted_alternatives ); - } - - const std::vector> &comp_reqs = rec->requirements().get_components(); - - // For components we need to multiply by batch size to stay even with tools - std::vector> adjusted_comp_reqs; - for( const std::vector &alternatives : comp_reqs ) { - std::vector adjusted_alternatives; - for( const item_comp &alternative : alternatives ) { - item_comp adjusted_alternative = alternative; - adjusted_alternative.count *= batch_size; - adjusted_alternatives.push_back( adjusted_alternative ); - } - adjusted_comp_reqs.push_back( adjusted_alternatives ); - } - - // Qualities don't need adjustment - const requirement_data start_reqs( adjusted_tool_reqs, - rec->requirements().get_qualities(), - adjusted_comp_reqs ); - - return start_reqs.can_make_with_inventory( crafting_inventory(), - rec->get_component_filter( flags ) ); + inventory inv = crafting_inventory(); + return rec->deduped_requirements().can_make_with_inventory( + inv, rec->get_component_filter( flags ), batch_size, craft_flags::start_only ); } const inventory &player::crafting_inventory( bool clear_path ) @@ -1306,7 +1270,7 @@ bool player::can_continue_craft( item &craft ) if( !craft.has_tools_to_continue() ) { - const std::vector> &tool_reqs = rec.requirements().get_tools(); + const std::vector> &tool_reqs = rec.simple_requirements().get_tools(); const int batch_size = craft.charges; std::vector> adjusted_tool_reqs; @@ -1357,6 +1321,14 @@ bool player::can_continue_craft( item &craft ) return true; } +const requirement_data &player::select_requirements( + const std::vector &alternatives, int /*batch*/, const inventory &, + const std::function &/*filter*/ ) const +{ + assert( !alternatives.empty() ); + // TODO: when this is the avatar, offer the choice to the player + return *alternatives.front(); +} /* selection of component if a recipe requirement has multiple options (e.g. 'duct tap' or 'welder') */ comp_selection player::select_item_component( const std::vector &components, diff --git a/src/crafting.h b/src/crafting.h index 4649ce9cd643e..18a00750e5fa0 100644 --- a/src/crafting.h +++ b/src/crafting.h @@ -8,6 +8,16 @@ class item; class player; class recipe; +enum class craft_flags : int { + none = 0, + start_only = 1, // Only require 5% (plus remainder) of tool charges +}; + +inline constexpr craft_flags operator&( craft_flags l, craft_flags r ) +{ + return static_cast( static_cast( l ) & static_cast( r ) ); +} + // removes any (removable) ammo from the item and stores it in the // players inventory. void remove_ammo( item &dis_item, player &p ); diff --git a/src/crafting_gui.cpp b/src/crafting_gui.cpp index d3b055b0704b9..b08d56dc42242 100644 --- a/src/crafting_gui.cpp +++ b/src/crafting_gui.cpp @@ -507,7 +507,7 @@ const recipe *select_crafting_recipe( int &batch_size ) int count = batch ? line + 1 : 1; // batch size nc_color col = available[ line ].color(); - const auto &req = current[ line ]->requirements(); + const auto &req = current[ line ]->simple_requirements(); draw_can_craft_indicator( w_head, 0, *current[line] ); wrefresh( w_head ); @@ -851,7 +851,7 @@ std::string peek_related_recipe( const recipe *current, const recipe_subset &ava { // current recipe components std::vector> related_components; - const requirement_data &req = current->requirements(); + const requirement_data &req = current->simple_requirements(); for( const std::vector &comp_list : req.get_components() ) { for( const item_comp &a : comp_list ) { related_components.push_back( { a.type, item::nname( a.type, 1 ) } ); diff --git a/src/faction_camp.cpp b/src/faction_camp.cpp index 6b646f4a569cc..604b0420e3ab7 100644 --- a/src/faction_camp.cpp +++ b/src/faction_camp.cpp @@ -631,8 +631,8 @@ void basecamp::add_available_recipes( mission_data &mission_key, const point &di const std::string &title_e = dir_abbr + recipe_data.second; const std::string &entry = craft_description( recipe_data.first ); const recipe &recp = recipe_data.first.obj(); - bool craftable = recp.requirements().can_make_with_inventory( _inv, - recp.get_component_filter() ); + bool craftable = recp.deduped_requirements().can_make_with_inventory( + _inv, recp.get_component_filter() ); mission_key.add_start( id, title_e, dir, entry, craftable ); } } @@ -1538,7 +1538,8 @@ void basecamp::start_upgrade( const std::string &bldg, const point &dir, { const recipe &making = recipe_id( bldg ).obj(); //Stop upgrade if you don't have materials - if( making.requirements().can_make_with_inventory( _inv, making.get_component_filter(), 1 ) ) { + if( making.deduped_requirements().can_make_with_inventory( + _inv, making.get_component_filter() ) ) { bool must_feed = bldg != "faction_base_camp_1"; basecamp_action_components components( making, 1, *this ); @@ -1970,7 +1971,7 @@ void basecamp::start_fortifications( std::string &bldg_exp ) if( !query_yn( _( "Trip Estimate:\n%s" ), camp_trip_description( total_time, build_time, travel_time, dist, trips, need_food ) ) ) { return; - } else if( !making.requirements().can_make_with_inventory( _inv, + } else if( !making.deduped_requirements().can_make_with_inventory( _inv, making.get_component_filter(), ( fortify_om.size() * 2 ) - 2 ) ) { popup( _( "You don't have the material to build the fortification." ) ); return; @@ -2036,8 +2037,8 @@ void basecamp::start_crafting( const std::string &cur_id, const point &cur_dir, if( it != recipes.end() ) { const recipe &making = it->first.obj(); - if( !making.requirements().can_make_with_inventory( _inv, - making.get_component_filter(), 1 ) ) { + if( !making.deduped_requirements().can_make_with_inventory( + _inv, making.get_component_filter() ) ) { popup( _( "You don't have the materials to craft that" ) ); return; } @@ -2851,8 +2852,8 @@ int basecamp::recipe_batch_max( const recipe &making ) const time_duration work_days = base_camps::to_workdays( making.batch_duration( max_batch + batch_size ) ); int food_req = time_to_food( work_days ); - bool can_make = making.requirements().can_make_with_inventory( _inv, - making.get_component_filter(), max_batch + batch_size ); + bool can_make = making.deduped_requirements().can_make_with_inventory( + _inv, making.get_component_filter(), max_batch + batch_size ); if( can_make && camp_food_supply() > food_req ) { max_batch += batch_size; } else { @@ -3472,8 +3473,9 @@ std::string basecamp::craft_description( const recipe_id &itm ) std::vector component_print_buffer; int pane = FULL_SCREEN_WIDTH; - auto tools = making.requirements().get_folded_tools_list( pane, c_white, _inv, 1 ); - auto comps = making.requirements().get_folded_components_list( pane, c_white, _inv, + const requirement_data &req = making.simple_requirements(); + auto tools = req.get_folded_tools_list( pane, c_white, _inv, 1 ); + auto comps = req.get_folded_components_list( pane, c_white, _inv, making.get_component_filter(), 1 ); component_print_buffer.insert( component_print_buffer.end(), tools.begin(), tools.end() ); diff --git a/src/item.cpp b/src/item.cpp index dbb326a8905d1..2473cacf2ab0a 100644 --- a/src/item.cpp +++ b/src/item.cpp @@ -3234,7 +3234,8 @@ void item::final_info( std::vector &info, const iteminfo_query *parts, } else { const std::string recipes = enumerate_as_string( known_recipes.begin(), known_recipes.end(), [ &inv ]( const recipe * r ) { - if( r->requirements().can_make_with_inventory( inv, r->get_component_filter() ) ) { + if( r->deduped_requirements().can_make_with_inventory( + inv, r->get_component_filter() ) ) { return r->result_name(); } else { return string_format( "%s", r->result_name() ); diff --git a/src/iuse.cpp b/src/iuse.cpp index aeb1593c3b92f..929da91155ac8 100644 --- a/src/iuse.cpp +++ b/src/iuse.cpp @@ -8684,8 +8684,8 @@ int iuse::multicooker( player *p, item *it, bool t, const tripoint &pos ) for( const auto &r : g->u.get_learned_recipes().in_category( "CC_FOOD" ) ) { if( multicooked_subcats.count( r->subcategory ) > 0 ) { dishes.push_back( r ); - const bool can_make = r->requirements().can_make_with_inventory( crafting_inv, - r->get_component_filter() ); + const bool can_make = r->deduped_requirements().can_make_with_inventory( + crafting_inv, r->get_component_filter() ); dmenu.addentry( counter++, can_make, -1, r->result_name() ); } @@ -8717,9 +8717,11 @@ int iuse::multicooker( player *p, item *it, bool t, const tripoint &pos ) return 0; } - auto reqs = meal->requirements(); + const auto filter = is_crafting_component; + const requirement_data &reqs = + meal->deduped_requirements().select_alternative( *p, filter ); for( auto it : reqs.get_components() ) { - p->consume_items( it, 1, is_crafting_component ); + p->consume_items( it, 1, filter ); } it->set_var( "RECIPE", meal->ident().str() ); diff --git a/src/player.h b/src/player.h index b8442775a3ee2..5eaf7322e556e 100644 --- a/src/player.h +++ b/src/player.h @@ -1091,6 +1091,9 @@ class player : public Character const inventory &crafting_inventory( bool clear_path ); const inventory &crafting_inventory( const tripoint &src_pos = tripoint_zero, int radius = PICKUP_RANGE, bool clear_path = true ); + const requirement_data &select_requirements( + const std::vector &, int batch, const inventory &, + const std::function &filter ) const; comp_selection select_item_component( const std::vector &components, int batch, inventory &map_inv, bool can_cancel = false, diff --git a/src/recipe.cpp b/src/recipe.cpp index e0ac262b4ddc3..be51cea2b84ab 100644 --- a/src/recipe.cpp +++ b/src/recipe.cpp @@ -273,6 +273,8 @@ void recipe::finalize() requirements_.consolidate(); } + deduped_requirements_ = deduped_requirement_data( requirements_, ident() ); + if( contained && container == "null" ) { container = item::find_type( result_ )->default_container.value_or( "null" ); } @@ -613,7 +615,7 @@ bool recipe::hot_result() const // // TODO: Make this less of a hack if( create_result().is_food() ) { - const requirement_data::alter_tool_comp_vector &tool_lists = requirements().get_tools(); + const requirement_data::alter_tool_comp_vector &tool_lists = simple_requirements().get_tools(); for( const std::vector &tools : tool_lists ) { for( const tool_comp &t : tools ) { if( t.type == "hotplate" ) { diff --git a/src/recipe.h b/src/recipe.h index 89374a519f572..81bde41ed21ac 100644 --- a/src/recipe.h +++ b/src/recipe.h @@ -60,11 +60,19 @@ class recipe int time = 0; // in movement points (100 per turn) int difficulty = 0; - /** Fetch combined requirement data (inline and via "using" syntax) */ - const requirement_data &requirements() const { + /** Fetch combined requirement data (inline and via "using" syntax). + * + * Use simple_requirements() for player display or when you just want to + * know the requirements as listed in the json files. Use + * deduped_requirements() to calculate actual craftability of a recipe. */ + const requirement_data &simple_requirements() const { return requirements_; } + const deduped_requirement_data &deduped_requirements() const { + return deduped_requirements_; + } + const recipe_id &ident() const { return ident_; } @@ -85,7 +93,11 @@ class recipe /** If recipe can be used for disassembly fetch the combined requirements */ requirement_data disassembly_requirements() const { - return reversible ? requirements().disassembly_requirements() : requirement_data(); + if( reversible ) { + return simple_requirements().disassembly_requirements(); + } else { + return {}; + } } /// @returns The name (@ref item::nname) of the resulting item (@ref result). @@ -183,6 +195,9 @@ class recipe /** Combined requirements cached when recipe finalized */ requirement_data requirements_; + /** Deduped version constructed from the above requirements_ */ + deduped_requirement_data deduped_requirements_; + std::set flags; /** If set (zero or positive) set charges of output result for items counted by charges */ diff --git a/src/recipe_dictionary.cpp b/src/recipe_dictionary.cpp index 68efd587926c7..e5f5f2802abf6 100644 --- a/src/recipe_dictionary.cpp +++ b/src/recipe_dictionary.cpp @@ -164,13 +164,13 @@ std::vector recipe_subset::search( const std::string &txt, return lcmatch( r->skill_used->name(), txt ); case search_type::component: - return search_reqs( r->requirements().get_components(), txt ); + return search_reqs( r->simple_requirements().get_components(), txt ); case search_type::tool: - return search_reqs( r->requirements().get_tools(), txt ); + return search_reqs( r->simple_requirements().get_tools(), txt ); case search_type::quality: - return search_reqs( r->requirements().get_qualities(), txt ); + return search_reqs( r->simple_requirements().get_qualities(), txt ); case search_type::quality_result: { const auto &quals = item::find_type( r->result() )->qualities; @@ -378,7 +378,7 @@ void recipe_dictionary::find_items_on_loops() } std::vector &potential_components = potential_components_of[i->get_id()]; for( const recipe_id &rec : i->recipes ) { - const requirement_data requirements = rec->requirements(); + const requirement_data requirements = rec->simple_requirements(); const requirement_data::alter_item_comp_vector &component_requirements = requirements.get_components(); @@ -504,7 +504,7 @@ void recipe_subset::include( const recipe *r, int custom_difficulty ) } } else { // add recipe to category and component caches - for( const auto &opts : r->requirements().get_components() ) { + for( const auto &opts : r->simple_requirements().get_components() ) { for( const item_comp &comp : opts ) { component[comp.type].insert( r ); } diff --git a/src/requirements.cpp b/src/requirements.cpp index 18a2d83b51728..e5bdce9db60d1 100644 --- a/src/requirements.cpp +++ b/src/requirements.cpp @@ -572,7 +572,7 @@ std::vector requirement_data::get_folded_tools_list( int width, nc_ } bool requirement_data::can_make_with_inventory( const inventory &crafting_inv, - const std::function &filter, int batch ) const + const std::function &filter, int batch, craft_flags flags ) const { if( g->u.has_trait( trait_DEBUG_HS ) ) { return true; @@ -583,7 +583,7 @@ bool requirement_data::can_make_with_inventory( const inventory &crafting_inv, if( !has_comps( crafting_inv, qualities, return_true ) ) { retval = false; } - if( !has_comps( crafting_inv, tools, return_true, batch ) ) { + if( !has_comps( crafting_inv, tools, return_true, batch, flags ) ) { retval = false; } if( !has_comps( crafting_inv, components, filter, batch ) ) { @@ -599,7 +599,7 @@ template bool requirement_data::has_comps( const inventory &crafting_inv, const std::vector< std::vector > &vec, const std::function &filter, - int batch ) + int batch, craft_flags flags ) { bool retval = true; int total_UPS_charges_used = 0; @@ -607,7 +607,8 @@ bool requirement_data::has_comps( const inventory &crafting_inv, bool has_tool_in_set = false; int UPS_charges_used = std::numeric_limits::max(); for( const auto &tool : set_of_tools ) { - if( tool.has( crafting_inv, filter, batch, [ &UPS_charges_used ]( int charges ) { + if( tool.has( crafting_inv, filter, batch, flags, + [ &UPS_charges_used ]( int charges ) { UPS_charges_used = std::min( UPS_charges_used, charges ); } ) ) { tool.available = a_true; @@ -631,8 +632,9 @@ bool requirement_data::has_comps( const inventory &crafting_inv, return retval; } -bool quality_requirement::has( const inventory &crafting_inv, - const std::function &, int, std::function ) const +bool quality_requirement::has( + const inventory &crafting_inv, const std::function &, int, + craft_flags, std::function ) const { if( g->u.has_trait( trait_DEBUG_HS ) ) { return true; @@ -649,9 +651,9 @@ nc_color quality_requirement::get_color( bool has_one, const inventory &, return has_one ? c_dark_gray : c_red; } -bool tool_comp::has( const inventory &crafting_inv, - const std::function &filter, int batch, - std::function visitor ) const +bool tool_comp::has( + const inventory &crafting_inv, const std::function &filter, int batch, + craft_flags flags, std::function visitor ) const { if( g->u.has_trait( trait_DEBUG_HS ) ) { return true; @@ -659,7 +661,11 @@ bool tool_comp::has( const inventory &crafting_inv, if( !by_charges() ) { return crafting_inv.has_tools( type, std::abs( count ), filter ); } else { - const int charges_required = count * batch * item::find_type( type )->charge_factor(); + int charges_required = count * batch * item::find_type( type )->charge_factor(); + + if( ( flags & craft_flags::start_only ) != craft_flags::none ) { + charges_required = charges_required / 20 + charges_required % 20; + } int charges_found = crafting_inv.charges_of( type, charges_required, filter, visitor ); return charges_found == charges_required; @@ -677,9 +683,9 @@ nc_color tool_comp::get_color( bool has_one, const inventory &crafting_inv, return has_one ? c_dark_gray : c_red; } -bool item_comp::has( const inventory &crafting_inv, - const std::function &filter, int batch, - std::function ) const +bool item_comp::has( + const inventory &crafting_inv, const std::function &filter, int batch, + craft_flags, std::function ) const { if( g->u.has_trait( trait_DEBUG_HS ) ) { return true; @@ -1162,7 +1168,8 @@ static void expand_item_in_reqs( } } -deduped_requirement_data::deduped_requirement_data( const requirement_data &in ) +deduped_requirement_data::deduped_requirement_data( const requirement_data &in, + const recipe_id &context ) { // This constructor works through a requirement_data, converting it into an // equivalent set of requirement_data alternatives, where each alternative @@ -1237,11 +1244,58 @@ deduped_requirement_data::deduped_requirement_data( const requirement_data &in ) // Because this algorithm is super-exponential in the worst case, add a // sanity check to prevent things getting too far out of control. - static constexpr size_t max_alternatives = 20; + // The worst case in the core game currently is chainmail_suit_faraday + // with 63 alternatives. + static constexpr size_t max_alternatives = 100; if( alternatives_.size() + pending.size() > max_alternatives ) { debugmsg( "Construction of deduped_requirement_data generated too many alternatives. " - "The recipe at fault should be simplified." ); + "The recipe %s should be simplified.", context.str() ); abort(); } } + + // Use this to find demanding recipes without aborting entirely + //if( alternatives_.size() > 50 ) { + // debugmsg( "Recipe %s has %zu alternatives, which is quite high.", + // context.str(), alternatives_.size() ); + //} +} + +bool deduped_requirement_data::can_make_with_inventory( + const inventory &crafting_inv, const std::function &filter, + int batch, craft_flags flags ) const +{ + return std::any_of( alternatives().begin(), alternatives().end(), + [&]( const requirement_data & alt ) { + return alt.can_make_with_inventory( crafting_inv, filter, batch, flags ); + } ); +} + +std::vector deduped_requirement_data::feasible_alternatives( + const inventory &crafting_inv, const std::function &filter, + int batch, craft_flags flags ) const +{ + std::vector result; + for( const requirement_data &req : alternatives() ) { + if( req.can_make_with_inventory( crafting_inv, filter, batch, flags ) ) { + result.push_back( &req ); + } + } + return result; +} + +const requirement_data &deduped_requirement_data::select_alternative( + player &crafter, const std::function &filter, int batch, + craft_flags flags ) const +{ + return select_alternative( crafter, crafter.crafting_inventory(), filter, batch, flags ); +} + +const requirement_data &deduped_requirement_data::select_alternative( + player &crafter, const inventory &inv, const std::function &filter, + int batch, craft_flags flags ) const +{ + const std::vector all_reqs = + feasible_alternatives( inv, filter, batch, flags ); + return crafter.select_requirements( all_reqs, 1, inv, filter ); } diff --git a/src/requirements.h b/src/requirements.h index f169579cabda3..336fde850067f 100644 --- a/src/requirements.h +++ b/src/requirements.h @@ -9,6 +9,7 @@ #include #include +#include "crafting.h" #include "string_id.h" #include "translations.h" #include "type_id.h" @@ -79,7 +80,8 @@ struct tool_comp : public component { void load( const JsonValue &value ); bool has( const inventory &crafting_inv, const std::function &filter, - int batch = 1, std::function visitor = std::function() ) const; + int batch = 1, craft_flags = craft_flags::none, + std::function visitor = std::function() ) const; std::string to_string( int batch = 1, int avail = 0 ) const; nc_color get_color( bool has_one, const inventory &crafting_inv, const std::function &filter, int batch = 1 ) const; @@ -95,7 +97,8 @@ struct item_comp : public component { void load( const JsonValue &value ); bool has( const inventory &crafting_inv, const std::function &filter, - int batch = 1, std::function visitor = std::function() ) const; + int batch = 1, craft_flags = craft_flags::none, + std::function visitor = std::function() ) const; std::string to_string( int batch = 1, int avail = 0 ) const; nc_color get_color( bool has_one, const inventory &crafting_inv, const std::function &filter, int batch = 1 ) const; @@ -116,7 +119,8 @@ struct quality_requirement { level( LEVEL ) { } void load( const JsonValue &value ); - bool has( const inventory &crafting_inv, const std::function &filter, int = 0, + bool has( const inventory &crafting_inv, const std::function &filter, + int = 0, craft_flags = craft_flags::none, std::function visitor = std::function() ) const; std::string to_string( int batch = 1, int avail = 0 ) const; void check_consistency( const std::string &display_name ) const; @@ -266,7 +270,8 @@ struct requirement_data { * or is_crafting_component otherwise. */ bool can_make_with_inventory( const inventory &crafting_inv, - const std::function &filter, int batch = 1 ) const; + const std::function &filter, int batch = 1, + craft_flags = craft_flags::none ) const; /** @param filter see @ref can_make_with_inventory */ std::vector get_folded_components_list( int width, nc_color col, @@ -316,8 +321,10 @@ struct requirement_data { static std::string print_missing_objs( const std::string &header, const std::vector< std::vector > &objs ); template - static bool has_comps( const inventory &crafting_inv, const std::vector< std::vector > &vec, - const std::function &filter, int batch = 1 ); + static bool has_comps( + const inventory &crafting_inv, const std::vector< std::vector > &vec, + const std::function &filter, int batch = 1, + craft_flags = craft_flags::none ); template std::vector get_folded_list( int width, const inventory &crafting_inv, @@ -360,11 +367,28 @@ class deduped_requirement_data public: using alter_item_comp_vector = requirement_data::alter_item_comp_vector; - deduped_requirement_data( const requirement_data & ); + deduped_requirement_data() = default; + deduped_requirement_data( const requirement_data &, const recipe_id &context ); std::vector const &alternatives() const { return alternatives_; } + + std::vector feasible_alternatives( + const inventory &crafting_inv, const std::function &filter, + int batch = 1, craft_flags = craft_flags::none ) const; + + const requirement_data &select_alternative( + player &, const std::function &filter, int batch = 1, + craft_flags = craft_flags::none ) const; + + const requirement_data &select_alternative( + player &, const inventory &, const std::function &filter, + int batch = 1, craft_flags = craft_flags::none ) const; + + bool can_make_with_inventory( + const inventory &crafting_inv, const std::function &filter, + int batch = 1, craft_flags = craft_flags::none ) const; private: std::vector alternatives_; }; diff --git a/tests/comestible_tests.cpp b/tests/comestible_tests.cpp index a18d06a17d55a..4d51d5285829c 100644 --- a/tests/comestible_tests.cpp +++ b/tests/comestible_tests.cpp @@ -127,8 +127,9 @@ TEST_CASE( "recipe_permutations", "[recipe]" ) const bool has_override = res_it.has_flag( "NUTRIENT_OVERRIDE" ); if( is_food && !has_override ) { // Collection of kcal values of all ingredient permutations - all_stats mystats = run_stats( recipe_permutations( recipe_obj.requirements().get_components() ), - byproduct_calories( recipe_obj ) ); + all_stats mystats = run_stats( + recipe_permutations( recipe_obj.simple_requirements().get_components() ), + byproduct_calories( recipe_obj ) ); if( mystats.calories.n() < 2 ) { continue; } diff --git a/tests/crafting_test.cpp b/tests/crafting_test.cpp index a3ec63c051d9a..94a64506051f1 100644 --- a/tests/crafting_test.cpp +++ b/tests/crafting_test.cpp @@ -273,10 +273,9 @@ static void prep_craft( const recipe_id &rid, const std::vector &tools, const recipe &r = rid.obj(); - const requirement_data &reqs = r.requirements(); inventory crafting_inv = g->u.crafting_inventory(); - bool can_craft = reqs.can_make_with_inventory( g->u.crafting_inventory(), - r.get_component_filter() ); + bool can_craft = r.deduped_requirements().can_make_with_inventory( + crafting_inv, r.get_component_filter() ); CHECK( can_craft == expect_craftable ); } diff --git a/tests/requirements_test.cpp b/tests/requirements_test.cpp index fc75bb7a8d2e3..7a4628eb05319 100644 --- a/tests/requirements_test.cpp +++ b/tests/requirements_test.cpp @@ -8,7 +8,7 @@ static void test_requirement_deduplication( ) { requirement_data in( {}, {}, before ); - deduped_requirement_data out( in ); + deduped_requirement_data out( in, recipe_id::NULL_ID() ); CHECK( out.alternatives().size() == after.size() ); while( after.size() < out.alternatives().size() ) { after.emplace_back(); From e1491563b94962017c75199d2bd8fd52a530b4f3 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Fri, 3 Jan 2020 19:49:09 -0500 Subject: [PATCH 06/10] Explain uncraftability due to overlaps When a recipe is not craftable, but all the ingredients are green in the crafting GUI, this might be confusing for players. Explain to them what's going on in this case by adding a new message. --- src/crafting_gui.cpp | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/crafting_gui.cpp b/src/crafting_gui.cpp index b08d56dc42242..45b913778a824 100644 --- a/src/crafting_gui.cpp +++ b/src/crafting_gui.cpp @@ -194,13 +194,20 @@ const recipe *select_crafting_recipe( int &batch_size ) std::vector current; struct availability { - availability( const recipe *r, int batch_size = 1 ) : - can_craft( g->u.can_start_craft( r, recipe_filter_flags::none, batch_size ) ), - can_craft_non_rotten( g->u.can_start_craft( r, recipe_filter_flags::no_rotten, - batch_size ) ) - {} + availability( const recipe *r, int batch_size = 1 ) { + inventory inv = g->u.crafting_inventory(); + auto all_items_filter = r->get_component_filter( recipe_filter_flags::none ); + auto no_rotten_filter = r->get_component_filter( recipe_filter_flags::no_rotten ); + const deduped_requirement_data &req = r->deduped_requirements(); + can_craft = req.can_make_with_inventory( inv, all_items_filter, batch_size ); + can_craft_non_rotten = req.can_make_with_inventory( inv, no_rotten_filter, batch_size ); + const requirement_data &simple_req = r->simple_requirements(); + apparently_craftable = + simple_req.can_make_with_inventory( inv, all_items_filter, batch_size ); + } bool can_craft; bool can_craft_non_rotten; + bool apparently_craftable; nc_color selected_color() const { return can_craft ? can_craft_non_rotten ? h_white : h_brown : h_dark_gray; @@ -603,6 +610,12 @@ const recipe *select_crafting_recipe( int &batch_size ) ypos += fold_and_print( w_data, point( xpos, ypos ), pane, col, _( "Will use rotten ingredients" ) ); } + if( !available[line].can_craft && available[line].apparently_craftable ) { + ypos += fold_and_print( + w_data, point( xpos, ypos ), pane, col, + _( "Cannot be crafted because the same item is needed " + "for multiple components" ) ); + } ypos += print_items( *current[line], w_data, ypos, xpos, col, batch ? line + 1 : 1 ); } From dc726406b34e1f26a8c4783d6cab869c6bc67851 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Sun, 5 Jan 2020 20:11:13 -0500 Subject: [PATCH 07/10] Allow player to select from deduped requirements Previously, when deduped_requirements_data contained multiple feasible selections of components, the game would always arbitrarily pick the first one, meaning that the player might consume items they had not intended to. Now, instead, offer the choice to the player in this situation. This is done via a uilist dialogue. To trim down the amount rendered in the dialogue, I added a new feature to requirements to format a requirements list while only showing those options which are available (the green ones) and use that here. As a side-effect, the player can now cancel the choice, so all the places calling this function now need to deal with the potential failure to make a choice. --- src/basecamp.cpp | 10 +++++++--- src/craft_command.cpp | 9 ++++++--- src/crafting.cpp | 35 ++++++++++++++++++++++++++++++----- src/crafting_gui.cpp | 10 ++++++---- src/iuse.cpp | 8 ++++++-- src/player.h | 2 +- src/requirements.cpp | 17 +++++++++++------ src/requirements.h | 27 +++++++++++++++++++++------ 8 files changed, 88 insertions(+), 30 deletions(-) diff --git a/src/basecamp.cpp b/src/basecamp.cpp index ac77a3719ef99..9ff6829e41bee 100644 --- a/src/basecamp.cpp +++ b/src/basecamp.cpp @@ -690,12 +690,16 @@ basecamp_action_components::basecamp_action_components( bool basecamp_action_components::choose_components() { const auto filter = is_crafting_component; - const requirement_data &req = making_.deduped_requirements().select_alternative( g->u, filter ); + const requirement_data *req = + making_.deduped_requirements().select_alternative( g->u, base_._inv, filter, batch_size_ ); + if( !req ) { + return false; + } if( !item_selections_.empty() || !tool_selections_.empty() ) { debugmsg( "Reused basecamp_action_components" ); return false; } - for( const auto &it : req.get_components() ) { + for( const auto &it : req->get_components() ) { comp_selection is = g->u.select_item_component( it, batch_size_, base_._inv, true, filter, !base_.by_radio ); @@ -705,7 +709,7 @@ bool basecamp_action_components::choose_components() item_selections_.push_back( is ); } // this may consume pseudo-resources from fake items - for( const auto &it : req.get_tools() ) { + for( const auto &it : req->get_tools() ) { comp_selection ts = g->u.select_tool_component( it, batch_size_, base_._inv, DEFAULT_HOTKEYS, true, !base_.by_radio ); diff --git a/src/craft_command.cpp b/src/craft_command.cpp index dc112e91bc733..aee928dd36ef7 100644 --- a/src/craft_command.cpp +++ b/src/craft_command.cpp @@ -145,10 +145,13 @@ void craft_command::execute( const tripoint &new_loc ) item_selections.clear(); const auto filter = rec->get_component_filter( flags ); - const requirement_data &needs = rec->deduped_requirements().select_alternative( + const requirement_data *needs = rec->deduped_requirements().select_alternative( *crafter, filter, batch_size, craft_flags::start_only ); + if( !needs ) { + return; + } - for( const auto &it : needs.get_components() ) { + for( const auto &it : needs->get_components() ) { comp_selection is = crafter->select_item_component( it, batch_size, map_inv, true, filter ); if( is.use_from == cancel ) { @@ -158,7 +161,7 @@ void craft_command::execute( const tripoint &new_loc ) } tool_selections.clear(); - for( const auto &it : needs.get_tools() ) { + for( const auto &it : needs->get_tools() ) { comp_selection ts = crafter->select_tool_component( it, batch_size, map_inv, DEFAULT_HOTKEYS, true, true, []( int charges ) { return charges / 20 + charges % 20; diff --git a/src/crafting.cpp b/src/crafting.cpp index d01944d7a78b7..1a02ad9944414 100644 --- a/src/crafting.cpp +++ b/src/crafting.cpp @@ -1321,13 +1321,38 @@ bool player::can_continue_craft( item &craft ) return true; } -const requirement_data &player::select_requirements( - const std::vector &alternatives, int /*batch*/, const inventory &, - const std::function &/*filter*/ ) const +const requirement_data *player::select_requirements( + const std::vector &alternatives, int batch, const inventory &inv, + const std::function &filter ) const { assert( !alternatives.empty() ); - // TODO: when this is the avatar, offer the choice to the player - return *alternatives.front(); + if( alternatives.size() == 1 || !is_avatar() ) { + return alternatives.front(); + } + + std::vector descriptions; + + uilist menu; + + for( const requirement_data *req : alternatives ) { + // Write with a large width and then just re-join the lines, because + // uilist does its own wrapping and we want to rely on that. + std::vector component_lines = + req->get_folded_components_list( TERMX - 4, c_light_gray, inv, filter, batch, "", + requirement_display_flags::no_unavailable ); + menu.addentry_desc( "", join( component_lines, "\n" ) ); + } + + menu.allow_cancel = true; + menu.desc_enabled = true; + menu.title = _( "Use which selection of components?" ); + menu.query(); + + if( menu.ret < 0 || static_cast( menu.ret ) >= alternatives.size() ) { + return nullptr; + } + + return alternatives[menu.ret]; } /* selection of component if a recipe requirement has multiple options (e.g. 'duct tap' or 'welder') */ diff --git a/src/crafting_gui.cpp b/src/crafting_gui.cpp index 45b913778a824..4d488146ea832 100644 --- a/src/crafting_gui.cpp +++ b/src/crafting_gui.cpp @@ -199,11 +199,13 @@ const recipe *select_crafting_recipe( int &batch_size ) auto all_items_filter = r->get_component_filter( recipe_filter_flags::none ); auto no_rotten_filter = r->get_component_filter( recipe_filter_flags::no_rotten ); const deduped_requirement_data &req = r->deduped_requirements(); - can_craft = req.can_make_with_inventory( inv, all_items_filter, batch_size ); - can_craft_non_rotten = req.can_make_with_inventory( inv, no_rotten_filter, batch_size ); + can_craft = req.can_make_with_inventory( + inv, all_items_filter, batch_size, craft_flags::start_only ); + can_craft_non_rotten = req.can_make_with_inventory( + inv, no_rotten_filter, batch_size, craft_flags::start_only ); const requirement_data &simple_req = r->simple_requirements(); - apparently_craftable = - simple_req.can_make_with_inventory( inv, all_items_filter, batch_size ); + apparently_craftable = simple_req.can_make_with_inventory( + inv, all_items_filter, batch_size, craft_flags::start_only ); } bool can_craft; bool can_craft_non_rotten; diff --git a/src/iuse.cpp b/src/iuse.cpp index 929da91155ac8..ad62a2efe7503 100644 --- a/src/iuse.cpp +++ b/src/iuse.cpp @@ -8718,9 +8718,13 @@ int iuse::multicooker( player *p, item *it, bool t, const tripoint &pos ) } const auto filter = is_crafting_component; - const requirement_data &reqs = + const requirement_data *reqs = meal->deduped_requirements().select_alternative( *p, filter ); - for( auto it : reqs.get_components() ) { + if( !reqs ) { + return 0; + } + + for( auto it : reqs->get_components() ) { p->consume_items( it, 1, filter ); } diff --git a/src/player.h b/src/player.h index 5eaf7322e556e..140559ec2556d 100644 --- a/src/player.h +++ b/src/player.h @@ -1091,7 +1091,7 @@ class player : public Character const inventory &crafting_inventory( bool clear_path ); const inventory &crafting_inventory( const tripoint &src_pos = tripoint_zero, int radius = PICKUP_RANGE, bool clear_path = true ); - const requirement_data &select_requirements( + const requirement_data *select_requirements( const std::vector &, int batch, const inventory &, const std::function &filter ) const; comp_selection diff --git a/src/requirements.cpp b/src/requirements.cpp index e5bdce9db60d1..2d5d87898d919 100644 --- a/src/requirements.cpp +++ b/src/requirements.cpp @@ -480,7 +480,7 @@ void requirement_data::reset() std::vector requirement_data::get_folded_components_list( int width, nc_color col, const inventory &crafting_inv, const std::function &filter, int batch, - std::string hilite ) const + std::string hilite, requirement_display_flags flags ) const { std::vector out_buffer; if( components.empty() ) { @@ -489,7 +489,7 @@ std::vector requirement_data::get_folded_components_list( int width out_buffer.push_back( colorize( _( "Components required:" ), col ) ); std::vector folded_buffer = - get_folded_list( width, crafting_inv, filter, components, batch, hilite ); + get_folded_list( width, crafting_inv, filter, components, batch, hilite, flags ); out_buffer.insert( out_buffer.end(), folded_buffer.begin(), folded_buffer.end() ); return out_buffer; @@ -498,11 +498,14 @@ std::vector requirement_data::get_folded_components_list( int width template std::vector requirement_data::get_folded_list( int width, const inventory &crafting_inv, const std::function &filter, - const std::vector< std::vector > &objs, int batch, const std::string &hilite ) const + const std::vector< std::vector > &objs, int batch, const std::string &hilite, + requirement_display_flags flags ) const { // hack: ensure 'cached' availability is up to date can_make_with_inventory( crafting_inv, filter ); + bool no_unavailable = static_cast( flags & requirement_display_flags::no_unavailable ); + std::vector out_buffer; for( const auto &comp_list : objs ) { const bool has_one = any_marked_available( comp_list ); @@ -530,7 +533,9 @@ std::vector requirement_data::get_folded_list( int width, color = yellow_background( color ); } - list_as_string.push_back( colorize( text, color ) ); + if( !no_unavailable || component.has( crafting_inv, filter, batch ) ) { + list_as_string.push_back( colorize( text, color ) ); + } buffer_has.push_back( text + color_tag ); } std::sort( list_as_string.begin(), list_as_string.end() ); @@ -1284,14 +1289,14 @@ std::vector deduped_requirement_data::feasible_alterna return result; } -const requirement_data &deduped_requirement_data::select_alternative( +const requirement_data *deduped_requirement_data::select_alternative( player &crafter, const std::function &filter, int batch, craft_flags flags ) const { return select_alternative( crafter, crafter.crafting_inventory(), filter, batch, flags ); } -const requirement_data &deduped_requirement_data::select_alternative( +const requirement_data *deduped_requirement_data::select_alternative( player &crafter, const inventory &inv, const std::function &filter, int batch, craft_flags flags ) const { diff --git a/src/requirements.h b/src/requirements.h index 336fde850067f..8fead88448965 100644 --- a/src/requirements.h +++ b/src/requirements.h @@ -131,6 +131,18 @@ struct quality_requirement { } }; +enum class requirement_display_flags { + none = 0, + no_unavailable = 1, +}; + +inline constexpr requirement_display_flags operator&( requirement_display_flags l, + requirement_display_flags r ) +{ + return static_cast( + static_cast( l ) & static_cast( r ) ); +} + /** * The *_vector members represent list of alternatives requirements: * alter_tool_comp_vector = { * { { a, b }, { c, d } } @@ -275,8 +287,9 @@ struct requirement_data { /** @param filter see @ref can_make_with_inventory */ std::vector get_folded_components_list( int width, nc_color col, - const inventory &crafting_inv, const std::function &filter, int batch = 1, - std::string hilite = "" ) const; + const inventory &crafting_inv, const std::function &filter, + int batch = 1, std::string hilite = "", + requirement_display_flags = requirement_display_flags::none ) const; std::vector get_folded_tools_list( int width, nc_color col, const inventory &crafting_inv, int batch = 1 ) const; @@ -328,8 +341,10 @@ struct requirement_data { template std::vector get_folded_list( int width, const inventory &crafting_inv, - const std::function &filter, const std::vector< std::vector > &objs, - int batch = 1, const std::string &hilite = "" ) const; + const std::function &filter, + const std::vector< std::vector > &objs, int batch = 1, + const std::string &hilite = "", + requirement_display_flags = requirement_display_flags::none ) const; template static bool any_marked_available( const std::vector &comps ); @@ -378,11 +393,11 @@ class deduped_requirement_data const inventory &crafting_inv, const std::function &filter, int batch = 1, craft_flags = craft_flags::none ) const; - const requirement_data &select_alternative( + const requirement_data *select_alternative( player &, const std::function &filter, int batch = 1, craft_flags = craft_flags::none ) const; - const requirement_data &select_alternative( + const requirement_data *select_alternative( player &, const inventory &, const std::function &filter, int batch = 1, craft_flags = craft_flags::none ) const; From c544aea040040c889fce002c7613925b358ff3cb Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Tue, 7 Jan 2020 16:49:01 -0500 Subject: [PATCH 08/10] Appease clang-tidy --- src/basecamp.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/basecamp.cpp b/src/basecamp.cpp index 9ff6829e41bee..e618cedd34622 100644 --- a/src/basecamp.cpp +++ b/src/basecamp.cpp @@ -732,16 +732,16 @@ void basecamp_action_components::consume_components() const tripoint &origin = target_map->getlocal( base_.get_dumping_spot() ); for( const comp_selection &sel : item_selections_ ) { g->u.consume_items( *target_map, sel, batch_size_, is_crafting_component, origin, - base_.inv_range ); + basecamp::inv_range ); } // this may consume pseudo-resources from fake items for( const comp_selection &sel : tool_selections_ ) { - g->u.consume_tools( *target_map, sel, batch_size_, origin, base_.inv_range, &base_ ); + g->u.consume_tools( *target_map, sel, batch_size_, origin, basecamp::inv_range, &base_ ); } // go back and consume the actual resources for( basecamp_resource &bcp_r : base_.resources ) { if( bcp_r.consumed > 0 ) { - target_map->use_charges( origin, base_.inv_range, bcp_r.ammo_id, bcp_r.consumed ); + target_map->use_charges( origin, basecamp::inv_range, bcp_r.ammo_id, bcp_r.consumed ); bcp_r.consumed = 0; } } From cac9295e51f9391d418c521de005bdfe7e335881 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Tue, 7 Jan 2020 16:54:49 -0500 Subject: [PATCH 09/10] Don't copy inventory unnecessarily --- src/crafting.cpp | 4 ++-- src/crafting_gui.cpp | 2 +- tests/crafting_test.cpp | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/crafting.cpp b/src/crafting.cpp index 1a02ad9944414..9516c2802c9de 100644 --- a/src/crafting.cpp +++ b/src/crafting.cpp @@ -472,7 +472,7 @@ std::vector player::get_eligible_containers_for_crafting() const bool player::can_make( const recipe *r, int batch_size ) { - inventory crafting_inv = crafting_inventory(); + const inventory &crafting_inv = crafting_inventory(); if( has_recipe( r, crafting_inv, get_crafting_helpers() ) < 0 ) { return false; @@ -488,7 +488,7 @@ bool player::can_start_craft( const recipe *rec, recipe_filter_flags flags, int return false; } - inventory inv = crafting_inventory(); + const inventory &inv = crafting_inventory(); return rec->deduped_requirements().can_make_with_inventory( inv, rec->get_component_filter( flags ), batch_size, craft_flags::start_only ); } diff --git a/src/crafting_gui.cpp b/src/crafting_gui.cpp index 4d488146ea832..8731941c1b015 100644 --- a/src/crafting_gui.cpp +++ b/src/crafting_gui.cpp @@ -195,7 +195,7 @@ const recipe *select_crafting_recipe( int &batch_size ) struct availability { availability( const recipe *r, int batch_size = 1 ) { - inventory inv = g->u.crafting_inventory(); + const inventory &inv = g->u.crafting_inventory(); auto all_items_filter = r->get_component_filter( recipe_filter_flags::none ); auto no_rotten_filter = r->get_component_filter( recipe_filter_flags::no_rotten ); const deduped_requirement_data &req = r->deduped_requirements(); diff --git a/tests/crafting_test.cpp b/tests/crafting_test.cpp index 94a64506051f1..4f98387a6249d 100644 --- a/tests/crafting_test.cpp +++ b/tests/crafting_test.cpp @@ -273,7 +273,7 @@ static void prep_craft( const recipe_id &rid, const std::vector &tools, const recipe &r = rid.obj(); - inventory crafting_inv = g->u.crafting_inventory(); + const inventory &crafting_inv = g->u.crafting_inventory(); bool can_craft = r.deduped_requirements().can_make_with_inventory( crafting_inv, r.get_component_filter() ); CHECK( can_craft == expect_craftable ); From 8370925069697c023b77d3cf7784df0505859917 Mon Sep 17 00:00:00 2001 From: John Bytheway Date: Tue, 7 Jan 2020 16:55:20 -0500 Subject: [PATCH 10/10] Minor changes from review --- src/requirements.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/requirements.cpp b/src/requirements.cpp index 2d5d87898d919..dd6b9d7d852c9 100644 --- a/src/requirements.cpp +++ b/src/requirements.cpp @@ -504,7 +504,8 @@ std::vector requirement_data::get_folded_list( int width, // hack: ensure 'cached' availability is up to date can_make_with_inventory( crafting_inv, filter ); - bool no_unavailable = static_cast( flags & requirement_display_flags::no_unavailable ); + const bool no_unavailable = + static_cast( flags & requirement_display_flags::no_unavailable ); std::vector out_buffer; for( const auto &comp_list : objs ) { @@ -1107,7 +1108,7 @@ void requirement_data::consolidate() /// would otherwise be duplicated between two requirements. /// /// It operates recursively (increasing @p index with the depth of recursion), -/// searching for another item_comp to marge @p leftover with. For each +/// searching for another item_comp to merge @p leftover with. For each /// compatible item_comp found it performs that merger and writes out a /// suitably updated form of the overall requirements to @p result. ///