Class management to differentiate between player keys

Question

Making a pong clone, and having some trouble with motion and classes. So I have a class Game that manages two instances of a class Paddle and in the constructor of the Game class initiate the variables in the two different paddle classes and set the variable player to 1 or 2 based on which keys the class will respond to for motion. Then inside the Paddle class events function I use an if(player == 1) or if(player == 2) to tell when keys to check for to change the y velocity of the paddle. The problem I'm having is that because the Game class's events function looks like this:

void Game::events()
{
    PlayerOne.events();
    PlayerTwo.events();
    Game_Ball.events();
}

And the events function and the class definition of the Paddle class look like this:

class Paddle
{
public:
    int player;
    SDL_Surface *sprite = NULL;
    float x, y, w, h;
    float yVel;
    SDL_Rect *clip;
    void events();
    void logic(Uint32 deltaTicks);
    void render();
    Paddle();
    ~Paddle();
};

void Paddle::events()
{
    while (SDL_PollEvent(&event))
    {
        Uint8 *keystates = SDL_GetKeyState(NULL);
        if (player == 1)
        {
            if (keystates[SDLK_o] == 1)
            {
                yVel -= 100;
            }
            if (keystates[SDLK_k] == 1)
            {
                yVel += 100;
            }
        }
        else
        {
            if (keystates[SDLK_o] == 0)
            {
                yVel += 100;
            }
            if (keystates[SDLK_k] == 0)
            {
                yVel -= 100;
            }
        }
        if (player == 2)
        {
            if (keystates[SDLK_w] == 1)
            {
                yVel -= 100;
            }
            if (keystates[SDLK_s] == 1)
            {
                yVel += 100;
            }
        }
        else
        {
            if (keystates[SDLK_w] == 0)
            {
                yVel += 100;
            }
            if (keystates[SDLK_s] == 0)
            {
                yVel -= 100;
            }
        }
    }
}

Something is happening that mixes up the keys. Usually PlayerOne uses "O/K" to move and PlayerTwo uses "W/S" to move, but sometimes I press either and the opposite class will move. It doesn't seem to make sense. Here is the Game class constructor and definition

class Game : public GameState
{
private:
    int server;
    TTF_Font *score = NULL;
    Paddle PlayerOne;
    Paddle PlayerTwo;
    Ball Game_Ball;
public:
    SDL_Rect clip[2];
    void events();
    void logic();
    void render();
    Game();
    ~Game();
};

Game::Game()
{
    //RESOURSES
    PlayerOne.sprite = load_image("Game_sprite.png");
    PlayerTwo.sprite = load_image("Game_sprite.png");
    clip[0].x = 0;
    clip[0].y = 0;
    clip[0].w = 20;
    clip[0].h = 20;
    clip[1].x = 0;
    clip[1].y = 20;
    clip[1].w = 20;
    clip[1].h = 100;
    //PLAYER ONE
    PlayerOne.x = 420;
    PlayerOne.y = 100;
    PlayerOne.w = 60;
    PlayerOne.h = 100;
    PlayerOne.clip = &clip[1];
    PlayerOne.yVel = 0;
    PlayerOne.player = 1;
    //PLAYER TWO
    PlayerTwo.x = 60;
    PlayerTwo.y = 100;
    PlayerTwo.w = 60;
    PlayerTwo.h = 100;
    PlayerTwo.clip = &clip[1];
    PlayerTwo.yVel = 0;
    PlayerTwo.player = 2;
    //BALL
    Game_Ball.Ball_Bound.x = 190;
    Game_Ball.Ball_Bound.y = 190;
    Game_Ball.Ball_Bound.w = 60;
    Game_Ball.Ball_Bound.h = 60;
    Game_Ball.clip = &clip[0];
}

player is clearly set in both objects and the events section of the Paddle class clearly differentiates between them, so, why do the keys get mixed up? I could easily write another class for a second player, but that would be inefficient, I'd rather use two copies of the same class that does everything the same except the keystates. So, I'm not exactly sure what's up.

Answer 1

On first glance it seems that you could just use:

    if (player == 1)
    {
        if (keystates[SDLK_o] == 1)
        {
            yVel -= 100;
        }
        if (keystates[SDLK_k] == 1)
        {
            yVel += 100;
        }
    }
    else // player == 2
    {
        if (keystates[SDLK_w] == 1)
        {
            yVel -= 100;
        }
        if (keystates[SDLK_s] == 1)
        {
            yVel += 100;
        }
    }

Would I be correct in guessing that when the wrong cursor moves, O would for example move the wrong cursor downwards?

Answer 2

Problem seems to be the fact that you have

if (player == 1) {
  check keys O K pressed
}
else {
  check keys O K released
} 

which makes no sense since this modifies values related to keys O K released on player2.

In any case you shouldn't let any game object poll for events. The main game loop should pool them once and dispatch them to the corresponding game elements, eg:

void gameLoop() {
    while (true) {
    draw();
    updatePositions();
    handleEvents();
  }
}

void handleEvents() {
  while (SDL_PollEvent(&event)) {
    Uint8 *keystates = SDL_GetKeyState(NULL);

    if (keystates[SDLK_o]) paddle1->startMovingUp();
    else if (keystates[SDLK_k]) paddle1->startMovingDown();
    else paddle1->stopMoving();

    // same thing for player2
  }
}

Paddle::startMovingUp() {
  uvelY = -100;
}

Paddle::startMovingDown() {
  uvelY = 100;
}

Paddle::stopMoving() {
  uvelY = 0;
}

Paddle::updatePosition() {  // called on each frame
  positionY += uvelY;
}

You can simplify the design and just update positions in the handleEvents() functions but separating what is the position update from what is the management of event is a good thing to do.