Fix notorious crash when switching worlds (#3336)

Previously, the WorldMap::change() method would create a unique pointer that replaces the which would clobber the Currenton state of the worldmap by pushing a new screen. However, this screws up the Currenton instance making it null at some point due to how it's freed.

This changes how the WorldMap initializes. WorldMap::change() instead simply changes it's state in-place.

May fix some other occasional issues that occured with the WorldMap.

Fixes #3280.
Fixes #3130.

Author: swagtoy

src/worldmap/worldmap.cpp

@@ -48,22 +48,41 @@ namespace worldmap {
 
 WorldMap::WorldMap(const std::string& filename, Savegame& savegame,
                    const std::string& force_sector, const std::string& force_spawnpoint) :
-  m_sector(),
+  m_sector(nullptr),
   m_sectors(),
-  m_force_spawnpoint(force_spawnpoint),
-  m_savegame(savegame),
-  m_tileset(),
+  m_force_spawnpoint(),
+  m_savegame(nullptr),
+  m_tileset(nullptr),
   m_name(),
-  m_map_filename(physfsutil::realpath(filename)),
-  m_levels_path(FileSystem::dirname(m_map_filename)),
-  m_next_worldmap(),
+  m_map_filename(),
+  m_levels_path(),
+  m_has_next_worldmap(false),
   m_passive_message(),
   m_passive_message_timer(),
   m_allow_item_pocket(true),
   m_enter_level(false),
   m_in_level(false),
-  m_in_world_select(false)
+  m_in_world_select(false),
+  m_next_filename(),
+  m_next_force_sector(),
+  m_next_force_spawnpoint()
 {
+  load(filename, savegame, force_sector, force_spawnpoint);
+}
+
+void
+WorldMap::load(const std::string& filename, Savegame& savegame,
+               const std::string& force_sector, const std::string& force_spawnpoint)
+{
+  m_map_filename = physfsutil::realpath(filename);
+  m_levels_path = FileSystem::dirname(m_map_filename);
+  /* We are reloading, so save now. */
+  if (m_has_next_worldmap)
+    m_savegame->get_player_status().last_worldmap = m_map_filename;
+  m_savegame = &savegame;
+  m_force_spawnpoint = force_spawnpoint;
+  m_sector = nullptr;
+  m_sectors.clear();
   SoundManager::current()->preload("sounds/warp.wav");
 
   /** Parse worldmap */
@@ -95,10 +114,13 @@ WorldMap::WorldMap(const std::string& filename, Savegame& savegame,
 
   /** Force the initial sector, if provided */
   if (!force_sector.empty())
+  {
     set_sector(force_sector, "", false);
+  }
+  if (m_has_next_worldmap)
+    setup();
 }
 
-
 void
 WorldMap::setup()
 {
@@ -142,11 +164,10 @@ WorldMap::update(float dt_sec, const Controller& controller)
   if (m_in_level) return;
   if (MenuManager::instance().is_active()) return;
 
-  if (m_next_worldmap) // A worldmap is scheduled to be changed to.
+  if (m_has_next_worldmap) // A worldmap is scheduled to be changed to.
   {
-    m_savegame.get_player_status().last_worldmap = m_next_worldmap->m_map_filename;
-    ScreenManager::current()->pop_screen();
-    ScreenManager::current()->push_screen(std::move(m_next_worldmap));
+    load(m_next_filename, *m_savegame, m_next_force_sector, m_next_force_spawnpoint);
+    m_has_next_worldmap = false;
     return;
   }
 
@@ -258,7 +279,10 @@ WorldMap::change(const std::string& filename, const std::string& force_sector,
                  const std::string& force_spawnpoint)
 {
   // Schedule worldmap to be changed to next frame.
-  m_next_worldmap = std::make_unique<WorldMap>(filename, m_savegame, force_sector, force_spawnpoint);
+  m_next_filename = filename;
+  m_next_force_sector = force_sector;
+  m_next_force_spawnpoint = force_spawnpoint;
+  m_has_next_worldmap = true;
 }
 
 

src/worldmap/worldmap.hpp

@@ -81,7 +81,7 @@ class WorldMap final : public Screen,
   inline void set_initial_spawnpoint(const std::string& spawnpoint) { m_force_spawnpoint = spawnpoint; }
 
   inline const std::string& get_title() const { return m_name; }
-  inline Savegame& get_savegame() const { return m_savegame; }
+  inline Savegame& get_savegame() const { return *m_savegame; }
   inline const std::string& get_levels_path() const { return m_levels_path; }
 
   WorldMapSector* get_sector(const std::string& name) const;
@@ -97,6 +97,9 @@ class WorldMap final : public Screen,
   bool is_item_pocket_allowed() const { return m_allow_item_pocket; }
 
 private:
+  void load(const std::string& filename, Savegame& savegame,
+            const std::string& force_sector = "", const std::string& force_spawnpoint = "");
+
   void process_input(const Controller& controller);
 
   void on_escape_press();
@@ -107,15 +110,15 @@ class WorldMap final : public Screen,
 
   std::string m_force_spawnpoint;
 
-  Savegame& m_savegame;
+  Savegame* m_savegame;
   TileSet* m_tileset;
 
   std::string m_name;
   std::string m_map_filename;
   std::string m_levels_path;
 
-  /* A worldmap, scheduled to change to next frame. */
-  std::unique_ptr<WorldMap> m_next_worldmap;
+  /* If true, the worldmap will reload on the next update */
+  bool m_has_next_worldmap;
 
   /** Passive map message variables */
   std::string m_passive_message;
@@ -125,6 +128,10 @@ class WorldMap final : public Screen,
   bool m_enter_level;
   bool m_in_level;
   bool m_in_world_select;
+  
+  std::string m_next_filename;
+  std::string m_next_force_sector;
+  std::string m_next_force_spawnpoint;
 
 private:
   WorldMap(const WorldMap&) = delete;

src/worldmap/worldmap_sector.cpp

@@ -355,7 +355,7 @@ WorldMapSector::update(float dt_sec)
                                     level_->get_pos().y +  8 - m_camera->get_offset().y);
           std::string levelfile = m_parent.m_levels_path + level_->get_level_filename();
 
-          auto game_session = std::make_unique<GameSession>(levelfile, m_parent.m_savegame, &level_->get_statistics());
+          auto game_session = std::make_unique<GameSession>(levelfile, m_parent.get_savegame(), &level_->get_statistics());
           game_session->restart_level();
 
           // update state and savegame

src/worldmap/worldmap_state.cpp

@@ -317,7 +317,7 @@ WorldMapState::save_state() const
     log_warning << "Failed to save worldmap state: " << err.what() << std::endl;
   }
 
-  m_worldmap.m_savegame.save();
+  m_worldmap.get_savegame().save();
 }
 
 } // namespace worldmap