Dynamic joystick add/remove/mapping is now working again. Or at least

it is on all my test systems with 4 different controllers.  We still
need bugtesters for this ...


git-svn-id: svn://svn.code.sf.net/p/stella/code/trunk@3112 8b62c5a3-ac7e-4cc8-8f21-d9a121418aba

Changes.txt

@@ -19,6 +19,9 @@
     Clang 3.3, gcc 4.9, Xcode 6, etc).  Eventually, this will bring more
     bug-free and (hopefully) faster code.
 
+  * Fixed major bug with joysticks, where mapping was being lost on reset,
+    the app would crash when plugging/unplugging certain sticks, etc.
+
   * The minimum supported version for the OSX port is now OSX 10.7.
     Because of this, the 32-bit version is also discontinued, as 10.7
     is 64-bit Intel only.

src/common/EventHandlerSDL2.cxx

@@ -207,7 +207,16 @@ EventHandlerSDL2::JoystickSDL2::JoystickSDL2(int idx)
   myStick = SDL_JoystickOpen(idx);
   if(myStick)
   {
-    initialize(idx, SDL_JoystickName(myStick),
+    // There still seems to be some issue with certain controllers not being
+    // recognized.  In this case, SDL names the controller as "XInput Controller".
+    // This would be fine except it also appends " #x", where x seems to vary.
+    // Obviously this wreaks havoc with the idea that a joystick will always
+    // have the same name.  So we truncate the number.
+    const char* sdlname = SDL_JoystickName(myStick);
+    const string& desc = BSPF_startsWithIgnoreCase(sdlname, "XInput Controller")
+                         ? "XInput Controller" : sdlname;
+
+    initialize(SDL_JoystickInstanceID(myStick), desc,
         SDL_JoystickNumAxes(myStick), SDL_JoystickNumButtons(myStick),
         SDL_JoystickNumHats(myStick), SDL_JoystickNumBalls(myStick));
   }
@@ -218,5 +227,4 @@ EventHandlerSDL2::JoystickSDL2::~JoystickSDL2()
 {
   if(myStick)
     SDL_JoystickClose(myStick);
-  myStick = nullptr;
 }

src/emucore/EventHandler.cxx

@@ -1073,9 +1073,10 @@ void EventHandler::setActionMappings(EventMode mode)
     }
 
 #ifdef JOYSTICK_SUPPORT
-    for(uInt32 stick = 0; stick < myJoyHandler->numSticks(); ++stick)
+    for(const auto& i: myJoyHandler->sticks())
     {
-      const StellaJoystick* joy = myJoyHandler->joy(stick);
+      uInt32 stick = i.first;
+      const StellaJoystick* joy = i.second;
       if(!joy)  continue;
 
       // Joystick button mapping/labeling

src/emucore/EventHandler.hxx

@@ -424,21 +424,23 @@ class EventHandler
 
     class JoystickHandler
     {
+      using StickList = map<int, StellaJoystick*>;
       public:
         JoystickHandler(OSystem& system);
         ~JoystickHandler();
 
         int add(StellaJoystick* stick);
         int remove(int id);
-        uInt32 numSticks() const { return (uInt32)mySticks.size(); }
         void mapStelladaptors(const string& saport);
         void setDefaultMapping(Event::Type type, EventMode mode);
         void eraseMapping(Event::Type event, EventMode mode);
         void saveMapping();
 
         const StellaJoystick* joy(int id) const {
-          return id < (int)mySticks.size() ? mySticks[id] : nullptr;
+          const auto& i = mySticks.find(id);
+          return i != mySticks.cend() ? i->second : nullptr;
         }
+        const StickList& sticks() const { return mySticks; }
 
       private:
         OSystem& myOSystem;
@@ -460,7 +462,7 @@ class EventHandler
         map<string,StickInfo> myDatabase;
 
         // Contains only joysticks that are currently available, indexed by id
-        vector<StellaJoystick*> mySticks;
+        StickList mySticks;
 
         void setStickDefaultMapping(int stick, Event::Type type, EventMode mode);
         void printDatabase() const;

src/emucore/EventJoyHandler.cxx

@@ -273,11 +273,11 @@ void EventHandler::JoystickHandler::printDatabase() const
   for(const auto& i: myDatabase)
     cerr << i.first << endl << i.second << endl << endl;
 
-  cerr << "---------------------------------------------------------" << endl
+  cerr << "---------------------" << endl
        << "joy active:"  << endl;
   for(const auto& i: mySticks)
-    cerr << i << endl;
+    cerr << i.first << ": " << *i.second << endl;
-  cerr << endl;
+  cerr << "---------------------------------------------------------" << endl << endl << endl;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -326,7 +326,8 @@ int EventHandler::JoystickHandler::add(StellaJoystick* stick)
     }
     stick->type = StellaJoystick::JT_REGULAR;
   }
-  Vec::insertAt(mySticks, stick->ID, stick);
+  // The stick *must* be inserted here, since it may be used below
+  mySticks[stick->ID] = stick;
 
   // Map the stelladaptors we've found according to the specified ports
   if(specialAdaptor)
@@ -358,9 +359,9 @@ int EventHandler::JoystickHandler::remove(int index)
 
   // Sticks that are removed must have initially been added
   // So we use the 'active' joystick list to access them
-  if(index >= 0 && index < (int)mySticks.size() && mySticks[index] != nullptr)
+  try
   {
-    StellaJoystick* stick = mySticks[index];
+    StellaJoystick* stick = mySticks.at(index);
 
     auto it = myDatabase.find(stick->name);
     if(it != myDatabase.end() && it->second.joy == stick)
@@ -373,11 +374,15 @@ int EventHandler::JoystickHandler::remove(int index)
       // Remove joystick, but remember mapping
       it->second.mapping = stick->getMap();
       delete it->second.joy;  it->second.joy = nullptr;
-      mySticks[index] = nullptr;
+      mySticks.erase(index);
 
       return index;
     }
   }
+  catch(std::out_of_range)
+  {
+  }
+
   return -1;
 }
 
@@ -396,33 +401,33 @@ void EventHandler::JoystickHandler::mapStelladaptors(const string& saport)
     saOrder[0] = 2; saOrder[1] = 1;
   }
 
-  for(auto stick: mySticks)
+  for(auto& stick: mySticks)
   {
-    if(BSPF_startsWithIgnoreCase(stick->name, "Stelladaptor"))
+    if(BSPF_startsWithIgnoreCase(stick.second->name, "Stelladaptor"))
     {
       if(saOrder[saCount] == 1)
       {
-        stick->name += " (emulates left joystick port)";
+        stick.second->name += " (emulates left joystick port)";
-        stick->type = StellaJoystick::JT_STELLADAPTOR_LEFT;
+        stick.second->type = StellaJoystick::JT_STELLADAPTOR_LEFT;
       }
       else if(saOrder[saCount] == 2)
       {
-        stick->name += " (emulates right joystick port)";
+        stick.second->name += " (emulates right joystick port)";
-        stick->type = StellaJoystick::JT_STELLADAPTOR_RIGHT;
+        stick.second->type = StellaJoystick::JT_STELLADAPTOR_RIGHT;
       }
       saCount++;
     }
-    else if(BSPF_startsWithIgnoreCase(stick->name, "2600-daptor"))
+    else if(BSPF_startsWithIgnoreCase(stick.second->name, "2600-daptor"))
     {
       if(saOrder[saCount] == 1)
       {
-        stick->name += " (emulates left joystick port)";
+        stick.second->name += " (emulates left joystick port)";
-        stick->type = StellaJoystick::JT_2600DAPTOR_LEFT;
+        stick.second->type = StellaJoystick::JT_2600DAPTOR_LEFT;
       }
       else if(saOrder[saCount] == 2)
       {
-        stick->name += " (emulates right joystick port)";
+        stick.second->name += " (emulates right joystick port)";
-        stick->type = StellaJoystick::JT_2600DAPTOR_RIGHT;
+        stick.second->type = StellaJoystick::JT_2600DAPTOR_RIGHT;
       }
       saCount++;
     }
@@ -434,8 +439,8 @@ void EventHandler::JoystickHandler::mapStelladaptors(const string& saport)
 void EventHandler::JoystickHandler::setDefaultMapping(Event::Type event, EventMode mode)
 {
   eraseMapping(event, mode);
-  setStickDefaultMapping(0, event, mode);
+  for(auto& i: mySticks)
-  setStickDefaultMapping(1, event, mode);
+    setStickDefaultMapping(i.first, event, mode);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -510,8 +515,6 @@ void EventHandler::JoystickHandler::setStickDefaultMapping(int stick,
 
         // Left joystick (assume joystick zero, button zero)
         setDefaultBtn( 0, 0, Event::UISelect );
-        // Right joystick (assume joystick one, button zero)
-        setDefaultBtn( 1, 0, Event::UISelect );
 
         setDefaultHat( 0, 0, EVENT_HATLEFT,  Event::UILeft  );
         setDefaultHat( 0, 0, EVENT_HATRIGHT, Event::UIRight );
@@ -533,13 +536,13 @@ void EventHandler::JoystickHandler::eraseMapping(Event::Type event, EventMode mo
   // Otherwise, only reset the given event
   if(event == Event::NoType)
   {
-    for(auto stick: mySticks)
+    for(auto& stick: mySticks)
-      stick->eraseMap(mode);          // erase all events
+      stick.second->eraseMap(mode);          // erase all events
   }
   else
   {
-    for(auto stick: mySticks)
+    for(auto& stick: mySticks)
-      stick->eraseEvent(event, mode); // only reset the specific event
+      stick.second->eraseEvent(event, mode); // only reset the specific event
   }
 }