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.
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
Multi-filter for recipes in crafting menu #27500
Merged
kevingranade
merged 10 commits into
CleverRaven:master
from
OrenAudeles:crafting_multi_filter
Jan 12, 2019
Merged
Changes from 8 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 a2d66b6
astyle
OrenAudeles 7b039c7
Remove unnecessary variable controlling filter loop
OrenAudeles db8ddc1
Reduce code duplication in filter loop
OrenAudeles 8384a14
Fix initial population if 'm' or 'h' are first filters
OrenAudeles c10c6b9
End filter loop if zero results found at any point
OrenAudeles 61d1bc2
Update filter description to include multi-filter delimiter
OrenAudeles 46493a2
Filter loop now uses reduce/intersection/difference functions to get …
OrenAudeles 74604c1
astyle
OrenAudeles 05afce5
Add new recipe_subset ctor, use it to clean up filter functions
OrenAudeles File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be a
recipe_subset
constructor, rather than a static member function (Assuming I'm correct in thinking that you only ever use it to insert into an emptyrecipe_subset
).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your assumption is correct, only ever adding to an empty
recipe_subset
. A constructor would take theconst recipe_subset &src
andconst std::vector<const recipe *> &recipes
arguments and transplant the functionality I guess.recipe_subset
doesn't currently have a constructor so I would need to define additional constructors (empty/copy at minimum). This is the only situation where we have a possible want for a new constructor, and at the moment would only be useful in the reduce/intersection/difference functions.Unless there is a broader requirement for new constructors I am reluctant to add one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to add a copy constructor; the default continues to exist even if you define others. And the default constructor is just a one line
recipe_subset() = default;
.Not vital; I think it would be neater, but that's subjective.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah okay, then my argument against is mostly moot. I'll try out the addition of
recipe_subset() = default;
andrecipe_subset( const recipe_subset &src, const std::vector<const recipe *> );
constructors. Probably is neater, at the very least it should clean up thereduce
/intersection
/difference
functions a little.