New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NetPlay: Remove PadMapping type #7580

Open
wants to merge 1 commit into
base: master
from

Conversation

1 participant
@Techjar
Contributor

Techjar commented Nov 19, 2018

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.

@Techjar Techjar force-pushed the Techjar:netplay-padmapping-type branch from 4b7a75b to a3a6b38 Nov 19, 2018

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.

@Techjar Techjar force-pushed the Techjar:netplay-padmapping-type branch from a3a6b38 to 529796b Nov 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment