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.
This commit is contained in:
Techjar 2018-11-19 05:30:39 -05:00
parent e6b2758ab4
commit 529796bd59
6 changed files with 37 additions and 37 deletions

View File

@ -340,7 +340,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet)
case NP_MSG_PAD_MAPPING: case NP_MSG_PAD_MAPPING:
{ {
for (PadMapping& mapping : m_pad_map) for (PlayerId& mapping : m_pad_map)
{ {
packet >> mapping; packet >> mapping;
} }
@ -353,7 +353,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet)
case NP_MSG_WIIMOTE_MAPPING: case NP_MSG_WIIMOTE_MAPPING:
{ {
for (PadMapping& mapping : m_wiimote_map) for (PlayerId& mapping : m_wiimote_map)
{ {
packet >> mapping; packet >> mapping;
} }
@ -366,7 +366,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet)
{ {
while (!packet.endOfPacket()) while (!packet.endOfPacket())
{ {
PadMapping map; PadIndex map;
packet >> map; packet >> map;
GCPadStatus pad; GCPadStatus pad;
@ -383,7 +383,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet)
case NP_MSG_WIIMOTE_DATA: case NP_MSG_WIIMOTE_DATA:
{ {
PadMapping map = 0; PadIndex map;
NetWiimote nw; NetWiimote nw;
u8 size; u8 size;
packet >> map >> size; packet >> map >> size;
@ -412,7 +412,7 @@ unsigned int NetPlayClient::OnData(sf::Packet& packet)
case NP_MSG_PAD_FIRST_RECEIVED: case NP_MSG_PAD_FIRST_RECEIVED:
{ {
PadMapping map; PadIndex map;
packet >> map; packet >> map;
packet >> m_first_pad_status_received[map]; packet >> m_first_pad_status_received[map];
m_first_pad_status_received_event.Set(); 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, void NetPlayClient::AddPadStateToPacket(const int in_game_pad, const GCPadStatus& pad,
sf::Packet& packet) sf::Packet& packet)
{ {
packet << static_cast<PadMapping>(in_game_pad); packet << static_cast<PadIndex>(in_game_pad);
packet << pad.button << pad.analogA << pad.analogB << pad.stickX << pad.stickY << pad.substickX packet << pad.button << pad.analogA << pad.analogB << pad.stickX << pad.stickY << pad.substickX
<< pad.substickY << pad.triggerLeft << pad.triggerRight << pad.isConnected; << 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; sf::Packet packet;
packet << static_cast<MessageId>(NP_MSG_WIIMOTE_DATA); packet << static_cast<MessageId>(NP_MSG_WIIMOTE_DATA);
packet << static_cast<PadMapping>(in_game_pad); packet << static_cast<PadIndex>(in_game_pad);
packet << static_cast<u8>(nw.size()); packet << static_cast<u8>(nw.size());
for (auto it : nw) for (auto it : nw)
{ {
@ -1814,7 +1814,7 @@ bool NetPlayClient::PollLocalPad(const int local_pad, sf::Packet& packet)
return data_added; return data_added;
} }
void NetPlayClient::SendPadHostPoll(const PadMapping pad_num) void NetPlayClient::SendPadHostPoll(const PadIndex pad_num)
{ {
if (!m_local_player->IsHost()) if (!m_local_player->IsHost())
return; 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]) while (!m_first_pad_status_received[pad_num])
{ {
@ -1909,7 +1909,7 @@ void NetPlayClient::SendPowerButtonEvent()
// called from ---GUI--- thread // called from ---GUI--- thread
bool NetPlayClient::LocalPlayerHasControllerMapped() const 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; return mapping == m_local_player->pid;
}; };

View File

@ -197,7 +197,7 @@ private:
std::optional<std::vector<u8>> DecompressPacketIntoBuffer(sf::Packet& packet); std::optional<std::vector<u8>> DecompressPacketIntoBuffer(sf::Packet& packet);
bool PollLocalPad(int local_pad, sf::Packet& packet); bool PollLocalPad(int local_pad, sf::Packet& packet);
void SendPadHostPoll(PadMapping pad_num); void SendPadHostPoll(PadIndex pad_num);
void UpdateDevices(); void UpdateDevices();
void AddPadStateToPacket(int in_game_pad, const GCPadStatus& np, sf::Packet& packet); void AddPadStateToPacket(int in_game_pad, const GCPadStatus& np, sf::Packet& packet);

View File

@ -184,8 +184,8 @@ using NetWiimote = std::vector<u8>;
using MessageId = u8; using MessageId = u8;
using PlayerId = u8; using PlayerId = u8;
using FrameNum = u32; using FrameNum = u32;
using PadMapping = s8; using PadIndex = s8;
using PadMappingArray = std::array<PadMapping, 4>; using PadMappingArray = std::array<PlayerId, 4>;
bool IsNetPlayRunning(); bool IsNetPlayRunning();
// Precondition: A netplay client instance must be present. In other words, // Precondition: A netplay client instance must be present. In other words,

View File

@ -96,8 +96,8 @@ NetPlayServer::NetPlayServer(const u16 port, const bool forward_port,
PanicAlertT("Enet Didn't Initialize"); PanicAlertT("Enet Didn't Initialize");
} }
m_pad_map.fill(-1); m_pad_map.fill(0);
m_wiimote_map.fill(-1); m_wiimote_map.fill(0);
if (traversal_config.use_traversal) if (traversal_config.use_traversal)
{ {
@ -301,9 +301,9 @@ unsigned int NetPlayServer::OnConnect(ENetPeer* socket)
enet_packet_destroy(epack); enet_packet_destroy(epack);
// try to automatically assign new user a pad // 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; mapping = player.pid;
break; break;
@ -396,7 +396,7 @@ unsigned int NetPlayServer::OnDisconnect(const Client& player)
if (m_is_running) if (m_is_running)
{ {
for (PadMapping mapping : m_pad_map) for (PlayerId& mapping : m_pad_map)
{ {
if (mapping == pid && pid != 1) if (mapping == pid && pid != 1)
{ {
@ -406,7 +406,7 @@ unsigned int NetPlayServer::OnDisconnect(const Client& player)
sf::Packet spac; sf::Packet spac;
spac << (MessageId)NP_MSG_DISABLE_GAME; spac << (MessageId)NP_MSG_DISABLE_GAME;
// this thread doesn't need players lock // this thread doesn't need players lock
SendToClients(spac, static_cast<PlayerId>(-1)); SendToClients(spac);
break; break;
} }
} }
@ -426,20 +426,20 @@ unsigned int NetPlayServer::OnDisconnect(const Client& player)
// alert other players of disconnect // alert other players of disconnect
SendToClients(spac); SendToClients(spac);
for (PadMapping& mapping : m_pad_map) for (PlayerId& mapping : m_pad_map)
{ {
if (mapping == pid) if (mapping == pid)
{ {
mapping = -1; mapping = 0;
UpdatePadMapping(); UpdatePadMapping();
} }
} }
for (PadMapping& mapping : m_wiimote_map) for (PlayerId& mapping : m_wiimote_map)
{ {
if (mapping == pid) if (mapping == pid)
{ {
mapping = -1; mapping = 0;
UpdateWiimoteMapping(); UpdateWiimoteMapping();
} }
} }
@ -477,7 +477,7 @@ void NetPlayServer::UpdatePadMapping()
{ {
sf::Packet spac; sf::Packet spac;
spac << (MessageId)NP_MSG_PAD_MAPPING; spac << (MessageId)NP_MSG_PAD_MAPPING;
for (PadMapping mapping : m_pad_map) for (PlayerId mapping : m_pad_map)
{ {
spac << mapping; spac << mapping;
} }
@ -489,7 +489,7 @@ void NetPlayServer::UpdateWiimoteMapping()
{ {
sf::Packet spac; sf::Packet spac;
spac << (MessageId)NP_MSG_WIIMOTE_MAPPING; spac << (MessageId)NP_MSG_WIIMOTE_MAPPING;
for (PadMapping mapping : m_wiimote_map) for (PlayerId mapping : m_wiimote_map)
{ {
spac << mapping; spac << mapping;
} }
@ -577,7 +577,7 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player)
while (!packet.endOfPacket()) while (!packet.endOfPacket())
{ {
PadMapping map; PadIndex map;
packet >> map; packet >> map;
// If the data is not from the correct player, // 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: case NP_MSG_PAD_HOST_POLL:
{ {
PadMapping pad_num; PadIndex pad_num;
packet >> pad_num; packet >> pad_num;
sf::Packet spac; 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++) for (size_t i = 0; i < m_pad_map.size(); i++)
{ {
if (m_pad_map[i] == -1) if (m_pad_map[i] == 0)
continue; continue;
const GCPadStatus& pad = m_last_pad_status[i]; const GCPadStatus& pad = m_last_pad_status[i];
spac << static_cast<PadMapping>(i) << pad.button << pad.analogA << pad.analogB << pad.stickX spac << static_cast<PadIndex>(i) << pad.button << pad.analogA << pad.analogB << pad.stickX
<< pad.stickY << pad.substickX << pad.substickY << pad.triggerLeft << pad.triggerRight << pad.stickY << pad.substickX << pad.substickY << pad.triggerLeft << pad.triggerRight
<< pad.isConnected; << 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]; const GCPadStatus& pad = m_last_pad_status[pad_num];
spac << pad_num << pad.button << pad.analogA << pad.analogB << pad.stickX << pad.stickY 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) if (player.current_game != m_current_game)
break; break;
PadMapping map = 0; PadIndex map;
u8 size; u8 size;
packet >> map >> size; packet >> map >> size;
std::vector<u8> data(size); std::vector<u8> data(size);
@ -774,7 +774,7 @@ unsigned int NetPlayServer::OnData(sf::Packet& packet, Client& player)
return pair.second == timebases[0].second; return pair.second == timebases[0].second;
})) }))
{ {
int pid_to_blame = -1; int pid_to_blame = 0;
for (auto pair : timebases) for (auto pair : timebases)
{ {
if (std::all_of(timebases.begin(), timebases.end(), [&](std::pair<PlayerId, u64> other) { if (std::all_of(timebases.begin(), timebases.end(), [&](std::pair<PlayerId, u64> other) {
@ -1591,7 +1591,7 @@ bool NetPlayServer::CompressBufferIntoPacket(const std::vector<u8>& in_buffer, s
return true; return true;
} }
void NetPlayServer::SendFirstReceivedToHost(const PadMapping map, const bool state) void NetPlayServer::SendFirstReceivedToHost(const PadIndex map, const bool state)
{ {
sf::Packet pac; sf::Packet pac;
pac << static_cast<MessageId>(NP_MSG_PAD_FIRST_RECEIVED); pac << static_cast<MessageId>(NP_MSG_PAD_FIRST_RECEIVED);

View File

@ -89,7 +89,7 @@ private:
void CheckSyncAndStartGame(); void CheckSyncAndStartGame();
bool CompressFileIntoPacket(const std::string& file_path, sf::Packet& packet); bool CompressFileIntoPacket(const std::string& file_path, sf::Packet& packet);
bool CompressBufferIntoPacket(const std::vector<u8>& in_buffer, sf::Packet& packet); bool CompressBufferIntoPacket(const std::vector<u8>& in_buffer, sf::Packet& packet);
void SendFirstReceivedToHost(PadMapping map, bool state); void SendFirstReceivedToHost(PadIndex map, bool state);
u64 GetInitialNetPlayRTC() const; u64 GetInitialNetPlayRTC() const;

View File

@ -90,7 +90,7 @@ int PadMappingDialog::exec()
const auto index = gc ? m_pad_mapping[i] : m_wii_mapping[i]; 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 gc_id = m_gc_boxes[i]->currentIndex();
int wii_id = m_wii_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_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 : -1; m_wii_mapping[i] = wii_id > 0 ? m_players[wii_id - 1]->pid : 0;
} }
} }