refactor: Clean up world saving/loading codepath (Phase 1 of v2 save …

…format) (#5774) * fix: Add 'cataclysm-bn-tiles' and 'json_formatter' to .gitignore All the other binaries are listed except for the ones that cmake actually generates for tiles builds. Signed-off-by: David Li <jiawei.davidli@gmail.com> * refactor: Begin consolidating world loading Clean up and refactor worldfactory and world. * The old 'WORLD' struct is now renamed `WORLDINFO` to better reflect its actual purpose. * Get rid of the WORLDPTR = WORLD* convention. This isn't the Windows API, and nothing else does this. * Move file saving logic into a new class called `world`, meant to encapsulate the idea of a loaded world. I will begin consolidating more world loading logic into this class. Signed-off-by: David Li <jiawei.davidli@gmail.com> * refactor: Simplify file I/O utility methods Replace the 2 write and 6 read methods with 1 write and 2 read. This simplification is needed because future save loaders will need to provide their own implementations of these methods. * The `_optional` variants can be replaced with default arguments. * The `JsonDeserializer` variants are never used. * The `_json` variants do save some boilerplate (`path`) so I'll keep that in for now. Signed-off-by: David Li <jiawei.davidli@gmail.com> * refactor: Consolidate more world loading methods into `world` Player file IO is still janky, particularly `mm` files - WIP. However, the game does work and the on-disk format should be unchanged. Signed-off-by: David Li <jiawei.davidli@gmail.com> * refactor: Consolidate player-specific save IO Clean up how the game determines where to place player- specific files, in preparation for the V2 save format. NOTE: This removes backwards compatibility for the pre-2020 map memory ('mm') format. This logic only triggered when loading a map that has not seen a save event since 2020 - I think it's safe to remove this now. Signed-off-by: David Li <jiawei.davidli@gmail.com> * style(autofix.ci): automated formatting * fix: Forgot to migrate some code in non-tiles build Signed-off-by: David Li <jiawei.davidli@gmail.com> * fix: Bug in initializing world dir structure Signed-off-by: David Li <jiawei.davidli@gmail.com> * fix: Subtle change in write_to_file error handling The consolidation of write_to_file methods introduced a subtle API change where passing nullptr to the third argument no longer caused write_to_file to eat exceptions. This largely only affected tests and a small number of edge cases. Alter the API so that the right way to eat exceptions is via passing in the empty string as the message. Signed-off-by: David Li <jiawei.davidli@gmail.com> * refactor: Remember save format in the WORLDINFO struct Plus misc bug fixes Signed-off-by: David Li <jiawei.davidli@gmail.com> * refactor: Address code style and const in save refactor Signed-off-by: David Li <jiawei.davidli@gmail.com> --------- Signed-off-by: David Li <jiawei.davidli@gmail.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
cataclysmbnteam · Dec 7, 2024 · 374bd34 · 374bd34
1 parent 7ccdf4b
commit 374bd34
Show file tree

Hide file tree

Showing 44 changed files with 921 additions and 682 deletions.
diff --git a/.gitignore b/.gitignore
@@ -42,6 +42,8 @@ cata_test
 cata_test-tiles
 cataclysm
 cataclysm-tiles
+cataclysm-bn-tiles
+json_formatter
 *.exe.manifest
 cataclysm-vcpkg
 cataclysmdda-*

diff --git a/src/artifact.cpp b/src/artifact.cpp
@@ -27,6 +27,7 @@
 #include "type_id.h"
 #include "units.h"
 #include "value_ptr.h"
+#include "world.h"
 
 template<typename V, typename B>
 inline units::quantity<V, B> rng( const units::quantity<V, B> &min,
@@ -1086,9 +1087,9 @@ std::string artifact_name( const std::string &type )
 
 /* Json Loading and saving */
 
-void load_artifacts( const std::string &path )
+void load_artifacts( const world *world, const std::string &path )
 {
-    read_from_file_optional_json( path, []( JsonIn & artifact_json ) {
+    world->read_from_file_json( path, []( JsonIn & artifact_json ) {
         artifact_json.start_array();
         while( !artifact_json.end_array() ) {
             JsonObject jo = artifact_json.get_object();
@@ -1101,7 +1102,7 @@ void load_artifacts( const std::string &path )
                 jo.throw_error( "unrecognized artifact type.", "type" );
             }
         }
-    } );
+    }, true );
 }
 
 void it_artifact_tool::deserialize( const JsonObject &jo )
@@ -1266,9 +1267,9 @@ void it_artifact_armor::deserialize( const JsonObject &jo )
     }
 }
 
-bool save_artifacts( const std::string &path )
+bool save_artifacts( const world *world, const std::string &path )
 {
-    return write_to_file( path, [&]( std::ostream & fout ) {
+    return world->write_to_file( path, [&]( std::ostream & fout ) {
         JsonOut json( fout, true );
         json.start_array();
         // We only want runtime types, otherwise static artifacts are loaded twice (on init and then on game load)

diff --git a/src/artifact.h b/src/artifact.h
@@ -7,6 +7,7 @@
 #include "enums.h"
 #include "itype.h"
 #include "relic.h"
+#include "world.h"
 
 class JsonObject;
 class JsonOut;
@@ -126,9 +127,9 @@ itype_id new_artifact();
 itype_id new_natural_artifact( artifact_natural_property prop );
 
 // note: needs to be called by main() before MAPBUFFER.load
-void load_artifacts( const std::string &path );
+void load_artifacts( const world *world, const std::string &path );
 // save artifact definitions to json, path must be the same as for loading.
-bool save_artifacts( const std::string &path );
+bool save_artifacts( const world *world, const std::string &path );
 
 bool check_art_charge_req( item &it );
 

diff --git a/src/auto_note.cpp b/src/auto_note.cpp
@@ -18,26 +18,23 @@
 #include "point.h"
 #include "translations.h"
 #include "ui_manager.h"
+#include "world.h"
 
 namespace auto_notes
 {
-std::string auto_note_settings::build_save_path() const
-{
-    return g->get_player_base_save_path() + ".ano.json";
-}
-
 void auto_note_settings::clear()
 {
     autoNoteEnabled.clear();
 }
 
 bool auto_note_settings::save()
 {
-    if( !file_exist( g->get_player_base_save_path() + ".sav" ) ) {
+    world *world = g->get_active_world();
+    if( !world->player_file_exist( ".sav" ) ) {
         return true;
     }
 
-    return write_to_file( build_save_path(), [&]( std::ostream & fstr ) {
+    return world->write_to_player_file( ".ano.json", [&]( std::ostream & fstr ) {
         JsonOut jout{ fstr, true };
 
         jout.start_object();
@@ -92,7 +89,8 @@ void auto_note_settings::load()
         }
     };
 
-    if( !read_from_file_optional_json( build_save_path(), parseJson ) ) {
+    if( !g->get_active_world()->read_from_player_file_json( ".ano.json", parseJson,
+            true ) ) {
         default_initialize();
         save();
     }

diff --git a/src/auto_note.h b/src/auto_note.h
@@ -72,10 +72,6 @@ class auto_note_settings
         /// registered map extras in order to determine their enable status.
         void default_initialize();
 
-    private:
-        /// Build string containing path to the auto notes save file for the active player.
-        std::string build_save_path() const;
-
     private:
         /// This set contains the ID strings of all map extras that have auto note enabled.
         std::unordered_set<string_id<map_extra>> autoNoteEnabled;

diff --git a/src/auto_pickup.cpp b/src/auto_pickup.cpp
@@ -29,6 +29,7 @@
 #include "translations.h"
 #include "type_id.h"
 #include "ui_manager.h"
+#include "world.h"
 
 using namespace auto_pickup;
 
@@ -714,22 +715,22 @@ bool player_settings::save_global()
 
 bool player_settings::save( const bool bCharacter )
 {
-    auto savefile = PATH_INFO::autopickup();
-
     if( bCharacter ) {
-        savefile = g->get_player_base_save_path() + ".apu.json";
-
-        const std::string player_save = g->get_player_base_save_path() + ".sav";
         //Character not saved yet.
-        if( !file_exist( player_save ) ) {
+        if( !g->get_active_world()->player_file_exist( ".sav" ) ) {
             return true;
         }
-    }
 
-    return write_to_file( savefile, [&]( std::ostream & fout ) {
-        JsonOut jout( fout, true );
-        ( bCharacter ? character_rules : global_rules ).serialize( jout );
-    }, _( "autopickup configuration" ) );
+        return g->get_active_world()->write_to_player_file( ".apu.json", [&]( std::ostream & fout ) {
+            JsonOut jout( fout, true );
+            ( bCharacter ? character_rules : global_rules ).serialize( jout );
+        }, _( "autopickup configuration" ) );
+    } else {
+        return write_to_file( PATH_INFO::autopickup(), [&]( std::ostream & fout ) {
+            JsonOut jout( fout, true );
+            ( bCharacter ? character_rules : global_rules ).serialize( jout );
+        }, _( "autopickup configuration" ) );
+    }
 }
 
 void player_settings::load_character()
@@ -744,15 +745,15 @@ void player_settings::load_global()
 
 void player_settings::load( const bool bCharacter )
 {
-    std::string sFile = PATH_INFO::autopickup();
     if( bCharacter ) {
-        sFile = g->get_player_base_save_path() + ".apu.json";
+        g->get_active_world()->read_from_player_file_json( ".apu.json", [&]( JsonIn & jsin ) {
+            ( bCharacter ? character_rules : global_rules ).deserialize( jsin );
+        }, true );
+    } else {
+        read_from_file_json( PATH_INFO::autopickup(), [&]( JsonIn & jsin ) {
+            ( bCharacter ? character_rules : global_rules ).deserialize( jsin );
+        }, true );
     }
-
-    read_from_file_optional_json( sFile, [&]( JsonIn & jsin ) {
-        ( bCharacter ? character_rules : global_rules ).deserialize( jsin );
-    } );
-
     invalidate();
 }
 

diff --git a/src/cata_utility.cpp b/src/cata_utility.cpp
@@ -457,24 +457,31 @@ std::istream *cata_ifstream::operator->()
     return &*_stream;
 }
 
-void write_to_file( const std::string &path, const std::function<void( std::ostream & )> &writer )
-{
-    // Any of the below may throw. ofstream_wrapper will clean up the temporary path on its own.
-    ofstream_wrapper fout( path, cata_ios_mode::binary );
-    writer( fout.stream() );
-    fout.close();
-}
-
-bool write_to_file( const std::string &path, const std::function<void( std::ostream & )> &writer,
-                    const char *const fail_message )
+/**
+ * If fail_message is provided, this method will eat any exceptions and display a popup with the
+ * exception detail and the message. If fail_message is not provided, the exception will be
+ * propagated.
+ *
+ * To eat any exceptions and not display a popup, pass the empty string as fail_message.
+ *
+ * @param path The path to write to.
+ * @param writer The function that writes to the file.
+ * @param fail_message The message to display if the write fails.
+ * @return True if the write was successful, false otherwise.
+ */
+bool write_to_file( const std::string &path, file_write_fn &writer, const char *const fail_message )
 {
     try {
-        write_to_file( path, writer );
+        // Any of the below may throw. ofstream_wrapper will clean up the temporary path on its own.
+        ofstream_wrapper fout( path, cata_ios_mode::binary );
+        writer( fout.stream() );
+        fout.close();
         return true;
-
     } catch( const std::exception &err ) {
-        if( fail_message ) {
+        if( fail_message && fail_message[0] != '\0' ) {
             popup( _( "Failed to write %1$s to \"%2$s\": %3$s" ), fail_message, path.c_str(), err.what() );
+        } else if( fail_message == nullptr ) {
+            std::throw_with_nested( std::runtime_error( "file write failed: " + path ) );
         }
         return false;
     }
@@ -523,8 +530,12 @@ std::istream &safe_getline( std::istream &ins, std::string &str )
     }
 }
 
-bool read_from_file( const std::string &path, const std::function<void( std::istream & )> &reader )
+bool read_from_file( const std::string &path, file_read_fn reader, bool optional )
 {
+    if( optional && !file_exist( path ) ) {
+        return false;
+    }
+
     try {
         cata_ifstream fin = std::move( cata_ifstream().mode( cata_ios_mode::binary ).open( path ) );
         if( !fin.is_open() ) {
@@ -542,44 +553,12 @@ bool read_from_file( const std::string &path, const std::function<void( std::ist
     }
 }
 
-bool read_from_file_json( const std::string &path, const std::function<void( JsonIn & )> &reader )
+bool read_from_file_json( const std::string &path, file_read_json_fn reader, bool optional )
 {
     return read_from_file( path, [&]( std::istream & fin ) {
         JsonIn jsin( fin, path );
         reader( jsin );
-    } );
-}
-
-bool read_from_file( const std::string &path, JsonDeserializer &reader )
-{
-    return read_from_file_json( path, [&reader]( JsonIn & jsin ) {
-        reader.deserialize( jsin );
-    } );
-}
-
-bool read_from_file_optional( const std::string &path,
-                              const std::function<void( std::istream & )> &reader )
-{
-    // Note: slight race condition here, but we'll ignore it. Worst case: the file
-    // exists and got removed before reading it -> reading fails with a message
-    // Or file does not exists, than everything works fine because it's optional anyway.
-    return file_exist( path ) && read_from_file( path, reader );
-}
-
-bool read_from_file_optional_json( const std::string &path,
-                                   const std::function<void( JsonIn & )> &reader )
-{
-    return read_from_file_optional( path, [&]( std::istream & fin ) {
-        JsonIn jsin( fin, path );
-        reader( jsin );
-    } );
-}
-
-bool read_from_file_optional( const std::string &path, JsonDeserializer &reader )
-{
-    return read_from_file_optional_json( path, [&reader]( JsonIn & jsin ) {
-        reader.deserialize( jsin );
-    } );
+    }, optional );
 }
 
 void ofstream_wrapper::open( cata_ios_mode mode )

diff --git a/src/catalua.cpp b/src/catalua.cpp
@@ -61,12 +61,12 @@ void debug_write_lua_backtrace( std::ostream &/*out*/ )
     // Nothing to do here
 }
 
-bool save_world_lua_state( const std::string & )
+bool save_world_lua_state( const world *world, const std::string & )
 {
     return true;
 }
 
-bool load_world_lua_state( const std::string & )
+bool load_world_lua_state( const world *world, const std::string & )
 {
     return true;
 }
@@ -166,7 +166,7 @@ void show_lua_console()
 void reload_lua_code()
 {
     cata::lua_state &state = *DynamicDataLoader::get_instance().lua;
-    const auto &packs = world_generator->active_world->active_mod_order;
+    const auto &packs = world_generator->active_world->info->active_mod_order;
     try {
         init::load_main_lua_scripts( state, packs );
     } catch( std::runtime_error &e ) {
@@ -194,14 +194,14 @@ static sol::table get_mod_storage_table( lua_state &state )
     return state.lua.globals()["game"]["cata_internal"]["mod_storage"];
 }
 
-bool save_world_lua_state( const std::string &path )
+bool save_world_lua_state( const world *world, const std::string &path )
 {
     lua_state &state = *DynamicDataLoader::get_instance().lua;
 
-    const mod_management::t_mod_list &mods = world_generator->active_world->active_mod_order;
+    const mod_management::t_mod_list &mods = world_generator->active_world->info->active_mod_order;
     sol::table t = get_mod_storage_table( state );
     run_on_game_save_hooks( state );
-    bool ret = write_to_file( path, [&]( std::ostream & stream ) {
+    bool ret = world->write_to_file( path, [&]( std::ostream & stream ) {
         JsonOut jsout( stream );
         jsout.start_object();
         for( const mod_id &mod : mods ) {
@@ -217,13 +217,13 @@ bool save_world_lua_state( const std::string &path )
     return ret;
 }
 
-bool load_world_lua_state( const std::string &path )
+bool load_world_lua_state( const world *world, const std::string &path )
 {
     lua_state &state = *DynamicDataLoader::get_instance().lua;
-    const mod_management::t_mod_list &mods = world_generator->active_world->active_mod_order;
+    const mod_management::t_mod_list &mods = world_generator->active_world->info->active_mod_order;
     sol::table t = get_mod_storage_table( state );
 
-    bool ret = read_from_file_optional( path, [&]( std::istream & stream ) {
+    bool ret = world->read_from_file( path, [&]( std::istream & stream ) {
         JsonIn jsin( stream );
         JsonObject jsobj = jsin.get_object();
 
@@ -239,7 +239,7 @@ bool load_world_lua_state( const std::string &path )
             JsonObject mod_obj = jsobj.get_object( mod.str() );
             deserialize_lua_table( t[mod.str()], mod_obj );
         }
-    } );
+    }, true );
 
     run_on_game_load_hooks( state );
     return ret;

diff --git a/src/catalua.h b/src/catalua.h
@@ -10,6 +10,7 @@ class Item_factory;
 class map;
 class time_point;
 struct tripoint;
+class world;
 
 namespace cata
 {
@@ -27,8 +28,8 @@ void show_lua_console();
 void reload_lua_code();
 void debug_write_lua_backtrace( std::ostream &out );
 
-bool save_world_lua_state( const std::string &path );
-bool load_world_lua_state( const std::string &path );
+bool save_world_lua_state( const world *world, const std::string &path );
+bool load_world_lua_state( const world *world, const std::string &path );
 
 std::unique_ptr<lua_state, lua_state_deleter> make_wrapped_state();