//main.cpp

int main (int argc, const char* argv[])
{
SDL_Window* window;
	SDL_Renderer* renderer;
	Controller controller = Controller();
	bool quit = false;

	if (SDL_Init(SDL_INIT_EVERYTHING) != 0)
		return 1;

	printf("SDL Initialized!\n");

	window = SDL_CreateWindow("Controller Test", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, 1280, 720, SDL_WINDOW_SHOWN | SDL_WINDOW_OPENGL);
	renderer = SDL_CreateRenderer(window, NULL, SDL_RENDERER_ACCELERATED);

	SDL_SetRenderDrawColor(renderer, 0x64, 0x95, 0xED, 0xFF);

	while (!quit)
	{
		SDL_RenderClear(renderer);	

		SDL_Event e;
		
		while (SDL_PollEvent(&e))
		{
			controller.ProcessEvents(e);
			switch (e.type)
			{
			case SDL_QUIT:
				quit = true;
				break;
			case SDL_KEYDOWN:
				switch (e.key.keysym.sym)
				{
				case SDLK_ESCAPE:
					quit = true;
					break;
				default:
					break;
				}
				break;
			default:
				break;
			}
		}

		for (int i = 0; i < SDL_NumJoysticks(); ++i)
		{
			if (controller.ButtonDown(i, SDL_CONTROLLER_BUTTON_BACK))
				quit = true;
			if (controller.ButtonDown(i, SDL_CONTROLLER_BUTTON_A))
			{
				printf("Controller %d Pressed!\n", i + 1);
			}
		}

		SDL_RenderPresent(renderer);
	}

	SDL_DestroyWindow(window);
	SDL_DestroyRenderer(renderer);
	SDL_Quit();

	return EXIT_SUCCESS;
}




//Controller.h

#ifndef _CONTROLLER_H_
#define _CONTROLLER_H_

#include <SDL.h>
#pragma comment (lib, "SDL2.lib")
#pragma comment (lib, "SDL2main.lib")
#undef main
#include <iostream>

enum PlayerIndex
{
	One = 0,
	Two = 1,
	Three = 2,
	Four = 3
};

class Controller
{
private:
	bool m_bConnected;
	SDL_GameController* m_Controller;
	SDL_Joystick* m_Joystick;
	SDL_JoystickID m_InstanceID;
	SDL_Haptic* m_Haptic;

	static Controller m_Controllers[4];
	static int GetControllerIndex(SDL_JoystickID instance);

	void Open(int device);
	void Close();

public:
	Controller() : m_bConnected(false), m_Controller(0), m_InstanceID(-1), m_Haptic(0) {}

	int ProcessEvents(const SDL_Event& e);

	bool ButtonDown(int Player, SDL_GameControllerButton Button)
	{
		return SDL_GameControllerGetButton(m_Controllers[Player].m_Controller, Button) == 1;
	}
};


#endif //_CONTROLLER_H_




//Controller.cpp

#include "Controller.h"

Controller Controller::m_Controllers[4];

void Controller::Open(int device)
{
	m_Controller = SDL_GameControllerOpen(device);
	m_Joystick = SDL_GameControllerGetJoystick(m_Controller);
	m_InstanceID = SDL_JoystickInstanceID(m_Joystick);
	m_bConnected = true;

	if (SDL_JoystickIsHaptic(m_Joystick))
	{
		m_Haptic = SDL_HapticOpenFromJoystick(m_Joystick);
		if (SDL_HapticRumbleSupported(m_Haptic))
		{
			if (SDL_HapticRumbleInit(m_Haptic) != 0)
			{
				printf("Haptic Rumble Init: %s\n", SDL_GetError());
				SDL_HapticClose(m_Haptic);
				m_Haptic = 0;
			}
		}
		else
		{
			SDL_HapticClose(m_Haptic);
			m_Haptic = 0;
		}
	}
}

void Controller::Close()
{
	if (m_bConnected)
	{
		m_bConnected = false;
		if (m_Haptic)
		{
			SDL_HapticClose(m_Haptic);
			m_Haptic = 0;
		}
		SDL_GameControllerClose(m_Controller);
		m_Controller = 0;
	}
}

int Controller::GetControllerIndex(SDL_JoystickID instance)
{
	for (int i = 0; i < 4; ++i)
	{
		if (m_Controllers[i].m_bConnected && m_Controllers[i].m_InstanceID == instance)
		{
			return i;
		}
	}
	return -1;
}

int Controller::ProcessEvents(const SDL_Event& e)
{
	switch (e.type)
	{
	case SDL_CONTROLLERBUTTONDOWN:
	case SDL_CONTROLLERBUTTONUP:
		break;
	case SDL_CONTROLLERDEVICEADDED:
		if (e.cdevice.which < 4)
		{
			Controller& c = m_Controllers[e.cdevice.which];
			c.Open(e.cdevice.which);
			printf("Controller %d Added\n", e.cdevice.which + 1);
		}
		break;
	case SDL_CONTROLLERDEVICEREMOVED:
		int cIndex = GetControllerIndex(e.cdevice.which);
		if (cIndex < 0) return 0;
		Controller& c = m_Controllers[cIndex];
		c.Close();
		printf("Controller %d Removed\n", cIndex + 1);
		break;
	}
	return 0;
}






My inputs when I debug

Controller 1 Added
Controller 2 Added
//I then turn on a controller
Controller 2 Added
//The new Controller 2 overwriting the old Controller 2 Rendering it unusable
//Plug in another controller
Controller 3 Added
//I remove the old Controller 2 or Controller 4 according to the physical Controller itself
//no input
//I plug the controller back in
Controller 4 Added

This sounds a lot like the issue I had, with a fix attached to this bug: https://bugzilla.libsdl.org/show_bug.cgi?id=3299
Does that patch solve your issue?

I ran into something like this separately when testing something a few months ago. Can you check the latest snapshot and see if this is fixed for you?
http://www.libsdl.org/tmp/SDL-2.0.zip

Created attachment 2841 [details]
Screenshot 1 of 3

Created attachment 2842 [details]
Screenshot 2 of 3

Created attachment 2843 [details]
Screenshot 3 of 3

The three screenshots I provided above are my inputs when testing the patch you provided.  The tests I performed previously were a tad bit different than last time, but it derives from the same bug in my opinion.  I am still encountering the bug where the controller is overwritten in memory.  I have both Xbox 360 controllers physically marked 1 and 3 connected to the computer before starting the program.  In screenshot 1 you can see the inputs when the program initializes.  Screenshot 2 is when I turn on and connect a new Xbox 360 controller that overwrites the controller physically marked as 3 in memory in which the new controller is physically marked as 2.  Screenshot 3 is when I add another new Xbox 360 controller which then shows up as controller 4 (which looks correct to me).

Note I am using the same exact code listed in the comment above with the SDL-2.0.5-11255 patch provided.

Inputs via console:

SDL Initialized!
Controller 1 Added
Controller 2 Added
Controller 2 Added
Controller 4 Added

Yeah, this sounds exactly like the issue I fixed in https://bugzilla.libsdl.org/show_bug.cgi?id=3299 - when you start up, controllers 1 and 3 get "GUIDs" 1 and 2, turn on controller 2, it's now given GUID 2, and 3 got remapped to GUID 3, and now SDL has two different "controllers" which have the same low-level device open.  Using device paths instead of "GUID"s should fix it.  Since my patch was never merged, I guess I'll try and update it this week to match the latest SDL codebase...

I added Jimb's patch, can you see if this fixes this bug as well?
http://www.libsdl.org/tmp/SDL-2.0.zip

Thanks!

I'm getting the same results as before with the 11281 build with the same setup as above.  So the bug still exists on my end with the updated code.

Thanks!

Ah, there seems to be some confusion (perhaps from my patch, though I'm not quite sure) over device_index and instance_id - in my app, I'm using instance_id as my primary identifier everywhere and have no problems, but your sample isn't quite right.

In your sample, you're mixing the two - you have an array of Controllers indexed by device_index (which gets reused).  Changing that code to handle a new device by instance_id like this (obviously inefficient) seems to mostly fix it:
	SDL_GameController *gc = SDL_GameControllerOpen(e.cdevice.which);
	int instance_id = SDL_JoystickInstanceID(SDL_GameControllerGetJoystick(gc));
	SDL_GameControllerClose(gc);
	Controller& c = m_Controllers[instance_id];
	c.Open(e.cdevice.which);
	printf("Controller %d Added\n", instance_id + 1);

However, even after that, something is still going wrong when removing devices (stop getting updates to device #2 after unplugging device #1) in your sample, not sure why.

API-wise, it seems really confusing to have both device indices and instance IDs exposed to the user (although I do see in the headers that it notes they're different), seems like it could almost all be replaced (without changing the public API) with instance IDs everywhere, although it may make SDL_GetNumJoysticks return an inflated number after some additions and removals.

Oh, problem with removing a controller is the sample code is doing "for (int i = 0; i < SDL_NumJoysticks(); ++i)"; however, when the second device is removed, your active joysticks are at index 0 and 2, but, of course SDL_NumJoysticks() returns only 2.  Changing that to always poll all 4 of the "controllers" in the sample code fixes it.

This bug also wouldn't happen if all gamecontroller/joystick APIs took instance_ids and SDL_NumJoysticks() just returned the largest instance id, of course then we'd need some *other* API for "is joystick connected" when the instance_id -> joystick mapping is sparse due to removed devices (and maybe need virtual instance ids for joysticks which are not yet opened, depending on when instance_ids currently get assigned).  That is how I wrapped everything in my engine's API, as using a device ID that might change leads to crazy complicated application code.

Also, to be clear, his problem probably has absolutely nothing with my patch (I was just over-excited to get my patch applied earlier and thought it might ;), as his sample code is using XInput by default, not DirectInput - however, his code has basically exactly the same "bug" that the the SDL DirectInput code had (in DirectInput, the "GUID" is acting exactly like SDL's "device_index" - it changes which device it is actually references at runtime, and the "HID Path" is just like SDL's instance_id - it's guaranteed unique and mapped to once device).

Yup, that makes sense.

To be clear, device_index is simply an index into the current list of connected controllers and has nothing to do with whether the controller is open. You can't use it for anything except querying information about the controller and opening it. Once a controller is open, the pointer and it's instance ID are the ways of referencing it.

I'm totally open to ways of making that clearer so other people don't run into the same issue.