Cataclysm-DDA
Cataclysm-DDA copied to clipboard
Vehicle water faucet fixes
Summary
None
Purpose of change
Fixes #45710
Describe the solution
Use the vpart and the liquid item instead of just item type when siphoning or drinking, this removes the need to do temperature overrides and makes UI correctly display (cold).
There are also various assumptions made about dealing with liquids using just the itype_ids that aren't correct; vehicle::fuel_specific_energy assumes temperatures of all liquids of same itype_id in all tanks can be averaged.
Describe alternatives you've considered
~~Tried using item_location + vehicle_cursor for consume_activity_actor, but seems it either deletes the item twice or has issues finding the item in the tank if it's the last charge drained and triggers a debugmsg.~~ apparently derped the item_location setup, testing solution from mqrause.
Testing
Before applying patch: Load attached save for convenience or repeat steps; spawn RV, install refrigerated tank, spawn 1 empty jerrycan, 10 bottles of clean water, fill tank with one bottle (refrigerated tank should have 20 water), turn on refrigerator, pass a few days until water becomes cold, siphon jerrycan, reload a bottle, use bottle to fill RV, you should have: 2 clean water (cold) in RV, 18 clean water (cold) in jerrycan.
- e the kitchen unit, "fill container", "pour into container", into jerrycan - see you have 40 clean water(cold) - extra 20 water spawned out of nowhere. Siphon doesn't limit itself as it was given "free" water item, rather than the water in the tank.
- e the kitchen unit, "drink water", v to see morale - see you got no bonus from drinking cold water. "drinking" from faucet was giving consume_activity_actor item generated from itype_id without temperature.
- fill the RV with water from jerrycan, start drinking until you can't drink anymore, continue drinking and cancelling - see water gets drained even if you cancel force drinking query.
Apply patch;
Repeat 1: jerrycan should only fill with water in the tank Repeat 2: should see +1 "enjoyed clean water" from cold bonus Repeat 3: once you're too full to drink the option to drink should be disabled
Additional context
Tried using item_location + vehicle_cursor for consume_activity_actor, but seems it either deletes the item twice or has issues finding the item in the tank if it's the last charge drained and triggers a debugmsg.
Kinda curious about this. Consume activity should handle reducing charges/removing items on it's own. So if you remove the last if
, I would think it should just work?
No, since I couldn't make item_location work and explicitly gave it the water item's copy and then manually subtracted the charges.
I tried using item_location, here's the patch with the test if you wanna try it out.
Load the attached save, it'll work right (as in, consumes water subtracting the charges) until you drink the last water charge, it then reaches the debugmsg here with this call stack:
> visitable::remove_item(item & it) Line 550 C++
item_location::impl::item_on_vehicle::remove_item() Line 537 C++
item_location::remove_item() Line 923 C++
Character::consume(item_location loc, bool force, bool refuel) Line 1875 C++
consume_activity_actor::finish(player_activity & act, Character & __formal) Line 2594 C++
player_activity::do_turn(Character & you) Line 377 C++
do_turn() Line 664 C++
WinMain(HINSTANCE__ * __formal, HINSTANCE__ * __formal, char * __formal, int __formal) Line 793 C++
[External Code]
So i guessed it either deletes the water item twice or fails to look it up (or i derped something when building the cursor/item_location)
So i guessed it either deletes the water item twice or fails to look it up (or i derped something when building the cursor/item_location)
Ah, yeah, the issue is that the water isn't in the vehicle part but in the base item of the vehicle part. So the item_location
would need to have the base item as the parent, which might be a bit sketchy. But at least I think something like this should work:
item_location base( vehicle_cursor( *this, vp_tank_idx ), &vp_tank.base ) );
item_location water( base, &vp_tank.base.only_item() );
No idea about potential issues arising from exposing the base item like this, though.
Ah so I did derp the setup, not sure about the exposing but I'll playtest it a bit and see
Actually thinking about it, I'm not sure the item_location base
is really correct like that. Vehicle cursor locations probably only really work for items in vehicle_part::items
. It should be good enough for removing the last charge at least, but it's definitely something to keep in mind for the future.
I added a comment near the code and added a test to check it in case item_location behavior changes