From 529796bd590c36c03978a43f1ee6eed0f4cba29d Mon Sep 17 00:00:00 2001 From: Techjar Date: Mon, 19 Nov 2018 05:30:39 -0500 Subject: [PATCH] NetPlay: Remove PadMapping type Its usage was inconsistent, confusing, and buggy, so I opted to just remove it entirely. It has been replaced with PadIndex for the appropriate instances (mainly networking), and inappropriate usages (where it was really just a player ID) have been replaced with the PlayerId type. The definition of "no mapping" has been changed from -1 to 0 to match the defintion of "no player", as -1 (255 unsigned) is actually a valid player ID. The bugs never manifested because it only occurs with a full lobby of 255 players, at which point the last player's ID collides with the "no mapping" definition and some undefined behavior occurs. Nevertheless, I thought it best to fix it anyways as the usage of PadMapping was confusing. --- Source/Core/Core/NetPlayClient.cpp | 20 +++++----- Source/Core/Core/NetPlayClient.h | 2 +- Source/Core/Core/NetPlayProto.h | 4 +- Source/Core/Core/NetPlayServer.cpp | 40 +++++++++---------- Source/Core/Core/NetPlayServer.h | 2 +- .../DolphinQt/NetPlay/PadMappingDialog.cpp | 6 +-- 6 files changed, 37 insertions(+), 37 deletions(-) diff --git a/Source/Core/Core/NetPlayClient.cpp b/Source/Core/Core/NetPlayClient.cpp index 6cc5d84528..59377554f6 100644 --- a/Source/Core/Core/NetPlayClient.cpp +++ b/Source/Core/Core/NetPlayClient.cpp @@ -340,7 +340,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) case NP_MSG_PAD_MAPPING: { - for (PadMapping& mapping : m_pad_map) + for (PlayerId& mapping : m_pad_map) { packet >> mapping; } @@ -353,7 +353,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) case NP_MSG_WIIMOTE_MAPPING: { - for (PadMapping& mapping : m_wiimote_map) + for (PlayerId& mapping : m_wiimote_map) { packet >> mapping; } @@ -366,7 +366,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) { while (!packet.endOfPacket()) { - PadMapping map; + PadIndex map; packet >> map; GCPadStatus pad; @@ -383,7 +383,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) case NP_MSG_WIIMOTE_DATA: { - PadMapping map = 0; + PadIndex map; NetWiimote nw; u8 size; packet >> map >> size; @@ -412,7 +412,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet) case NP_MSG_PAD_FIRST_RECEIVED: { - PadMapping map; + PadIndex map; packet >> map; packet >> m_first_pad_status_received[map]; m_first_pad_status_received_event.Set(); @@ -1250,7 +1250,7 @@ void NetPlayClient::SendChatMessage(const std::string& msg) void NetPlayClient::AddPadStateToPacket(const int in_game_pad, const GCPadStatus& pad, sf::Packet& packet) { - packet << static_cast(in_game_pad); + packet << static_cast(in_game_pad); packet << pad.button << pad.analogA << pad.analogB << pad.stickX << pad.stickY << pad.substickX << pad.substickY << pad.triggerLeft << pad.triggerRight << pad.isConnected; } @@ -1260,7 +1260,7 @@ void NetPlayClient::SendWiimoteState(const int in_game_pad, const NetWiimote& nw { sf::Packet packet; packet << static_cast(NP_MSG_WIIMOTE_DATA); - packet << static_cast(in_game_pad); + packet << static_cast(in_game_pad); packet << static_cast(nw.size()); for (auto it : nw) { @@ -1814,7 +1814,7 @@ bool NetPlayClient::PollLocalPad(const int local_pad, sf::Packet& packet) return data_added; } -void NetPlayClient::SendPadHostPoll(const PadMapping pad_num) +void NetPlayClient::SendPadHostPoll(const PadIndex pad_num) { if (!m_local_player->IsHost()) return; @@ -1835,7 +1835,7 @@ void NetPlayClient::SendPadHostPoll(const PadMapping pad_num) } } } - else if (m_pad_map[pad_num] > 0) + else if (m_pad_map[pad_num] != 0) { while (!m_first_pad_status_received[pad_num]) { @@ -1909,7 +1909,7 @@ void NetPlayClient::SendPowerButtonEvent() // called from ---GUI--- thread bool NetPlayClient::LocalPlayerHasControllerMapped() const { - const auto mapping_matches_player_id = [this](const PadMapping& mapping) { + const auto mapping_matches_player_id = [this](const PlayerId& mapping) { return mapping == m_local_player->pid; }; diff --git a/Source/Core/Core/NetPlayClient.h b/Source/Core/Core/NetPlayClient.h index 6b3746c2b6..6f6adad9c9 100644 --- a/Source/Core/Core/NetPlayClient.h +++ b/Source/Core/Core/NetPlayClient.h @@ -197,7 +197,7 @@ private: std::optional> DecompressPacketIntoBuffer(sf::Packet& packet); bool PollLocalPad(int local_pad, sf::Packet& packet); - void SendPadHostPoll(PadMapping pad_num); + void SendPadHostPoll(PadIndex pad_num); void UpdateDevices(); void AddPadStateToPacket(int in_game_pad, const GCPadStatus& np, sf::Packet& packet); diff --git a/Source/Core/Core/NetPlayProto.h b/Source/Core/Core/NetPlayProto.h index a45bc8f96d..5354161e73 100644 --- a/Source/Core/Core/NetPlayProto.h +++ b/Source/Core/Core/NetPlayProto.h @@ -184,8 +184,8 @@ using NetWiimote = std::vector; using MessageId = u8; using PlayerId = u8; using FrameNum = u32; -using PadMapping = s8; -using PadMappingArray = std::array; +using PadIndex = s8; +using PadMappingArray = std::array; bool IsNetPlayRunning(); // Precondition: A netplay client instance must be present. In other words, diff --git a/Source/Core/Core/NetPlayServer.cpp b/Source/Core/Core/NetPlayServer.cpp index dc1c05896d..a2c3f12fac 100644 --- a/Source/Core/Core/NetPlayServer.cpp +++ b/Source/Core/Core/NetPlayServer.cpp @@ -96,8 +96,8 @@ NetPlayServer::NetPlayServer(const u16 port, const bool forward_port, PanicAlertT("Enet Didn't Initialize"); } - m_pad_map.fill(-1); - m_wiimote_map.fill(-1); + m_pad_map.fill(0); + m_wiimote_map.fill(0); if (traversal_config.use_traversal) { @@ -301,9 +301,9 @@ unsigned int NetPlayServer::OnConnect(ENetPeer* socket) enet_packet_destroy(epack); // try to automatically assign new user a pad - for (PadMapping& mapping : m_pad_map) + for (PlayerId& mapping : m_pad_map) { - if (mapping == -1) + if (mapping == 0) { mapping = player.pid; break; @@ -396,7 +396,7 @@ unsigned int NetPlayServer::OnDisconnect(const Client& player) if (m_is_running) { - for (PadMapping mapping : m_pad_map) + for (PlayerId& mapping : m_pad_map) { if (mapping == pid && pid != 1) { @@ -406,7 +406,7 @@ unsigned int NetPlayServer::OnDisconnect(const Client& player) sf::Packet spac; spac << (MessageId)NP_MSG_DISABLE_GAME; // this thread doesn't need players lock - SendToClients(spac, static_cast(-1)); + SendToClients(spac); break; } } @@ -426,20 +426,20 @@ unsigned int NetPlayServer::OnDisconnect(const Client& player) // alert other players of disconnect SendToClients(spac); - for (PadMapping& mapping : m_pad_map) + for (PlayerId& mapping : m_pad_map) { if (mapping == pid) { - mapping = -1; + mapping = 0; UpdatePadMapping(); } } - for (PadMapping& mapping : m_wiimote_map) + for (PlayerId& mapping : m_wiimote_map) { if (mapping == pid) { - mapping = -1; + mapping = 0; UpdateWiimoteMapping(); } } @@ -477,7 +477,7 @@ void NetPlayServer::UpdatePadMapping() { sf::Packet spac; spac << (MessageId)NP_MSG_PAD_MAPPING; - for (PadMapping mapping : m_pad_map) + for (PlayerId mapping : m_pad_map) { spac << mapping; } @@ -489,7 +489,7 @@ void NetPlayServer::UpdateWiimoteMapping() { sf::Packet spac; spac << (MessageId)NP_MSG_WIIMOTE_MAPPING; - for (PadMapping mapping : m_wiimote_map) + for (PlayerId mapping : m_wiimote_map) { spac << mapping; } @@ -577,7 +577,7 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) while (!packet.endOfPacket()) { - PadMapping map; + PadIndex map; packet >> map; // If the data is not from the correct player, @@ -616,7 +616,7 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) case NP_MSG_PAD_HOST_POLL: { - PadMapping pad_num; + PadIndex pad_num; packet >> pad_num; sf::Packet spac; @@ -626,16 +626,16 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) { for (size_t i = 0; i < m_pad_map.size(); i++) { - if (m_pad_map[i] == -1) + if (m_pad_map[i] == 0) continue; const GCPadStatus& pad = m_last_pad_status[i]; - spac << static_cast(i) << pad.button << pad.analogA << pad.analogB << pad.stickX + spac << static_cast(i) << pad.button << pad.analogA << pad.analogB << pad.stickX << pad.stickY << pad.substickX << pad.substickY << pad.triggerLeft << pad.triggerRight << pad.isConnected; } } - else if (m_pad_map.at(pad_num) != -1) + else if (m_pad_map.at(pad_num) != 0) { const GCPadStatus& pad = m_last_pad_status[pad_num]; spac << pad_num << pad.button << pad.analogA << pad.analogB << pad.stickX << pad.stickY @@ -653,7 +653,7 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) if (player.current_game != m_current_game) break; - PadMapping map = 0; + PadIndex map; u8 size; packet >> map >> size; std::vector data(size); @@ -774,7 +774,7 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player) return pair.second == timebases[0].second; })) { - int pid_to_blame = -1; + int pid_to_blame = 0; for (auto pair : timebases) { if (std::all_of(timebases.begin(), timebases.end(), [&](std::pair other) { @@ -1591,7 +1591,7 @@ bool NetPlayServer::CompressBufferIntoPacket(const std::vector& in_buffer, s return true; } -void NetPlayServer::SendFirstReceivedToHost(const PadMapping map, const bool state) +void NetPlayServer::SendFirstReceivedToHost(const PadIndex map, const bool state) { sf::Packet pac; pac << static_cast(NP_MSG_PAD_FIRST_RECEIVED); diff --git a/Source/Core/Core/NetPlayServer.h b/Source/Core/Core/NetPlayServer.h index ce75d2605d..b2f30bae1c 100644 --- a/Source/Core/Core/NetPlayServer.h +++ b/Source/Core/Core/NetPlayServer.h @@ -89,7 +89,7 @@ private: void CheckSyncAndStartGame(); bool CompressFileIntoPacket(const std::string& file_path, sf::Packet& packet); bool CompressBufferIntoPacket(const std::vector& in_buffer, sf::Packet& packet); - void SendFirstReceivedToHost(PadMapping map, bool state); + void SendFirstReceivedToHost(PadIndex map, bool state); u64 GetInitialNetPlayRTC() const; diff --git a/Source/Core/DolphinQt/NetPlay/PadMappingDialog.cpp b/Source/Core/DolphinQt/NetPlay/PadMappingDialog.cpp index 3d374de49c..388d0f1dc3 100644 --- a/Source/Core/DolphinQt/NetPlay/PadMappingDialog.cpp +++ b/Source/Core/DolphinQt/NetPlay/PadMappingDialog.cpp @@ -90,7 +90,7 @@ int PadMappingDialog::exec() const auto index = gc ? m_pad_mapping[i] : m_wii_mapping[i]; - combo->setCurrentIndex(index == -1 ? 0 : index); + combo->setCurrentIndex(index); } } @@ -114,7 +114,7 @@ void PadMappingDialog::OnMappingChanged() int gc_id = m_gc_boxes[i]->currentIndex(); int wii_id = m_wii_boxes[i]->currentIndex(); - m_pad_mapping[i] = gc_id > 0 ? m_players[gc_id - 1]->pid : -1; - m_wii_mapping[i] = wii_id > 0 ? m_players[wii_id - 1]->pid : -1; + m_pad_mapping[i] = gc_id > 0 ? m_players[gc_id - 1]->pid : 0; + m_wii_mapping[i] = wii_id > 0 ? m_players[wii_id - 1]->pid : 0; } }