RetroPie / EmulationStation

A Fork of Emulation Station for RetroPie. Emulation Station is a flexible emulator front-end supporting keyboardless navigation and custom system themes.
Other
863 stars 344 forks source link

EmulationStation crash when switching themes (after directlaunch kodi is installed) #37

Closed HerbFargus closed 8 years ago

HerbFargus commented 8 years ago

This is the error given:

emulationstation: /home/pi/RetroPie-Setup/tmp/build/emulationstation/es-core/src/components/IList.h:124: const UserData& IList<EntryData, UserData>::getSelected() const [with EntryData = TextListData; UserData = FileData*]: Assertion `size() > 0' failed.

Theme changes work fine on a fresh 3.6 image until kodi is installed with the new directlaunch function and then it crashes anytime a theme is changed.

I can manually make changes in es_settings.cfg and they'll stick on reboot but something within the emulationstation menu is being janky

default settings.cfg

http://pastebin.com/nf0rukKz

joolswills commented 8 years ago

@taalas

PR introducing the problem - https://github.com/RetroPie/EmulationStation/pull/1

joolswills commented 8 years ago

@taalas - unfortunately if I don't find time to look at this myself, your additional functionality will be removed - so please take a look if you get a chance. Thanks.

joolswills commented 8 years ago

Crashes in ViewController::reloadAll due to cursorMap[it->first] = it->second->getCursor(); - getCursor calls getSelected which fails when there is no list (due to the assert)

Not sure the current implementation of directLaunch is too safe really as there may be other places where it causes trouble too.

joolswills commented 8 years ago

I have a possible / mostly untested hack / fix for this that changes reloadAll, but I'm not keen on it and I'm not sure there wont be other problems with the directLaunch implementation (eg with the gamelist only parameter - or if there is just a single directlaunch entry in es_systems I think it will have trouble too)

diff --git a/es-app/src/views/ViewController.cpp b/es-app/src/views/ViewController.cpp
index 9716cfd..59d014d 100644
--- a/es-app/src/views/ViewController.cpp
+++ b/es-app/src/views/ViewController.cpp
@@ -370,14 +370,18 @@ void ViewController::reloadAll()
        std::map<SystemData*, FileData*> cursorMap;
        for(auto it = mGameListViews.begin(); it != mGameListViews.end(); it++)
        {
-               cursorMap[it->first] = it->second->getCursor();
+               if (it->first->getDirectLaunch())
+                       cursorMap[it->first] = nullptr;
+               else
+                       cursorMap[it->first] = it->second->getCursor();
        }
        mGameListViews.clear();

        for(auto it = cursorMap.begin(); it != cursorMap.end(); it++)
        {
                it->first->loadTheme();
-               getGameListView(it->first)->setCursor(it->second);
+               if (it->second)
+                       getGameListView(it->first)->setCursor(it->second);
        }

        mSystemListView.reset();
joolswills commented 8 years ago

I'm going to remove directLaunch - it can be re-added if implemented in a nicer way (that is fully tested)

taalas commented 8 years ago

Hi @joolswills @HerbFargus I was on holiday so couldn't check myself. I agree though that it might be best to remove the functionality until it is more stable. Tried to test the implementation as much as possible, seem to have missed it though. I will try for a more stable approach once I find the time...

HerbFargus commented 8 years ago

I can confirm your above patch doesnt crash while switching theme anymore but still causes an error when scraping.

joolswills commented 8 years ago

Must be another case of getSelected being called

joolswills commented 8 years ago

reverted