Qt: Clean up multiplayer attaching/detaching

src/platform/qt/InputController.cpp

@@ -25,9 +25,11 @@
 
 using namespace QGBA;
 
-InputController::InputController(int playerId, QWidget* topLevel, QObject* parent)
+int InputController::s_claimedPlayers = 0;
+
+InputController::InputController(QWidget* topLevel, QObject* parent)
 	: QObject(parent)
-	, m_playerId(playerId)
+	, m_playerId(claimPlayer())
 	, m_topLevel(topLevel)
 	, m_focusParent(topLevel)
 {
@@ -130,6 +132,7 @@ InputController::InputController(int playerId, QWidget* topLevel, QObject* paren
 
 InputController::~InputController() {
 	mInputMapDeinit(&m_inputMap);
+	freePlayer(m_playerId);
 }
 
 void InputController::addInputDriver(std::shared_ptr<InputDriver> driver) {
@@ -550,6 +553,20 @@ bool InputController::hasPendingEvent(int key) const {
 	return m_pendingEvents.contains(key);
 }
 
+int InputController::claimPlayer() {
+	for (int i = 0; i < MAX_GBAS; ++i) {
+		if (!(s_claimedPlayers & (1 << i))) {
+			s_claimedPlayers |= 1 << i;
+			return i;
+		}
+	}
+	qFatal("Can't claim 5th player. Please report this bug.");
+}
+
+void InputController::freePlayer(int player) {
+	s_claimedPlayers &= ~(1 << player);
+}
+
 void InputController::stealFocus(QWidget* focus) {
 	m_focusParent = focus;
 }

src/platform/qt/InputController.h

@@ -53,7 +53,7 @@ public:
 
 	static const uint32_t KEYBOARD = 0x51545F4B;
 
-	InputController(int playerId = 0, QWidget* topLevel = nullptr, QObject* parent = nullptr);
+	InputController(QWidget* topLevel = nullptr, QObject* parent = nullptr);
 	~InputController();
 
 	void addInputDriver(std::shared_ptr<InputDriver>);
@@ -140,6 +140,9 @@ private:
 	bool hasPendingEvent(int key) const;
 	void sendGamepadEvent(QEvent*);
 
+	static int claimPlayer();
+	static void freePlayer(int);
+
 	Gamepad* gamepad(uint32_t type);
 	QList<Gamepad*> gamepads();
 
@@ -172,6 +175,7 @@ private:
 #endif
 #endif
 
+	static int s_claimedPlayers;
 	mInputMap m_inputMap;
 	ConfigController* m_config = nullptr;
 	int m_playerId;

src/platform/qt/MultiplayerController.cpp

@@ -6,6 +6,7 @@
 #include "MultiplayerController.h"
 
 #include "CoreController.h"
+#include "LogController.h"
 
 #ifdef M_CORE_GBA
 #include <mgba/internal/gba/gba.h>
@@ -18,21 +19,10 @@
 
 using namespace QGBA;
 
-#ifdef M_CORE_GB
-MultiplayerController::Player::Player(CoreController* coreController, GBSIOLockstepNode* gbNode)
+MultiplayerController::Player::Player(CoreController* coreController)
 	: controller(coreController)
 {
-	node.gb = gbNode;
 }
-#endif
-
-#ifdef M_CORE_GBA
-MultiplayerController::Player::Player(CoreController* coreController, GBASIOLockstepNode* gbaNode)
-	: controller(coreController)
-{
-	node.gba = gbaNode;
-}
-#endif
 
 int MultiplayerController::Player::id() const {
 	switch (controller->platform()) {
@@ -67,7 +57,7 @@ MultiplayerController::MultiplayerController() {
 	};
 	m_lockstep.signal = [](mLockstep* lockstep, unsigned mask) {
 		MultiplayerController* controller = static_cast<MultiplayerController*>(lockstep->context);
-		Player* player = &controller->m_players[0];
+		Player* player = controller->player(0);
 		bool woke = false;
 		player->waitMask &= ~mask;
 		if (!player->waitMask && player->awake < 1) {
@@ -79,7 +69,7 @@ MultiplayerController::MultiplayerController() {
 	};
 	m_lockstep.wait = [](mLockstep* lockstep, unsigned mask) {
 		MultiplayerController* controller = static_cast<MultiplayerController*>(lockstep->context);
-		Player* player = &controller->m_players[0];
+		Player* player = controller->player(0);
 		bool slept = false;
 		player->waitMask |= mask;
 		if (player->awake > 0) {
@@ -214,6 +204,12 @@ MultiplayerController::~MultiplayerController() {
 }
 
 bool MultiplayerController::attachGame(CoreController* controller) {
+	QList<CoreController::Interrupter> interrupters;
+	interrupters.append(controller);
+	for (Player& p : m_pids.values()) {
+		interrupters.append(p.controller);
+	}
+
 	if (m_lockstep.attached == 0) {
 		switch (controller->platform()) {
 #ifdef M_CORE_GBA
@@ -239,6 +235,7 @@ bool MultiplayerController::attachGame(CoreController* controller) {
 		return false;
 	}
 
+	Player player{controller};
 	switch (controller->platform()) {
 #ifdef M_CORE_GBA
 	case mPLATFORM_GBA: {
@@ -251,13 +248,11 @@ bool MultiplayerController::attachGame(CoreController* controller) {
 		GBASIOLockstepNode* node = new GBASIOLockstepNode;
 		GBASIOLockstepNodeCreate(node);
 		GBASIOLockstepAttachNode(&m_gbaLockstep, node);
-		m_players.append({controller, node});
+		player.node.gba = node;
 
 		GBASIOSetDriver(&gba->sio, &node->d, SIO_MULTI);
 		GBASIOSetDriver(&gba->sio, &node->d, SIO_NORMAL_32);
-
-		emit gameAttached();
-		return true;
+		break;
 	}
 #endif
 #ifdef M_CORE_GB
@@ -271,19 +266,22 @@ bool MultiplayerController::attachGame(CoreController* controller) {
 		GBSIOLockstepNode* node = new GBSIOLockstepNode;
 		GBSIOLockstepNodeCreate(node);
 		GBSIOLockstepAttachNode(&m_gbLockstep, node);
-		m_players.append({controller, node});
+		player.node.gb = node;
 
 		GBSIOSetDriver(&gb->sio, &node->d);
-
-		emit gameAttached();
-		return true;
+		break;
 	}
 #endif
 	default:
-		break;
+		return false;
 	}
 
-	return false;
+	m_pids.insert(m_nextPid, player);
+	++m_nextPid;
+	fixOrder();
+
+	emit gameAttached();
+	return true;
 }
 
 void MultiplayerController::detachGame(CoreController* controller) {
@@ -296,8 +294,18 @@ void MultiplayerController::detachGame(CoreController* controller) {
 	}
 	QList<CoreController::Interrupter> interrupters;
 
+	int pid = -1;
 	for (int i = 0; i < m_players.count(); ++i) {
-		interrupters.append(m_players[i].controller);
+		Player* p = player(i);
+		if (!p) {
+			LOG(QT, ERROR) << tr("Trying to detach a multiplayer player that's not attached");
+			return;
+		}
+		CoreController* playerController = p->controller;
+		if (playerController == controller) {
+			pid = m_players[i];
+		}
+		interrupters.append(playerController);
 	}
 	switch (controller->platform()) {
 #ifdef M_CORE_GBA
@@ -329,18 +337,23 @@ void MultiplayerController::detachGame(CoreController* controller) {
 		break;
 	}
 
-	for (int i = 0; i < m_players.count(); ++i) {
-		if (m_players[i].controller == controller) {
-			m_players.removeAt(i);
-			break;
-		}
+	m_pids.remove(pid);
+	if (m_pids.size() == 0) {
+		m_platform = mPLATFORM_NONE;
+	} else {
+		fixOrder();
 	}
 	emit gameDetached();
 }
 
 int MultiplayerController::playerId(CoreController* controller) const {
 	for (int i = 0; i < m_players.count(); ++i) {
-		if (m_players[i].controller == controller) {
+		const Player* p = player(i);
+		if (!p) {
+			LOG(QT, ERROR) << tr("Trying to get player ID for a multiplayer player that's not attached");
+			return -1;
+		}
+		if (p->controller == controller) {
 			return i;
 		}
 	}
@@ -354,27 +367,52 @@ int MultiplayerController::attached() {
 }
 
 MultiplayerController::Player* MultiplayerController::player(int id) {
-	Player* player = &m_players[id];
-	switch (player->controller->platform()) {
+	if (id >= m_players.size()) {
+		return nullptr;
+	}
+	int pid = m_players[id];
+	auto iter = m_pids.find(pid);
+	if (iter == m_pids.end()) {
+		return nullptr;
+	}
+	return &iter.value();
+}
+
+const MultiplayerController::Player* MultiplayerController::player(int id) const {
+	if (id >= m_players.size()) {
+		return nullptr;
+	}
+	int pid = m_players[id];
+	auto iter = m_pids.find(pid);
+	if (iter == m_pids.end()) {
+		return nullptr;
+	}
+	return &iter.value();
+}
+
+void MultiplayerController::fixOrder() {
+	m_players.clear();
+	m_players = m_pids.keys();
+	std::sort(m_players.begin(), m_players.end());
+	switch (m_platform) {
 #ifdef M_CORE_GBA
 	case mPLATFORM_GBA:
-		if (player->node.gba->id != id) {
-			std::sort(m_players.begin(), m_players.end());
-			player = &m_players[id];
+		for (int pid : m_pids.keys()) {
+			Player& p = m_pids.find(pid).value();
+			GBA* gba = static_cast<GBA*>(p.controller->thread()->core->board);
+			GBASIOLockstepNode* node = reinterpret_cast<GBASIOLockstepNode*>(gba->sio.drivers.multiplayer);
+			m_players[node->id] = pid;
 		}
 		break;
 #endif
 #ifdef M_CORE_GB
 	case mPLATFORM_GB:
-		if (player->node.gb->id != id) {
+		if (player(0)->node.gb->id == 1) {
 			std::swap(m_players[0], m_players[1]);
-			player = &m_players[id];
 		}
 		break;
 #endif
 	case mPLATFORM_NONE:
 		break;
 	}
-
-	return player;
 }

src/platform/qt/MultiplayerController.h

@@ -5,6 +5,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 #pragma once
 
+#include <QHash>
 #include <QList>
 #include <QMutex>
 #include <QObject>
@@ -50,12 +51,7 @@ private:
 		GBASIOLockstepNode* gba;
 	};
 	struct Player {
-#ifdef M_CORE_GB
-		Player(CoreController* controller, GBSIOLockstepNode* node);
-#endif
-#ifdef M_CORE_GBA
-		Player(CoreController* controller, GBASIOLockstepNode* node);
-#endif
+		Player(CoreController* controller);
 
 		int id() const;
 		bool operator<(const Player&) const;
@@ -68,6 +64,8 @@ private:
 	};
 
 	Player* player(int id);
+	const Player* player(int id) const;
+	void fixOrder();
 
 	union {
 		mLockstep m_lockstep;
@@ -80,7 +78,9 @@ private:
 	};
 
 	mPlatform m_platform = mPLATFORM_NONE;
-	QList<Player> m_players;
+	int m_nextPid = 0;
+	QHash<int, Player> m_pids;
+	QList<int> m_players;
 	QMutex m_lock;
 };
 

src/platform/qt/Window.cpp

@@ -92,7 +92,7 @@ Window::Window(CoreManager* manager, ConfigController* config, int playerId, QWi
 	, m_logView(new LogView(&m_log, this))
 	, m_screenWidget(new WindowBackground())
 	, m_config(config)
-	, m_inputController(playerId, this)
+	, m_inputController(this)
 	, m_shortcutController(new ShortcutController(this))
 	, m_playerId(playerId)
 {
@@ -257,12 +257,7 @@ void Window::resizeFrame(const QSize& size) {
 
 void Window::updateMultiplayerStatus(bool canOpenAnother) {
 	m_multiWindow->setEnabled(canOpenAnother);
-	if (m_controller) {
-		MultiplayerController* multiplayer = m_controller->multiplayerController();
-		if (multiplayer) {
-			m_playerId = multiplayer->playerId(m_controller.get());
-		}
-	}
+	multiplayerChanged();
 }
 
 void Window::updateMultiplayerActive(bool active) {
@@ -412,6 +407,7 @@ void Window::multiplayerChanged() {
 	MultiplayerController* multiplayer = m_controller->multiplayerController();
 	if (multiplayer) {
 		attached = multiplayer->attached();
+		m_playerId = multiplayer->playerId(m_controller.get());
 	}
 	for (Action* action : m_nonMpActions) {
 		action->setEnabled(attached < 2);