-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Craft Gui show amounts #43393
Craft Gui show amounts #43393
Conversation
src/crafting_gui.cpp
Outdated
} | ||
} | ||
ypos += fold_and_print( w_data, point( xpos, ypos ), pane, col, | ||
_( "Nearby: <color_cyan>%s</color>" ), nearby_string ); |
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.
I think just "Nearby" is a little ambiguous, and there's certainly space in the UI to clarify a little more. I'm not sure how best to tweak it; maybe "Already present nearby: %s" or "There are already %s nearby"?
Also, it seems like the color_cyan
tag is redundant here; there's always another colour inside it.
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.
cyan tag is a result of me not paying attention to copy and paste when I rewrote it.
I struggled with what the label should be but ended up deciding to be as general as possible because I didn't want to get caught up with amount/charges/portions discrepancies.
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.
I think you need to at least make it clear that it's the recipe output that's being counted here, otherwise it's just an arbitrary number without context.
src/inventory.cpp
Outdated
@@ -1034,6 +1034,20 @@ enchantment inventory::get_active_enchantment_cache( const Character &owner ) co | |||
return temp_cache; | |||
} | |||
|
|||
int inventory::count_item( const itype_id& item_type ) const |
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.
Surely we already have a function that can do this? Can't you use the code already used to look for crafting ingredients, asking for 9000 of whatever item. Since there's feedback in the UI about how many items you have when you're short, it must be returning that partial count.
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.
I spent some time looking around and while there were a couple I could theoretically use, they all seemed to be either threshold driven (has_components) or, charges vs amounts specific (visitable amount_of, charges_of). Since I would have to write a wrapper to use those anyways, I figured it would be better to just write a function that did what I wanted.
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.
I think a threshold-driven one might be a good option. As you already pointed out, you don't need to count all when there are many.
Summary
SUMMARY: Interface "Crafting GUI - Show nearby + produced amounts"
Purpose of change
Resolves #29546
also resolves another ticket for the nearby amounts that I can't seem to find
Describe the solution
Add count of amount/charges nearby and, amount produced if >1 to recipe info
Testing
Getting build errors on the tests but, went through a few rounds of manual testing before I got it to all work consistently.
Additional context
Just came back to Cataclysm after ~20 months \(^v^)/