fix detergent use in washing#87668
Conversation
Fixes CleverRaven#87507. This was a regression introduced by CleverRaven#87543, which removed charges from the detergent item, but did not change the C++ code for washing to use detergent amounts instead of charges. I grepped through the codebase for similar cases for the other items modified in CleverRaven#87543, but this appears to be the only one. However, the regex I used would not catch situations like the washing machine, which use a flag rather than the item id.
|
I haven't been able to reproduce the test failure locally with |
| std::vector<const item *> detergents = inv.items_with( [inv]( const item & it ) { | ||
| return it.has_flag( json_flag_DETERGENT ) && inv.has_charges( it.typeId(), 5 ); | ||
| return it.has_flag( json_flag_DETERGENT ) && | ||
| ( it.count_by_charges() ? inv.has_charges( it.typeId(), 5 ) : inv.has_amount( it.typeId(), 5 ) ); |
There was a problem hiding this comment.
I'm not sure that count_by_charges is actually the correct call here. Should this be can_have_charges on the item type instead?
There was a problem hiding this comment.
Looking at the code for can_have_charges(), it seems to cover too much, as it will return true for tools that can have charges, which you wouldn't want. It also covers count_by_charges() and the flag "json_flag_CAN_HAVE_CHARGES", whatever that does.
What I think you'd actually want is to access the "stackable_" field of the item, but it's private and only reachable via count_by_charges().
I would suggest the test for "json_flag_DETERGENT" would weed out the undesirable charge things, though (ammo and non solid comestibles), which should work as long as soap isn't made comestible (I think soap made from natural ingredients is actually edible in a dire emergency).
Given the flag filtering, I think using can_have_charges() would also work, but more by accident than by design, as neither power tools nor magazines would be flagged as detergent.
There was a problem hiding this comment.
I would suggest the test for "json_flag_DETERGENT" would weed out the undesirable charge things, though (ammo and non solid comestibles), which should work as long as soap isn't made comestible (I think soap made from natural ingredients is actually edible in a dire emergency).
liquid soap is comestible, but as I understand it using charges there is correct. the other two uncharged DETERGENT items are soap flakes and detergent, where we want to use amount.
There was a problem hiding this comment.
Yes, you're correct. I managed to screw up my logic (solid soap obviously fails the liquid test), and liquid soap matching two criteria isn't a problem. The comparison of the operations is still correct, though, but I'm not sure about wanting to access "stackable_" directly, as that seems to depend on whether liquid comestibles not subject to rot are "stackable_" or not (as rot is the reason you can't just stack liquids such as milk).
There was a problem hiding this comment.
I think I might be missing where stackable_ comes up. is it that has_charges is wrong for items that are only count_by_charges because they are stackable? none of the detergent items have "stackable": true, so none of them should be stackable, right?
There was a problem hiding this comment.
I did try to investigate this further, and it turns out that "stackable" isn't relevant here, as you point out. It's used for actual ammo (i.e. 7.62, etc.), ammo links, and a weird outlier in the form of a toxic slurry that's liquid, and probably should be shifted to the comestible category.
Apart from that, you get charges from items being "ammo" or from them being liquid comestibles (and I don't really understand why being liquid shouldn't be sufficient, unless it is to avoid weird results on phase changes, such as metal being molten into liquid metal and then being cast, which I don't think is in the game in an explicit form anyway).
Anything liquid in the base game (haven't looked at mods) is either ammo or comestible, except for the slurry.
Summary
Bugfixes "Fix use of detergent for washing"
Purpose of change
Fixes #87507. This was a regression introduced by #87543, which removed charges from the detergent item, but did not change the C++ code for washing to use detergent amounts instead of charges.
Describe the solution
Change all of the places where detergent is consumed/checked by charges in the code to instead consume/check it by amount.
Describe alternatives you've considered
Testing
Tested all of the allowed cases for washing actions:
Additional context
I grepped through the codebase for similar cases for the other items modified in #87543, but this appears to be the only one. However, the regex I used would not catch situations like the washing machine, which use a flag rather than the item id.
rg 'charges.*itype_(tin|sheet_metal_small|silver_small|magnesium|lead|zinc_metal|platinum_small|gold_small|bismuth|scrap|iodine_crystal|white_phosphorous|red_phosphorous_small|red_phosphorous|rosin|wooden_bead|corpse_ash|ash|nanomaterial|soap_flakes|detergent|chem_apex|chem_hmtd|chem_compositionb|chem_rdx|chem_anfo|chem_thermite|chem_ammonium_nitrate|chem_ammonium_nitrate_pellets|chem_saltpetre|chem_aluminium_sulphate|cathod_mix|chem_ferric_chloride|chem_antimony_trichloride|chem_potassium_chloride|chunk_feldspar|flux_ash|chunk_borax|chem_borax|chunk_caco|chunk_graphite|chunk_sulfur|chem_sulphur|chem_caco|chem_carbide|chem_chromium_oxide|chem_potassium_hydroxide|chem_manganese_dioxide|chem_zinc_oxide|chem_zinc_powder|chem_nickel_powder|chem_aluminium_powder|material_cement|material_limestone|chem_slaked_lime|shredded_rubber)'