Cataclysm-dda: Loading a save file which has spawned an item set with relic data 'intermittent_activation' crashes the game

[ghost]

As of my recent tests with relic_data, I stumbled upon this error. Apparently, this breaks the save file and render it unplayable.

Steps To Reproduce

1. Spawn an item with intermittent_activation relic data.
2. Save the game.
3. Reload back.
4. This should appear.

DEBUG    : Bad save json
line 22:6302: expecting string but found '['

ers", "last_rot_check": 0, "last_temp_check": 0, "curammo": "battery", "relic_data": { "moves": 100, "charges_per_activation": 1, "passive_effects": [ { "has": "WORN", "condition": "ALWAYS", "intermittent_activation": { "duration": 600, [
                                                                                                                                                                                                                                                ^

                  { "id": "<spell_id>", "hit_self": false, "once_in": 1, "max_level": -1, "min_level": 0 }
                ] }, "values": [ { "value": "METABOLISM", "multiply": 0.050000 }, { "value": "REGEN_STAMINA


 FUNCTION : void game::unserialize(std::istream&)
 FILE     : src/savegame.cpp
 LINE     : 249

Expected behavior

The game should load the save without any error.

Versions and configuration

• OS: Windows 10
• Game Version: 0.E-1477-g8cea0fc
• Graphics version: Tiles
• Mods loaded: [ dda, secronom ]

Additional context

Secronom mod uses this relic data in some items.

[anothersimulacrum]

This is the problematic code, from enchantment::serialize.

    if( !intermittent_activation.empty() ) {
        jsout.member( "intermittent_activation" );
        jsout.start_object();
        for( const std::pair<time_duration, std::vector<fake_spell>> pair : intermittent_activation ) {
            jsout.member( "duration", pair.first );
            jsout.start_array( "effects" );
            for( const fake_spell &sp : pair.second ) {
                sp.serialize( jsout );
            }
            jsout.end_array();
        }
        jsout.end_object();
    }

There are two problems here - that jsout.start_array( "effect" ) is attempting to create an array paired with a key effects, but the deserialize function is expecting the key to be spell_effects.
But that's not the issue - the actual issue is that that is not the correct way to create an array paired with a key - that's instead calling JsonOut::start_array( bool wrap ) silently converting the string to a bool - there is no key paired with the array, making invalid JSON.
https://github.com/CleverRaven/Cataclysm-DDA/blob/master/src/json.h#L585

[anothersimulacrum]

It appears that this would be the correct way to do that:

diff --git a/src/magic_enchantment.cpp b/src/magic_enchantment.cpp
index d4252bf590..6233706361 100644
--- a/src/magic_enchantment.cpp
+++ b/src/magic_enchantment.cpp
@@ -293,7 +293,8 @@ void enchantment::serialize( JsonOut &jsout ) const
         jsout.start_object();
         for( const std::pair<time_duration, std::vector<fake_spell>> pair : intermittent_activation ) {
             jsout.member( "duration", pair.first );
-            jsout.start_array( "effects" );
+            jsout.member( "spell_effects" );
+            jsout.start_array();
             for( const fake_spell &sp : pair.second ) {
                 sp.serialize( jsout );
             }

[ghost]

Thanks :heart:

[ghost]

I just found an alternate way of using intermittent_activation. Instead of an item, I'll just apply this to a mutation. So far, my tests has proven that mutation does not throw an error. Sorry for confusion :sweat_smile:

[anothersimulacrum]

Okay, I'm going to reopen this, there's still a bug.

[jbytheway]

In most cases you shouldn't need to use these low-level JSON functions; you can just say jsout.member( "spell_effects", pair.second ) and it should figure out that you want to generate an array if pair.second is a container.