Rewrote logic of ScreenManager to handle ScreenManager::quit() better…

… and not have dangling current_screen pointers around

src/scripting/functions.cpp

@@ -69,7 +69,7 @@ void wait_for_screenswitch(HSQUIRRELVM vm)
 
 void exit_screen()
 {
-  g_screen_manager->exit_screen();
+  g_screen_manager->pop_screen();
 }
 
 void fadeout_screen(float seconds)

src/supertux/game_session.cpp

@@ -123,7 +123,7 @@ GameSession::restart_level()
     }
   } catch(std::exception& e) {
     log_fatal << "Couldn't start level: " << e.what() << std::endl;
-    g_screen_manager->exit_screen();
+    g_screen_manager->pop_screen();
     return (-1);
   }
 
@@ -252,7 +252,7 @@ void
 GameSession::abort_level()
 {
   MenuManager::instance().clear_menu_stack();
-  g_screen_manager->exit_screen();
+  g_screen_manager->pop_screen();
   currentsector->player->set_bonus(bonus_at_start);
   PlayerStatus *currentStatus = get_player_status();
   currentStatus->coins = coins_at_start;
@@ -488,7 +488,7 @@ GameSession::finish(bool win)
       WorldMap::current()->finished_level(level.get());
   }
 
-  g_screen_manager->exit_screen();
+  g_screen_manager->pop_screen();
 }
 
 void

src/supertux/levelintro.cpp

@@ -60,7 +60,7 @@ LevelIntro::update(float elapsed_time)
      || controller->pressed(Controller::ACTION)
      || controller->pressed(Controller::MENU_SELECT)
      || controller->pressed(Controller::PAUSE_MENU)) {
-    g_screen_manager->exit_screen(std::unique_ptr<ScreenFade>(new FadeOut(0.1)));
+    g_screen_manager->pop_screen(std::unique_ptr<ScreenFade>(new FadeOut(0.1)));
   }
 
   player_sprite_py += player_sprite_vy * elapsed_time;

src/supertux/menu/worldmap_menu.cpp

@@ -44,7 +44,7 @@ WorldmapMenu::check_menu()
 
     case MNID_QUITWORLDMAP:
       MenuManager::instance().clear_menu_stack();
-      g_screen_manager->exit_screen();
+      g_screen_manager->pop_screen();
       break;
   }
 }

src/supertux/screen_manager.cpp

@@ -46,13 +46,10 @@ ScreenManager::ScreenManager() :
   m_waiting_threads(),
   m_menu_storage(new MenuStorage),
   m_menu_manager(new MenuManager),
-  m_running(),
   m_speed(1.0),
-  m_nextpop(false),
-  m_nextpush(false),
+  m_action(NO_ACTION),
   m_fps(0),
   m_next_screen(),
-  m_current_screen(),
   m_screen_fade(),
   m_screen_stack(),
   m_screenshot_requested(false)
@@ -71,21 +68,44 @@ ScreenManager::~ScreenManager()
 void
 ScreenManager::push_screen(std::unique_ptr<Screen> screen, std::unique_ptr<ScreenFade> screen_fade)
 {
-  assert(!m_next_screen);
+  log_debug << "ScreenManager::push_screen(): " << screen.get() << std::endl;
+  assert(screen);
+
+  if (m_action == PUSH_ACTION)
+  {
+    assert(m_next_screen);
+    // this happens for example when a Screen::setup() calls
+    // push_screen() itself, i.e. GameSessions/LevelIntro, in that
+    // case just commit the last action directly to the stack
+    m_screen_stack.push_back(std::move(m_next_screen));
+    m_action = NO_ACTION;
+  }
+
+  assert(m_action == NO_ACTION || m_action == POP_ACTION);
+
   m_next_screen = std::move(screen);
   m_screen_fade = std::move(screen_fade);
-  m_nextpush = !m_nextpop;
-  m_nextpop = false;
+
+  if (m_action == POP_ACTION)
+  {
+    m_action = REPLACE_ACTION;
+  }
+  else
+  {
+    m_action = PUSH_ACTION;
+  }
   m_speed = 1.0f;
 }
 
 void
-ScreenManager::exit_screen(std::unique_ptr<ScreenFade> screen_fade)
+ScreenManager::pop_screen(std::unique_ptr<ScreenFade> screen_fade)
 {
+  log_debug << "ScreenManager::pop_screen(): stack_size: " << m_screen_stack.size() << std::endl;
+  assert(m_action == NO_ACTION);
+
   m_next_screen.reset();
   m_screen_fade = std::move(screen_fade);
-  m_nextpop = true;
-  m_nextpush = false;
+  m_action = POP_ACTION;
 }
 
 void
@@ -97,8 +117,8 @@ ScreenManager::set_screen_fade(std::unique_ptr<ScreenFade> screen_fade)
 void
 ScreenManager::quit(std::unique_ptr<ScreenFade> screen_fade)
 {
-  m_screen_stack.clear();
-  exit_screen(std::move(screen_fade));
+  m_screen_fade = std::move(screen_fade);
+  m_action = QUIT_ACTION;
 }
 
 void
@@ -113,12 +133,6 @@ ScreenManager::get_speed() const
   return m_speed;
 }
 
-bool
-ScreenManager::has_no_pending_fadeout() const
-{
-  return !m_screen_fade || m_screen_fade->done();
-}
-
 void
 ScreenManager::draw_fps(DrawingContext& context, float fps_fps)
 {
@@ -134,15 +148,19 @@ ScreenManager::draw_fps(DrawingContext& context, float fps_fps)
 void
 ScreenManager::draw(DrawingContext& context)
 {
+  assert(!m_screen_stack.empty());
+
   static Uint32 fps_ticks = SDL_GetTicks();
   static int frame_count = 0;
 
-  m_current_screen->draw(context);
+  m_screen_stack.back()->draw(context);
   m_menu_manager->draw(context);
+
   if (m_screen_fade)
   {
     m_screen_fade->draw(context);
   }
+
   Console::instance->draw(context);
 
   if (g_config->show_fps)
@@ -177,12 +195,19 @@ ScreenManager::update_gamelogic(float elapsed_time)
 {
   scripting::update_debugger();
   scripting::TimeScheduler::instance->update(game_time);
-  m_current_screen->update(elapsed_time);
+
+  if (!m_screen_stack.empty())
+  {
+    m_screen_stack.back()->update(elapsed_time);
+  }
+
   m_menu_manager->process_input();
+
   if (m_screen_fade)
   {
     m_screen_fade->update(elapsed_time);
   }
+
   Console::instance->update(elapsed_time);
 }
 
@@ -242,36 +267,62 @@ ScreenManager::process_events()
   }
 }
 
+bool
+ScreenManager::has_pending_fadeout() const
+{
+  return m_screen_fade && !m_screen_fade->done();
+}
+
 void
 ScreenManager::handle_screen_switch()
 {
-  while ((m_next_screen || m_nextpop) &&
-        has_no_pending_fadeout())
+  if (m_action != NO_ACTION && !has_pending_fadeout())
   {
-    if (m_current_screen) {
-      m_current_screen->leave();
+    if (m_action == POP_ACTION)
+    {
+      assert(!m_screen_stack.empty());
+      m_action = NO_ACTION;
+
+      m_screen_stack.back()->leave();
+      m_screen_stack.pop_back();
+
+      if (!m_screen_stack.empty())
+      {
+        m_screen_stack.back()->setup();
+      }
     }
+    else if (m_action == PUSH_ACTION)
+    {
+      assert(m_next_screen);
+      m_action = NO_ACTION;
 
-    if (m_nextpop) {
-      if (m_screen_stack.empty()) {
-        m_running = false;
-        break;
+      if (!m_screen_stack.empty())
+      {
+        m_screen_stack.back()->leave();
       }
-      m_next_screen = std::move(m_screen_stack.back());
+
+      m_screen_stack.push_back(std::move(m_next_screen));
+      m_screen_stack.back()->setup();
+    }
+    else if (m_action == REPLACE_ACTION)
+    {
+      assert(!m_screen_stack.empty());
+      assert(!m_next_screen);
+      m_action = NO_ACTION;
+
+      m_screen_stack.back()->leave();
       m_screen_stack.pop_back();
+
+      m_screen_stack.push_back(std::move(m_next_screen));
+      m_screen_stack.back()->setup();
     }
-    if (m_nextpush && m_current_screen) {
-      m_screen_stack.push_back(std::move(m_current_screen));
+    else if (m_action == QUIT_ACTION)
+    {
+      m_screen_stack.clear();
     }
 
-    m_nextpush = false;
-    m_nextpop = false;
     m_speed = 1.0;
-    m_current_screen = std::move(m_next_screen);
-    if (m_current_screen)
-    {
-      m_current_screen->setup();
-    }
+
     m_screen_fade.reset();
 
     m_waiting_threads.wakeup();
@@ -284,15 +335,12 @@ ScreenManager::run(DrawingContext &context)
   Uint32 last_ticks = 0;
   Uint32 elapsed_ticks = 0;
 
-  m_running = true;
-  while (m_running)
-  {
-    handle_screen_switch();
-    if (!m_running || !m_current_screen)
-    {
-      break;
-    }
+  assert(m_action == PUSH_ACTION);
+  m_screen_stack.push_back(std::move(m_next_screen));
+  m_action = NO_ACTION;
 
+  while (!m_screen_stack.empty())
+  {
     Uint32 ticks = SDL_GetTicks();
     elapsed_ticks += ticks - last_ticks;
     last_ticks = ticks;
@@ -330,9 +378,14 @@ ScreenManager::run(DrawingContext &context)
       frames += 1;
     }
 
-    draw(context);
+    if (!m_screen_stack.empty())
+    {
+      draw(context);
+    }
 
     sound_manager->update();
+
+    handle_screen_switch();
   }
 }
 

src/supertux/screen_manager.hpp

@@ -39,11 +39,10 @@ class ScreenManager
   ~ScreenManager();
 
   void run(DrawingContext &context);
-  void exit_screen(std::unique_ptr<ScreenFade> fade = {});
   void quit(std::unique_ptr<ScreenFade> fade = {});
   void set_speed(float speed);
   float get_speed() const;
-  bool has_no_pending_fadeout() const;
+  bool has_pending_fadeout() const;
 
   /**
    * requests that a screenshot be taken after the next frame has been rendered
@@ -52,6 +51,7 @@ class ScreenManager
 
   // push new screen on screen_stack
   void push_screen(std::unique_ptr<Screen> screen, std::unique_ptr<ScreenFade> fade = {});
+  void pop_screen(std::unique_ptr<ScreenFade> fade = {});
   void set_screen_fade(std::unique_ptr<ScreenFade> fade);
 
   /// threads that wait for a screenswitch
@@ -67,14 +67,13 @@ class ScreenManager
 private:
   std::unique_ptr<MenuStorage> m_menu_storage;
   std::unique_ptr<MenuManager> m_menu_manager;
-  bool m_running;
+
   float m_speed;
-  bool m_nextpop;
-  bool m_nextpush;
+  enum Action { NO_ACTION, PUSH_ACTION, POP_ACTION, REPLACE_ACTION, QUIT_ACTION };
+  Action m_action;
   /// measured fps
   float m_fps;
   std::unique_ptr<Screen> m_next_screen;
-  std::unique_ptr<Screen> m_current_screen;
   std::unique_ptr<ScreenFade> m_screen_fade;
   std::vector<std::unique_ptr<Screen> > m_screen_stack;
   bool m_screenshot_requested; /**< true if a screenshot should be taken after the next frame has been rendered */

src/supertux/textscroller.cpp

@@ -107,7 +107,7 @@ TextScroller::update(float elapsed_time)
      )&& !(controller->pressed(Controller::UP))) // prevent skipping if jump with up is enabled
     scroll += SCROLL;
   if(controller->pressed(Controller::PAUSE_MENU)) {
-    g_screen_manager->exit_screen(std::unique_ptr<ScreenFade>(new FadeOut(0.5)));
+    g_screen_manager->pop_screen(std::unique_ptr<ScreenFade>(new FadeOut(0.5)));
   }
 
   scroll += speed * elapsed_time;
@@ -134,7 +134,7 @@ TextScroller::draw(DrawingContext& context)
 
   if(y < 0 && !fading ) {
     fading = true;
-    g_screen_manager->exit_screen(std::unique_ptr<ScreenFade>(new FadeOut(0.5)));
+    g_screen_manager->pop_screen(std::unique_ptr<ScreenFade>(new FadeOut(0.5)));
   }
 }
 

src/supertux/title_screen.cpp

@@ -141,7 +141,7 @@ TitleScreen::update(float elapsed_time)
 
   // reopen menu if user closed it (so that the app doesn't close when user
   // accidently hit ESC)
-  if(!MenuManager::instance().is_active() && g_screen_manager->has_no_pending_fadeout())
+  if(!MenuManager::instance().is_active() && !g_screen_manager->has_pending_fadeout())
   {
     MenuManager::instance().set_menu(MenuStorage::MAIN_MENU);
   }

src/worldmap/worldmap.cpp

@@ -238,7 +238,7 @@ WorldMap::move_to_spawnpoint(const std::string& spawnpoint, bool pan)
 void
 WorldMap::change(const std::string& filename, const std::string& force_spawnpoint)
 {
-  g_screen_manager->exit_screen();
+  g_screen_manager->pop_screen();
   g_screen_manager->push_screen(std::unique_ptr<Screen>(new WorldMap(filename, player_status, force_spawnpoint)));
 }