-
Notifications
You must be signed in to change notification settings - Fork 2
[#62] [WIP] En Passant Logic For Pawn #48
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
base: master
Are you sure you want to change the base?
Changes from all commits
b1e2c8e
5418744
00755cc
ff0f3b7
d8db2ed
3fc6f00
4bad9e0
11ad23c
2dab1d0
728cb71
3d78eb0
30efba2
cbdc2e8
681a00f
00ee14c
b7d4f74
b9d8175
f64eeb4
666acce
6b1ba61
68ce476
08b4406
2f3b195
a19fa07
97fd6df
43aa66d
1883571
80a9573
63194d8
2c09cd3
4a2b24a
d8c230a
db950b6
9d9af53
775dd87
50c24f8
0e1383f
8adf4ee
463a9ee
d4ca87d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,7 +6,7 @@ class Pawn < Piece | |
| # takes place by moving forward. Check, and checkmate logic | ||
| # are not implemented yet and thus ignored. | ||
| def valid_move?(x, y) | ||
| return true if fwd_diagonal_attack?(x, y) | ||
| return true if fwd_diagonal_attack?(x, y) || en_passant_capture?(x, y) | ||
| moved ? one_fwd_move?(x, y) : first_fwd_move?(x, y) && !fwd_attack?(x, y) | ||
| end | ||
|
|
||
|
|
@@ -20,6 +20,49 @@ def promote!(new_type) | |
|
|
||
| private | ||
|
|
||
| # En Passant Logic: | ||
| # 1)target pawn just moved 2 space forward | ||
| # 2)only capture within the next turn | ||
| # 3)can only capture by opponent's pawn | ||
| # 4)opponent pawn must be on the left or right of target piece's y axis | ||
|
|
||
| # En Passant Capture logic | ||
| def en_passant_capture?(x, y) | ||
| return true if en_passant_possible?(x, y) || en_passant_y_diff?(x, y) | ||
| end | ||
|
|
||
| # Return false if victim piece is nil or same color | ||
| # Return true if victim piece's y axis difference is equal to 1 | ||
| def en_passant_y_diff?(x, y) | ||
| victim = last_moved_piece | ||
| return false if victim.nil? || victim.color == color | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. last moved piece will be always opponents piece, so
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since turn base logic is not implemented yet, this is needed at this point. Will refactor this method turn base logic is added in separate PR.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn't turn-based logic completely implemented in this PR? Turn in game model, start turn at 1, increment turn at end of every successful move.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It about 50% done in this PR, same color can move multiple turns in a row in this PR. I am working on the other half right now.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm confused by this, Is the turn logic being implemented in this PR then? We definitely require turn logic for a valid en passant |
||
| return true if x == victim.x_position && | ||
| (y == victim.y_position - 1 || y == victim.y_position + 1) | ||
| end | ||
|
|
||
| # Return false if last move piece is nil (Start of game) | ||
| # Return false last moved piece is not pawn or its y axis difference is !2 | ||
| # Check both both adjacent side of pawn | ||
| # If any have valid En Passant pawn piece, return true | ||
| def en_passant_possible?(x, y) | ||
| return false if last_moved_piece.nil? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do you need this guard clause if
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This line is require for edge case, where at first turn there is no last moved piece.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I understand that, but why do you need this guard clause? If the statement returns nil, it will be handled by next statement.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would cause an error in the next statement since you can't call any method on nil object.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Guards should be as close to the error as possible so as to guarantee that the error is caught in the right spot & reduce the need for logic duplication. |
||
| return false unless last_moved_piece.y_distance(y) == 2 && | ||
| type == 'Pawn' | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand this correctly, you moved y_distance to a public method and are now invoking it on the last_moved_piece, which would be the pawn victim. But if you pass in the coordinate of where pawn is going to "attack" as the coordinate behind where the victim is now, I don't see how this logic could work. If victim is at 3,3 and aggressor pawn is at 4,3, calling this method on 3,2 which is where aggressor pawn would 'attack' would fail this check. It would resolve as: abs(victim.y_position - y) EDIT: Are you intending on passing in the coords of where the victim is or the tile in which pawn is attacking?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The intent of this method is to pass the coordinates of where victim piece is at. I just refactor the method; now it works with |
||
| [valid_en_passant_pawn?(x, y, true), valid_en_passant_pawn?(x, y, false)] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this bit should be also made more readable
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method refactor version of following methods: def valid_en_passant_pawn?(x, y)
return false unless valid_passant_pawn_left?(x, y)
return false unless valid_passant_pawn_right?(x. y)
false
end
def valid_passant_pawn_left?(x, y)
left_piece = find_piece(x - 1, y)
return false if left_piece.nil?
return true if left_piece.type == 'Pawn' && side_piece.color != color
false
end
def valid_passant_pawn_right?(x, y)
right_piece = find_piece(x + 1, y)
return false if right_piece.nil?
return true if right_piece.type == 'Pawn' && side_piece.color != color
false
end |
||
| .include?(true) | ||
| end | ||
|
|
||
| # Check for any adjacent piece | ||
| # Return false if none if found | ||
| # Return true if opponent's pawn is found | ||
| def valid_en_passant_pawn?(x, y, adjacent) | ||
| adjacent_piece = occupant_piece(x + (adjacent ? 1 : -1), y) | ||
| return false if adjacent_piece.nil? | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't need this guard clause.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Tried to remove this line, but it causes the method to return Nil class error.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain what the adjacent parameter is doing? I see you passing true and false between these 2 methods but they are not commented.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just because a pawn is adjacent to another pawn does not make it a valid en passant victim so I think the name and return of this method are misleading. What makes it valid is this PLUS the victim having moved 2 spaces forward last turn.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This method checks if there is any valid pawn adjacent to the victim pawn(include? true or false). If any of adjacent side have a valid pawn, it will return true. A separate method check if the pawn moved 2 space forward in the last turn. This was the rafactored version of following methods: def valid_en_passant_pawn?(x, y)
return false unless valid_passant_pawn_left?(x, y)
return false unless valid_passant_pawn_right?(x. y)
false
end
def valid_passant_pawn_left?(x, y)
left_piece = find_piece(x - 1, y)
return false if left_piece.nil?
return true if left_piece.type == 'Pawn' && side_piece.color != color
false
end
def valid_passant_pawn_right?(x, y)
right_piece = find_piece(x + 1, y)
return false if right_piece.nil?
return true if right_piece.type == 'Pawn' && side_piece.color != color
false
end
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't see the other method you are referring to that checks the 2 spaces forward. IMO this logic should all be one method, it is really hard to follow otherwise. |
||
| return true if adjacent_piece.type == 'Pawn' && | ||
| adjacent_piece.color != color | ||
| false | ||
| end | ||
|
|
||
| # Returns true if the coordinates provided are 1 tile forward | ||
| # from the piece's origin and no capture occured. | ||
| def one_fwd_move?(x, y) | ||
|
|
@@ -63,4 +106,9 @@ def fwd_diagonal_attack?(x, y) | |
| y_position - 1 | ||
| end | ||
| end | ||
|
|
||
| # Find the last moved piece by subtracting current move number - 1 | ||
| def last_moved_piece | ||
| game.pieces.find_by(turn_last_moved: game.turn - 1) | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| #Application.rb | ||
| require File.expand_path('../boot', __FILE__) | ||
|
|
||
| require 'rails/all' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,9 @@ | ||
| # Be sure to restart your server when you modify this file. | ||
|
|
||
| # You can add backtrace silencers for libraries that you're using but don't wish to see in your backtraces. | ||
| # You can add backtrace silencers for libraries that you're using | ||
| # but don't wish to see in your backtraces. | ||
| # Rails.backtrace_cleaner.add_silencer { |line| line =~ /my_noisy_library/ } | ||
|
|
||
| # You can also remove all the silencers if you're trying to debug a problem that might stem from framework code. | ||
| # You can also remove all the silencers if you're trying to debug a problem | ||
| # that might stem from framework code. | ||
| # Rails.backtrace_cleaner.remove_silencers! |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| # Be sure to restart your server when you modify this file. | ||
|
|
||
| StoicChess::Application.config.session_store :cookie_store, key: '_StoicChess_session' | ||
| StoicChess::Application.config.session_store :cookie_store, key: | ||
| '_StoicChess_session' |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| #devise auto migration | ||
| class DeviseCreateUsers < ActiveRecord::Migration | ||
| def change | ||
| create_table(:users) do |t| | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class AddMoveNumberToGames < ActiveRecord::Migration[5.0] | ||
| def change | ||
| add_column :games, :move_number, :integer | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class AddEnPassantToGames < ActiveRecord::Migration[5.0] | ||
| def change | ||
| add_column :games, :en_passant, :boolean, default: false | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class ChangeTurnTypeToString < ActiveRecord::Migration[5.0] | ||
| def change | ||
| change_column :games, :turn, :string | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class AddLastMovedPieceToPiecesTable < ActiveRecord::Migration[5.0] | ||
| def change | ||
| add_column :pieces, :last_moved_piece, :integer | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class RemoveEnPassantFromGame < ActiveRecord::Migration[5.0] | ||
| def change | ||
| remove_column :games, :en_passant, :boolean | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| class ChangeTurnFromGames < ActiveRecord::Migration[5.0] | ||
| def change | ||
| remove_column :games, :turn, :integer | ||
| remove_column :games, :move_number, :integer | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class AddTurnToGames < ActiveRecord::Migration[5.0] | ||
| def change | ||
| add_column :games, :turn, :integer | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class ChangeLastMovedPieceFromPieces < ActiveRecord::Migration[5.0] | ||
| def change | ||
| rename_column :pieces, :last_moved_piece, :turn_last_moved | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what step 3 means. En Passant does not require that the ONLY piece available to capture the victim be the pawn, only that the pawn COULD have captured it in the first space on the previous turn.