Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Multi-filter for recipes in crafting menu #27500

Merged
merged 10 commits into from Jan 12, 2019
Merged

Multi-filter for recipes in crafting menu #27500

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b1572c2
allow multiple recipe filters in crafting menu
OrenAudeles Jan 8, 2019
a2d66b6
astyle
OrenAudeles Jan 8, 2019
7b039c7
Remove unnecessary variable controlling filter loop
OrenAudeles Jan 8, 2019
db8ddc1
Reduce code duplication in filter loop
OrenAudeles Jan 8, 2019
8384a14
Fix initial population if 'm' or 'h' are first filters
OrenAudeles Jan 8, 2019
c10c6b9
End filter loop if zero results found at any point
OrenAudeles Jan 8, 2019
61d1bc2
Update filter description to include multi-filter delimiter
OrenAudeles Jan 8, 2019
46493a2
Filter loop now uses reduce/intersection/difference functions to get …
OrenAudeles Jan 8, 2019
74604c1
astyle
OrenAudeles Jan 8, 2019
05afce5
Add new recipe_subset ctor, use it to clean up filter functions
OrenAudeles Jan 9, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
146 changes: 93 additions & 53 deletions src/crafting_gui.cpp
View file
Open in desktop
Expand Up @@ -258,64 +258,104 @@ const recipe *select_crafting_recipe( int &batch_size )
std::vector<const recipe *> picking;
if( !filterstring.empty() ) {
auto qry = trim( filterstring );
if( qry.size() > 2 && qry[1] == ':' ) {
switch( qry[0] ) {
case 't':
picking = available_recipes.search( qry.substr( 2 ), recipe_subset::search_type::tool );
break;

case 'c':
picking = available_recipes.search( qry.substr( 2 ), recipe_subset::search_type::component );
break;

case 's':
picking = available_recipes.search( qry.substr( 2 ), recipe_subset::search_type::skill );
break;

case 'p':
picking = available_recipes.search( qry.substr( 2 ), recipe_subset::search_type::primary_skill );
break;

case 'Q':
picking = available_recipes.search( qry.substr( 2 ), recipe_subset::search_type::quality );
break;

case 'q':
picking = available_recipes.search( qry.substr( 2 ), recipe_subset::search_type::quality_result );
break;

case 'd':
picking = available_recipes.search( qry.substr( 2 ),
recipe_subset::search_type::description_result );
break;

case 'm': {
auto &learned = g->u.get_learned_recipes();
if( query_is_yes( qry ) ) {
std::set_intersection( available_recipes.begin(), available_recipes.end(), learned.begin(),
learned.end(), std::back_inserter( picking ) );
} else {
std::set_difference( available_recipes.begin(), available_recipes.end(), learned.begin(),
learned.end(),
std::back_inserter( picking ) );
size_t qry_begin = 0;
size_t qry_end = 0;
do {
// Find next ','
qry_end = qry.find_first_of( ',', qry_begin );

auto qry_filter_str = trim( qry.substr( qry_begin, qry_end - qry_begin ) );
// First call to ::search will do initial population of filtered recipes.
// Following calls will reduce that population.
if( qry_filter_str.size() > 2 && qry_filter_str[1] == ':' ) {
switch( qry_filter_str[0] ) {
case 't':
picking = available_recipes.search( picking, qry_filter_str.substr( 2 ),
recipe_subset::search_type::tool );
break;

case 'c':
picking = available_recipes.search( picking, qry_filter_str.substr( 2 ),
recipe_subset::search_type::component );
break;

case 's':
picking = available_recipes.search( picking, qry_filter_str.substr( 2 ),
recipe_subset::search_type::skill );
break;

case 'p':
picking = available_recipes.search( picking, qry_filter_str.substr( 2 ),
recipe_subset::search_type::primary_skill );
break;

case 'Q':
picking = available_recipes.search( picking, qry_filter_str.substr( 2 ),
recipe_subset::search_type::quality );
break;

case 'q':
picking = available_recipes.search( picking, qry_filter_str.substr( 2 ),
recipe_subset::search_type::quality_result );
break;

case 'd':
picking = available_recipes.search( picking, qry_filter_str.substr( 2 ),
recipe_subset::search_type::description_result );
break;

case 'm': {
auto &learned = g->u.get_learned_recipes();
std::vector<const recipe *> intersection_result;
if( query_is_yes( qry_filter_str ) ) {
std::set_intersection( available_recipes.begin(), available_recipes.end(), learned.begin(),
learned.end(), std::back_inserter( intersection_result ) );
} else {
std::set_difference( available_recipes.begin(), available_recipes.end(), learned.begin(),
learned.end(),
std::back_inserter( intersection_result ) );
}
// Populate if the first query filter, reduce otherwise
if( qry_begin == 0 ) {
picking.swap( intersection_result );
} else {
std::vector<const recipe *> col_result;
std::set_intersection( intersection_result.begin(), intersection_result.end(), picking.begin(),
picking.end(), std::back_inserter( col_result ) );
picking.swap( col_result );
}
break;
}
break;
}

case 'h': {
std::copy( available_recipes.begin(), available_recipes.end(), std::back_inserter( picking ) );
if( query_is_yes( qry ) ) {
show_hidden = true;
case 'h': {
std::vector<const recipe *> available;
std::copy( available_recipes.begin(), available_recipes.end(), std::back_inserter( available ) );
std::vector<const recipe *> col_result;

// Populate if the first query filter, reduce otherwise
if( qry_begin == 0 ) {
picking.swap( available );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using swap to assign a vector is entirely correct, but no longer necessary in a C++11 world. It would probably be less confusing to move-assign.

(Similarly for col_result below)

} else {
std::set_intersection( available.begin(), available.end(), picking.begin(), picking.end(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

col_result can be declared inside this else block.

std::back_inserter( col_result ) );
picking.swap( col_result );
}

if( query_is_yes( qry_filter_str ) ) {
show_hidden = true;
}
break;
}
break;
}

default:
current.clear();
default:
current.clear();
}
} else {
picking = available_recipes.search( picking, qry_filter_str );
}
} else {
picking = available_recipes.search( qry );
}

qry_begin = qry_end + 1;
} while( qry_end != std::string::npos && !picking.empty() );
} else if( subtab.cur() == "CSC_*_FAVORITE" ) {
picking = available_recipes.favorite();
} else if( subtab.cur() == "CSC_*_RECENT" ) {
Expand Down
53 changes: 53 additions & 0 deletions src/recipe_dictionary.cpp
View file
Open in desktop
Expand Up @@ -120,7 +120,60 @@ std::vector<const recipe *> recipe_subset::recent() const

return res;
}
std::vector<const recipe *> recipe_subset::search( const std::vector<const recipe *> subset,
const std::string &txt,
const search_type key ) const
{
std::vector<const recipe *> res;

// Subset is not empty, so we need to filter from its data
if( !subset.empty() ) {
std::copy_if( subset.begin(), subset.end(), std::back_inserter( res ), [&]( const recipe * r ) {
if( !*r ) {
return false;
}
switch( key ) {
case search_type::name:
return lcmatch( r->result_name(), txt );

case search_type::skill:
return lcmatch( r->required_skills_string( nullptr ), txt ) ||
lcmatch( r->skill_used->name(), txt );

case search_type::primary_skill:
return lcmatch( r->skill_used->name(), txt );

case search_type::component:
return search_reqs( r->requirements().get_components(), txt );

case search_type::tool:
return search_reqs( r->requirements().get_tools(), txt );

case search_type::quality:
return search_reqs( r->requirements().get_qualities(), txt );

case search_type::quality_result: {
const auto &quals = item::find_type( r->result() )->qualities;
return std::any_of( quals.begin(), quals.end(), [&]( const std::pair<quality_id, int> &e ) {
return lcmatch( e.first->name, txt );
} );
}

case search_type::description_result: {
const item result = r->create_result();
return lcmatch( remove_color_tags( result.info( true ) ), txt );
}

default:
return false;
}
} );
} else { // Subset is empty, populate from recipe_subset data
res = search( txt, key );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the subset is empty you are resetting the search to search from all known recipes once more? Perhaps I have misunderstood, but that seems like it could lead to oddities. If the first few search terms filter down to zero results, then suddenly it will expand again to all recipes matching the next filter, effectively ignoring those first few search terms.

Edit: I see you abort the loop once the subset is empty, so this shouldn't happen. It's still confusing code, though. Is this alternative branch really needed?

}

return res;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new recipe_subset::search has a lot of code duplication with the existing one. It would be preferable to not duplicate this logic.

I think the simplest way to achieve that is to have the old function call the new one (rather than the other way around, as you currently have). Alternatively, both of them could call some third (new) function.

std::vector<const recipe *> recipe_subset::search( const std::string &txt,
const search_type key ) const
{
Expand Down
4 changes: 4 additions & 0 deletions src/recipe_dictionary.h
View file
Open in desktop
Expand Up @@ -133,6 +133,10 @@ class recipe_subset
/** Find recipes matching query (left anchored partial matches are supported) */
std::vector<const recipe *> search( const std::string &txt,
const search_type key = search_type::name ) const;
/** Find recipes matching query existing in subset of recipes (left anchored partial matches are supported) */
std::vector<const recipe *> search( const std::vector<const recipe *> subset,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit strange to be asking a recipe_subset object to filter a different subset of recipes. The name suggests that the recipe_subset already represents a subset of recipes. So this interface is confusing (though I don't blame you for taking this approach; it's straightforward).

I feel like a more logical solution (which requires more changes) is that recipe_subset::search should not return a vector<recipe*>, but instead return a new recipe_subset object representing the new, smaller subset. Then you would not need to add this new function; it would simply be a matter of repeated search calls. That would naturally fix some of my objections above.

This is just an idea; I haven't thought it through in detail; it may not work out in practice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works surprisingly well actually! Gets rid of that second search function, made a new function reduce that calls into search before returning a recipe_subset reducible by future reduce calls :D

const std::string &txt,
const search_type key = search_type::name ) const;
/** Find recipes producing the item */
std::vector<const recipe *> search_result( const itype_id &item ) const;

Expand Down