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

Bug Fixes #63

merged 7 commits into from Dec 21, 2015


None yet
2 participants
Copy link

jonathanpike commented Dec 18, 2015

Lots of bugs squashed in this PR:

  1. As per issue #59, the AJAX request to move a piece was returning 404 (not found). Turns out, this was because of check_player_color. Since it was being called with the before_action callback, any time :game_id wasn't in params (ie. whenever we're doing something with the pieces controller), it was returning 404. I've switched it to find a Piece by id and then a Game by piece.game_id, and also changed it to only check before update.
  2. As per issue #58, the user was not being given any feedback when they made an invalid move. I added a Flash alert informing them. This was not as easy as it seems, as I had to make the Flash alert return status 303 (See Other) rather than the usual 302 (Redirect) when using redirect_to. This was because it was using PUT because the redirect was being called from update, and since GamesController#update isn't where we wanted to go (not to mention it doesn't have a template), it was causing problems.
  3. The most insidious bug was also the hardest to find, as per issue #62. This was an intermittent bug that seemed to only affect certain pieces. The culprit? The way we were calling @piece in move_to!. We weren't specifying a game_id, so it was checking every single game for pieces that existed in the square we were moving to. Hence, pieces constantly thought that they were moving to a square that had one of their own pieces currently in it.
  4. As per issue #64, I fixed the capturing piece jumping to an adjacent square when moved on the captured piece. The Javascript initially used .appendTo() to essentially append the capturing piece to the captured piece's square. This ended up nesting the capturing piece in the captured piece's span tag, which pushed it to the next row. I've changed this behaviour to use .replaceWith(), which essentially takes the captured piece off of the board in the DOM and replaces it with the capturing piece.

Also, I moved the inline javascript from games/show to a separate file, games.js. Just thought it was better for organization in the future.

ronny2205 added a commit that referenced this pull request Dec 21, 2015

@ronny2205 ronny2205 merged commit 85ef5e5 into master Dec 21, 2015

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
continuous-integration/travis-ci/push The Travis CI build passed

@jonathanpike jonathanpike deleted the bug_fixes branch Dec 21, 2015

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