Improve generic factory pair readers (#82964)

* Revert "Generic reader for vector of pairs"

This reverts commit 991994f.

* Improve generic factory pair readers

Document what JSON formats the readers accept, and modify pair_reader so
that it can accept arbitrary types.

Add a new pair type { "a": b } that this pair_reader supports, and
expand key_from_json_string to enable testing if the type of a can
support this.

Author: ehughsbaird

src/generic_factory.h

@@ -32,6 +32,7 @@
 #include "flexbuffer_json.h"
 #include "init.h"
 #include "int_id.h"
+#include "json.h"
 #include "mod_tracker.h"
 #include "string_formatter.h"
 #include "string_id.h"
@@ -1908,13 +1909,19 @@ class string_id_reader : public generic_typed_reader<string_id_reader<T>>
 
 /**
  * Loads std::pair of [K = string_id, V = int/float] values from JSON -- usually into an std::map
+ * It can load either an array, an object, or a single value
  * Accepted formats for elements in an array:
  * 1. A named key/value pair object: "addiction_type": [ { "addiction": "caffeine", "potential": 3 } ]
  * 2. A key/value pair array: "addiction_type": [ [ "caffeine", 3 ] ]
  * 3. A single value: "addiction_type": [ "caffeine" ]
- * A single value can also be provided outside of an array, e.g. "addiction_type": "caffeine"
+ * For an object, only the following format is supported
+ * 4. "addiction_type": { "caffeine": 3, "alcohol": 2 }
+ * A single value can be provided as follows:
+ * 5. "addiction_type": "caffeine"
  * For single values, weights are assigned default_weight
  */
+// TODO: format 1 is quite gross, and just accepts arbitrary keys. The only way it works is by limiting
+// the types of V, which is unfortunate
 template<typename K, typename V>
 class weighted_string_id_reader : public generic_typed_reader<weighted_string_id_reader<K, V>>
 {
@@ -1966,87 +1973,86 @@ class weighted_string_id_reader : public generic_typed_reader<weighted_string_id
     }
 };
 
+// For loading pairs from a JSON object. { "a1": b1, "a2": b2 }, etc
+// Only supports JSON objects! Json objects are not ordered, so if you're using something where
+// the order matters, use pair_reader.
 template<typename K, typename V>
 class generic_map_reader : public generic_typed_reader<generic_map_reader<K, V>>
 {
 public:
     static constexpr bool read_objects = true;
 
+    static_assert( key_from_json_string<K>::valid, "Type K must have a known conversion from string" );
+
     std::pair<K, V> get_next( const JsonValue &jv ) const {
         const JsonMember *jm = dynamic_cast<const JsonMember *>( &jv );
         if( jm == nullptr ) {
             jv.throw_error( "not part of a JsonObject" );
         }
-        K key( jm->name() );
+        K key = key_from_json_string<K>()( jm->name() );
         V value;
         jv.read( value, true );
         return std::pair<K, V>( key, value );
     }
 };
 
-// Support shorthand for a single value.
-template<typename T>
-class pair_reader : public generic_typed_reader<pair_reader<T>>
-{
-public:
-    std::pair<T, T> get_next( const JsonValue &jv ) const {
-        if( jv.test_float() ) {
-            T val;
-            jv.read( val, true );
-            return std::make_pair( val, val );
-        }
-        if( !jv.test_array() ) {
-            jv.throw_error( "bad pair" );
-        }
-        JsonArray ja = jv.get_array();
-        if( ja.size() != 2 ) {
-            ja.throw_error( "Must have 2 elements" );
-        }
-        T l;
-        T h;
-        ja[0].read( l, true );
-        ja[1].read( h, true );
-        return std::make_pair( l, h );
-    }
-};
-
-// Reads into a vector of pairs in the format (example)
-// "growth_stages": [ { "GROWTH_SEED": "23 days" }, { "GROWTH_SEEDLING": "23 days" }, { "GROWTH_MATURE": "23 days" }, { "GROWTH_HARVEST": "22 days" } ]
-// OR
-// "growth_stages": [ [ "GROWTH_SEED", "23 days" ], [ "GROWTH_SEEDLING", "23 days" ], [ "GROWTH_MATURE", "23 days" ], [ "GROWTH_HARVEST", "22 days" ] ]
-// The key(K) of the pair object must be string constructable.
-template<typename K, typename V>
-class vector_pair_reader : public generic_typed_reader<vector_pair_reader<K, V>>
+// Supports three formats:
+// 1. [ a, b ]
+// 2. { "a": b }, if there exists a key_from_json_string specialization for a
+// 3. a, if T = U and a is not represented in JSON as an array or object (iff the object format is valid)
+// This would load [ "a", [ "b", "c" ], { "d": "e" } ] into three pairs:
+// ("a", "a"), ("b", "c"), ("d", "e")
+//
+// If you only need format 1, you can also use json_read_reader<std::pair<T, U>>
+//
+// TODO: constructor argument to enable/disable the shorthand format?
+template<typename T, typename U = T>
+class pair_reader : public generic_typed_reader<pair_reader<T, U>>
 {
 public:
-    std::pair<K, V> get_next( const JsonValue &jv ) const {
-        if( jv.test_object() ) {
-            JsonObject jo = jv.get_object();
-            if( jo.size() != 1 ) {
-                jv.throw_error( string_format( "Expected size of 1 for JsonObject, found size %d", jo.size() ) );
+    std::pair<T, U> get_next( const JsonValue &jv ) const {
+        // elements of the pair to return
+        T a;
+        U b;
+        // [ a, b ]
+        if( jv.test_array() ) {
+            JsonArray ja = jv.get_array();
+            if( ja.size() != 2 ) {
+                ja.throw_error( "Must have 2 elements" );
             }
-            for( JsonMember jm : jo ) {
-                K key( jm.name() );
-                V ret;
-                jm.read( ret );
-                return std::make_pair( key, ret );
+            ja[0].read( a, true );
+            ja[1].read( b, true );
+            return std::make_pair( a, b );
+        }
+        // { "a": b }
+        if constexpr( key_from_json_string<T>::valid ) {
+            if( jv.test_object() ) {
+                JsonObject jo = jv.get_object();
+                if( jo.size() != 1 ) {
+                    jo.throw_error( "bad object pair, has too many elements" );
+                }
+                // gross - we need to get the name of the member, this just unwraps it
+                for( JsonMember jm : jo ) {
+                    a = key_from_json_string<T>()( jm.name() );
+                    jm.read( b, true );
+                    return std::make_pair( a, b );
+                }
             }
         }
-        if( !jv.test_array() ) {
-            jv.throw_error( "bad pair" );
-        }
-        JsonArray ja = jv.get_array();
-        if( ja.size() != 2 ) {
-            ja.throw_error( "Must have 2 elements" );
+        // shorthand for a = b
+        if constexpr( std::is_same_v<T, U> ) {
+            jv.read( a, true );
+            b = a;
+            return std::make_pair( a, b );
         }
-        K l;
-        V h;
-        ja[0].read( l, true );
-        ja[1].read( h, true );
-        return std::make_pair( l, h );
+        jv.throw_error( "bad pair" );
+        return std::make_pair( a, b );
     }
 };
 
+// Loads pairs as objects { key1: t1, key2: t2 }, or just t1, which uses the default for the value of t2
+// e.g. named_pair_reader<string, int>( "a", "b", 3 ) loads
+// [ "1", { "a": "3", "b": 4 } ] -> ("1", 3), ("3", 4)
 template<typename T1, typename T2>
 class named_pair_reader : public generic_typed_reader<named_pair_reader<T1, T2>>
 {
@@ -2070,7 +2076,7 @@ class named_pair_reader : public generic_typed_reader<named_pair_reader<T1, T2>>
             jo.read( key2, ret.second, true );
             return ret;
         }
-        // TODO: support pair format?
+        // TODO: support pair format? Forward to pair_reader<T1, T2>?
         if( jv.test_array() ) {
             jv.throw_error( "invalid format" );
         }

src/item_factory.cpp

@@ -3439,7 +3439,7 @@ void islot_seed::deserialize( const JsonObject &jo )
     mandatory( jo, was_loaded, "plant_name", plant_name );
     mandatory( jo, was_loaded, "fruit", fruit_id );
     mandatory( jo, was_loaded, "growth_stages", growth_stages,
-               vector_pair_reader<flag_id, time_duration> {} );
+               pair_reader<flag_id, time_duration> {} );
     optional( jo, was_loaded, "seeds", spawn_seeds, true );
     optional( jo, was_loaded, "byproducts", byproducts );
     optional( jo, was_loaded, "required_terrain_flag", required_terrain_flag,

src/json.h

@@ -51,38 +51,45 @@ struct is_string_like<T, std::void_t<decltype( std::declval<T &>().substr() )>>
 std::true_type { };
 
 template<typename T, typename Enable = void>
-struct key_from_json_string;
+struct key_from_json_string {
+    static constexpr bool valid = false;
+};
 
 template<>
 struct key_from_json_string<std::string, void> {
+    static constexpr bool valid = true;
     std::string operator()( const std::string &s ) {
         return s;
     }
 };
 
 template<typename T>
 struct key_from_json_string<string_id<T>, void> {
+    static constexpr bool valid = true;
     string_id<T> operator()( const std::string &s ) {
         return string_id<T>( s );
     }
 };
 
 template<typename T>
 struct key_from_json_string<int_id<T>, void> {
+    static constexpr bool valid = true;
     int_id<T> operator()( const std::string &s ) {
         return int_id<T>( s );
     }
 };
 
 template<typename Enum>
 struct key_from_json_string<Enum, std::enable_if_t<std::is_enum_v<Enum>>> {
+    static constexpr bool valid = true;
     Enum operator()( const std::string &s ) {
         return io::string_to_enum<Enum>( s );
     }
 };
 
 template<typename T>
 struct key_from_json_string<T, std::void_t<decltype( std::declval<T &>().to_string_writable() )>> {
+    static constexpr bool valid = true;
     T operator()( const std::string &s ) {
         return T::from_string( s );
     }