From 0a370713fcdadb8299f81196b3bd0e64c8c5ff46 Mon Sep 17 00:00:00 2001
From: anothersimulacrum <42699974+anothersimulacrum@users.noreply.github.com>
Date: Sat, 11 Jul 2020 13:57:11 -0700
Subject: [PATCH] Don't silently fail when attempting to modify vits

For some reason, attempting to modify a vitamin would silently fail when
the character's `vitamin_levels` map did not contain that vitamin.
However, this map never actually had vitamins added to it... except when
the game was loaded, in deserializing the character's vitamin levels.
This doesn't appear to be very purposeful, just a side effect of using
the ids of the vitamins as keys for the amount of that vitamin, and not
bailing out if the key was not found.

This meant on the first time a new character was loaded, before you
saved and reloaded, any vitamins would not work, particularly the blood
vitamins.

Stop silently failing when attempting to modify a vitamin that is not
already contained, and instead add it to the `vitamin_levels` map, and
change the character loading function to avoid the behavior that hid
this problem in the past - filling out the vitamin map completely even
if there was nothing to load for it.
---
 src/consumption.cpp   | 10 +++++++---
 src/savegame_json.cpp |  7 ++++---
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/src/consumption.cpp b/src/consumption.cpp
index 7b87a3f40a15..c7374728fe3a 100644
--- a/src/consumption.cpp
+++ b/src/consumption.cpp
@@ -542,11 +542,15 @@ time_duration Character::vitamin_rate( const vitamin_id &vit ) const
 
 int Character::vitamin_mod( const vitamin_id &vit, int qty, bool capped )
 {
-    auto it = vitamin_levels.find( vit );
-    if( it == vitamin_levels.end() ) {
+    if( !vit.is_valid() ) {
+        debugmsg( "Vitamin with id %s does not exist, and cannot be modified", vit.str() );
         return 0;
     }
-    const auto &v = it->first.obj();
+    // What's going on here? Emplace returns either an iterator to the inserted
+    // item or, if it already exists, an iterator to the (unchanged) extant item
+    // (Okay, technically it returns a pair<iterator, bool>, the iterator is what we want)
+    auto it = vitamin_levels.emplace( vit, 0 ).first;
+    const vitamin &v = *it->first;
 
     if( qty > 0 ) {
         // Accumulations can never occur from food sources
diff --git a/src/savegame_json.cpp b/src/savegame_json.cpp
index b2753b9a091a..d52b560a5ea9 100644
--- a/src/savegame_json.cpp
+++ b/src/savegame_json.cpp
@@ -434,9 +434,10 @@ void Character::load( const JsonObject &data )
     JsonObject vits = data.get_object( "vitamin_levels" );
     vits.allow_omitted_members();
     for( const std::pair<const vitamin_id, vitamin> &v : vitamin::all() ) {
-        int lvl = vits.get_int( v.first.str(), 0 );
-        lvl = std::max( std::min( lvl, v.first.obj().max() ), v.first.obj().min() );
-        vitamin_levels[v.first] = lvl;
+        if( vits.has_member( v.first.str() ) ) {
+            int lvl = vits.get_int( v.first.str() );
+            vitamin_levels[v.first] = clamp( lvl, v.first->min(), v.first->max() );
+        }
     }
     data.read( "consumption_history", consumption_history );
     data.read( "activity", activity );