million/suyu
1
0
Fork 0
forked from suyu/suyu
Code Pull requests Activity

sdl_impl: Fix controller reconnection issues

Browse source 
It turns out that after a controller is disconnected, there is a chance that events from the previous controller are sent/processed after it has been disconnected.
This causes the previously disconnected controller to reappear as connected due to GetSDLJoystickBySDLID() emplacing this controller back to the map.

Fix this by only returning an SDLJoystick if and only if it exists in the map.
This commit is contained in:
Morph 2020-10-21 08:42:11 -04:00
parent 1fc61d09d3
commit 2f852f182a
1 changed files with 84 additions and 85 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

127
src/input_common/sdl/sdl_impl.cpp
View file

@ -155,15 +155,15 @@ public:
return sdl_joystick.get(); return sdl_joystick.get();
} }
void SetSDLJoystick(SDL_Joystick* joystick, SDL_GameController* controller) {
sdl_controller.reset(controller);
sdl_joystick.reset(joystick);
}
SDL_GameController* GetSDLGameController() const { SDL_GameController* GetSDLGameController() const {
return sdl_controller.get(); return sdl_controller.get();
} }
void SetSDLJoystick(SDL_Joystick* joystick, SDL_GameController* controller) {
sdl_joystick.reset(joystick);
sdl_controller.reset(controller);
}
private: private:
struct State { struct State {
std::unordered_map<int, bool> buttons; std::unordered_map<int, bool> buttons;
@ -186,69 +186,58 @@ private:
std::shared_ptr<SDLJoystick> SDLState::GetSDLJoystickByGUID(const std::string& guid, int port) { std::shared_ptr<SDLJoystick> SDLState::GetSDLJoystickByGUID(const std::string& guid, int port) {
std::lock_guard lock{joystick_map_mutex}; std::lock_guard lock{joystick_map_mutex};
const auto it = joystick_map.find(guid); const auto it = joystick_map.find(guid);
if (it != joystick_map.end()) { if (it != joystick_map.end()) {
while (it->second.size() <= static_cast<std::size_t>(port)) { while (it->second.size() <= static_cast<std::size_t>(port)) {
auto joystick = std::make_shared<SDLJoystick>(guid, static_cast<int>(it->second.size()), auto joystick = std::make_shared<SDLJoystick>(guid, static_cast<int>(it->second.size()),
nullptr, nullptr); nullptr, nullptr);
it->second.emplace_back(std::move(joystick)); it->second.emplace_back(std::move(joystick));
} }
return it->second[static_cast<std::size_t>(port)]; return it->second[static_cast<std::size_t>(port)];
} }
auto joystick = std::make_shared<SDLJoystick>(guid, 0, nullptr, nullptr); auto joystick = std::make_shared<SDLJoystick>(guid, 0, nullptr, nullptr);
return joystick_map[guid].emplace_back(std::move(joystick)); return joystick_map[guid].emplace_back(std::move(joystick));
} }
std::shared_ptr<SDLJoystick> SDLState::GetSDLJoystickBySDLID(SDL_JoystickID sdl_id) { std::shared_ptr<SDLJoystick> SDLState::GetSDLJoystickBySDLID(SDL_JoystickID sdl_id) {
auto sdl_joystick = SDL_JoystickFromInstanceID(sdl_id); auto sdl_joystick = SDL_JoystickFromInstanceID(sdl_id);
auto sdl_controller = SDL_GameControllerFromInstanceID(sdl_id);
const std::string guid = GetGUID(sdl_joystick); const std::string guid = GetGUID(sdl_joystick);
std::lock_guard lock{joystick_map_mutex}; std::lock_guard lock{joystick_map_mutex};
const auto map_it = joystick_map.find(guid); const auto map_it = joystick_map.find(guid);
if (map_it != joystick_map.end()) {
const auto vec_it = if (map_it == joystick_map.end()) {
std::find_if(map_it->second.begin(), map_it->second.end(), return nullptr;
[&sdl_joystick](const std::shared_ptr<SDLJoystick>& joystick) { }
return sdl_joystick == joystick->GetSDLJoystick();
const auto vec_it = std::find_if(map_it->second.begin(), map_it->second.end(),
[&sdl_joystick](const auto& joystick) {
return joystick->GetSDLJoystick() == sdl_joystick;
}); });
if (vec_it != map_it->second.end()) {
// This is the common case: There is already an existing SDL_Joystick mapped to a if (vec_it == map_it->second.end()) {
// SDLJoystick. return the SDLJoystick return nullptr;
}
return *vec_it; return *vec_it;
} }
// Search for a SDLJoystick without a mapped SDL_Joystick...
const auto nullptr_it = std::find_if(map_it->second.begin(), map_it->second.end(),
[](const std::shared_ptr<SDLJoystick>& joystick) {
return joystick->GetSDLJoystick() == nullptr;
});
if (nullptr_it != map_it->second.end()) {
// ... and map it
(*nullptr_it)->SetSDLJoystick(sdl_joystick, sdl_controller);
return *nullptr_it;
}
// There is no SDLJoystick without a mapped SDL_Joystick
// Create a new SDLJoystick
const int port = static_cast<int>(map_it->second.size());
auto joystick = std::make_shared<SDLJoystick>(guid, port, sdl_joystick, sdl_controller);
return map_it->second.emplace_back(std::move(joystick));
}
auto joystick = std::make_shared<SDLJoystick>(guid, 0, sdl_joystick, sdl_controller);
return joystick_map[guid].emplace_back(std::move(joystick));
}
void SDLState::InitJoystick(int joystick_index) { void SDLState::InitJoystick(int joystick_index) {
SDL_Joystick* sdl_joystick = SDL_JoystickOpen(joystick_index); SDL_Joystick* sdl_joystick = SDL_JoystickOpen(joystick_index);
SDL_GameController* sdl_gamecontroller = nullptr; SDL_GameController* sdl_gamecontroller = nullptr;
if (SDL_IsGameController(joystick_index)) { if (SDL_IsGameController(joystick_index)) {
sdl_gamecontroller = SDL_GameControllerOpen(joystick_index); sdl_gamecontroller = SDL_GameControllerOpen(joystick_index);
} }
if (!sdl_joystick) { if (!sdl_joystick) {
LOG_ERROR(Input, "Failed to open joystick {}", joystick_index); LOG_ERROR(Input, "Failed to open joystick {}", joystick_index);
return; return;
} }
const std::string guid = GetGUID(sdl_joystick); const std::string guid = GetGUID(sdl_joystick);
std::lock_guard lock{joystick_map_mutex}; std::lock_guard lock{joystick_map_mutex};
@ -257,14 +246,17 @@ void SDLState::InitJoystick(int joystick_index) {
joystick_map[guid].emplace_back(std::move(joystick)); joystick_map[guid].emplace_back(std::move(joystick));
return; return;
} }
auto& joystick_guid_list = joystick_map[guid]; auto& joystick_guid_list = joystick_map[guid];
const auto it = std::find_if( const auto joystick_it =
joystick_guid_list.begin(), joystick_guid_list.end(), std::find_if(joystick_guid_list.begin(), joystick_guid_list.end(),
[](const std::shared_ptr<SDLJoystick>& joystick) { return !joystick->GetSDLJoystick(); }); [](const auto& joystick) { return !joystick->GetSDLJoystick(); });
if (it != joystick_guid_list.end()) {
(*it)->SetSDLJoystick(sdl_joystick, sdl_gamecontroller); if (joystick_it != joystick_guid_list.end()) {
(*joystick_it)->SetSDLJoystick(sdl_joystick, sdl_gamecontroller);
return; return;
} }
const int port = static_cast<int>(joystick_guid_list.size()); const int port = static_cast<int>(joystick_guid_list.size());
auto joystick = std::make_shared<SDLJoystick>(guid, port, sdl_joystick, sdl_gamecontroller); auto joystick = std::make_shared<SDLJoystick>(guid, port, sdl_joystick, sdl_gamecontroller);
joystick_guid_list.emplace_back(std::move(joystick)); joystick_guid_list.emplace_back(std::move(joystick));
@ -274,18 +266,14 @@ void SDLState::CloseJoystick(SDL_Joystick* sdl_joystick) {
const std::string guid = GetGUID(sdl_joystick); const std::string guid = GetGUID(sdl_joystick);
std::lock_guard lock{joystick_map_mutex}; std::lock_guard lock{joystick_map_mutex};
auto& joystick_guid_list = joystick_map[guid]; // This call to guid is safe since the joystick is guaranteed to be in the map
auto joystick_it = std::find_if( const auto& joystick_guid_list = joystick_map[guid];
joystick_guid_list.begin(), joystick_guid_list.end(), const auto joystick_it = std::find_if(joystick_guid_list.begin(), joystick_guid_list.end(),
[&sdl_joystick](auto& joystick) { return joystick->GetSDLJoystick() == sdl_joystick; }); [&sdl_joystick](const auto& joystick) {
return joystick->GetSDLJoystick() == sdl_joystick;
});
if (joystick_it != joystick_guid_list.end()) {
(*joystick_it)->SetSDLJoystick(nullptr, nullptr); (*joystick_it)->SetSDLJoystick(nullptr, nullptr);
joystick_guid_list.erase(joystick_it);
if (joystick_guid_list.empty()) {
joystick_map.erase(guid);
}
}
} }
void SDLState::HandleGameControllerEvent(const SDL_Event& event) { void SDLState::HandleGameControllerEvent(const SDL_Event& event) {
@ -720,8 +708,7 @@ std::vector<Common::ParamPackage> SDLState::GetInputDevices() {
std::vector<Common::ParamPackage> devices; std::vector<Common::ParamPackage> devices;
for (const auto& [key, value] : joystick_map) { for (const auto& [key, value] : joystick_map) {
for (const auto& joystick : value) { for (const auto& joystick : value) {
auto* joy = joystick->GetSDLJoystick(); if (auto* const controller = joystick->GetSDLGameController()) {
if (auto* controller = joystick->GetSDLGameController()) {
std::string name = std::string name =
fmt::format("{} {}", SDL_GameControllerName(controller), joystick->GetPort()); fmt::format("{} {}", SDL_GameControllerName(controller), joystick->GetPort());
devices.emplace_back(Common::ParamPackage{ devices.emplace_back(Common::ParamPackage{
@ -730,7 +717,7 @@ std::vector<Common::ParamPackage> SDLState::GetInputDevices() {
{"guid", joystick->GetGUID()}, {"guid", joystick->GetGUID()},
{"port", std::to_string(joystick->GetPort())}, {"port", std::to_string(joystick->GetPort())},
}); });
} else if (joy) { } else if (auto* const joy = joystick->GetSDLJoystick()) {
std::string name = fmt::format("{} {}", SDL_JoystickName(joy), joystick->GetPort()); std::string name = fmt::format("{} {}", SDL_JoystickName(joy), joystick->GetPort());
devices.emplace_back(Common::ParamPackage{ devices.emplace_back(Common::ParamPackage{
{"class", "sdl"}, {"class", "sdl"},
@ -797,22 +784,28 @@ Common::ParamPackage BuildHatParamPackageForButton(int port, std::string guid, s
Common::ParamPackage SDLEventToButtonParamPackage(SDLState& state, const SDL_Event& event) { Common::ParamPackage SDLEventToButtonParamPackage(SDLState& state, const SDL_Event& event) {
switch (event.type) { switch (event.type) {
case SDL_JOYAXISMOTION: { case SDL_JOYAXISMOTION: {
const auto joystick = state.GetSDLJoystickBySDLID(event.jaxis.which); if (const auto joystick = state.GetSDLJoystickBySDLID(event.jaxis.which)) {
return BuildAnalogParamPackageForButton(joystick->GetPort(), joystick->GetGUID(), return BuildAnalogParamPackageForButton(joystick->GetPort(), joystick->GetGUID(),
static_cast<s32>(event.jaxis.axis), static_cast<s32>(event.jaxis.axis),
event.jaxis.value); event.jaxis.value);
} }
break;
}
case SDL_JOYBUTTONUP: { case SDL_JOYBUTTONUP: {
const auto joystick = state.GetSDLJoystickBySDLID(event.jbutton.which); if (const auto joystick = state.GetSDLJoystickBySDLID(event.jbutton.which)) {
return BuildButtonParamPackageForButton(joystick->GetPort(), joystick->GetGUID(), return BuildButtonParamPackageForButton(joystick->GetPort(), joystick->GetGUID(),
static_cast<s32>(event.jbutton.button)); static_cast<s32>(event.jbutton.button));
} }
break;
}
case SDL_JOYHATMOTION: { case SDL_JOYHATMOTION: {
const auto joystick = state.GetSDLJoystickBySDLID(event.jhat.which); if (const auto joystick = state.GetSDLJoystickBySDLID(event.jhat.which)) {
return BuildHatParamPackageForButton(joystick->GetPort(), joystick->GetGUID(), return BuildHatParamPackageForButton(joystick->GetPort(), joystick->GetGUID(),
static_cast<s32>(event.jhat.hat), static_cast<s32>(event.jhat.hat),
static_cast<s32>(event.jhat.value)); static_cast<s32>(event.jhat.value));
} }
break;
}
} }
return {}; return {};
} }
@ -820,22 +813,28 @@ Common::ParamPackage SDLEventToButtonParamPackage(SDLState& state, const SDL_Eve
Common::ParamPackage SDLEventToMotionParamPackage(SDLState& state, const SDL_Event& event) { Common::ParamPackage SDLEventToMotionParamPackage(SDLState& state, const SDL_Event& event) {
switch (event.type) { switch (event.type) {
case SDL_JOYAXISMOTION: { case SDL_JOYAXISMOTION: {
const auto joystick = state.GetSDLJoystickBySDLID(event.jaxis.which); if (const auto joystick = state.GetSDLJoystickBySDLID(event.jaxis.which)) {
return BuildAnalogParamPackageForButton(joystick->GetPort(), joystick->GetGUID(), return BuildAnalogParamPackageForButton(joystick->GetPort(), joystick->GetGUID(),
static_cast<s32>(event.jaxis.axis), static_cast<s32>(event.jaxis.axis),
event.jaxis.value); event.jaxis.value);
} }
break;
}
case SDL_JOYBUTTONUP: { case SDL_JOYBUTTONUP: {
const auto joystick = state.GetSDLJoystickBySDLID(event.jbutton.which); if (const auto joystick = state.GetSDLJoystickBySDLID(event.jbutton.which)) {
return BuildButtonParamPackageForButton(joystick->GetPort(), joystick->GetGUID(), return BuildButtonParamPackageForButton(joystick->GetPort(), joystick->GetGUID(),
static_cast<s32>(event.jbutton.button)); static_cast<s32>(event.jbutton.button));
} }
break;
}
case SDL_JOYHATMOTION: { case SDL_JOYHATMOTION: {
const auto joystick = state.GetSDLJoystickBySDLID(event.jhat.which); if (const auto joystick = state.GetSDLJoystickBySDLID(event.jhat.which)) {
return BuildHatParamPackageForButton(joystick->GetPort(), joystick->GetGUID(), return BuildHatParamPackageForButton(joystick->GetPort(), joystick->GetGUID(),
static_cast<s32>(event.jhat.hat), static_cast<s32>(event.jhat.hat),
static_cast<s32>(event.jhat.value)); static_cast<s32>(event.jhat.value));
} }
break;
}
} }
return {}; return {};
} }
@ -1062,9 +1061,8 @@ public:
// Simplify controller config by testing if game controller support is enabled. // Simplify controller config by testing if game controller support is enabled.
if (event.type == SDL_JOYAXISMOTION) { if (event.type == SDL_JOYAXISMOTION) {
const auto axis = event.jaxis.axis; const auto axis = event.jaxis.axis;
const auto joystick = state.GetSDLJoystickBySDLID(event.jaxis.which); if (const auto joystick = state.GetSDLJoystickBySDLID(event.jaxis.which);
auto* const controller = joystick->GetSDLGameController(); auto* const controller = joystick->GetSDLGameController()) {
if (controller) {
const auto axis_left_x = const auto axis_left_x =
SDL_GameControllerGetBindForAxis(controller, SDL_CONTROLLER_AXIS_LEFTX) SDL_GameControllerGetBindForAxis(controller, SDL_CONTROLLER_AXIS_LEFTX)
.value.axis; .value.axis;
@ -1098,13 +1096,14 @@ public:
} }
if (analog_x_axis != -1 && analog_y_axis != -1) { if (analog_x_axis != -1 && analog_y_axis != -1) {
const auto joystick = state.GetSDLJoystickBySDLID(event.jaxis.which); if (const auto joystick = state.GetSDLJoystickBySDLID(event.jaxis.which)) {
auto params = BuildParamPackageForAnalog(joystick->GetPort(), joystick->GetGUID(), auto params = BuildParamPackageForAnalog(joystick->GetPort(), joystick->GetGUID(),
analog_x_axis, analog_y_axis); analog_x_axis, analog_y_axis);
analog_x_axis = -1; analog_x_axis = -1;
analog_y_axis = -1; analog_y_axis = -1;
return params; return params;
} }
}
return {}; return {};
} }