Added pawn move logic in Game model, and test

I wrote some code in the Game model that tests whether a pawn move is valid, and an accompanying test for it using FactoryGirl. 

The new "self.is_move_allowed? function in Game.rb first calls a helper function, one per piece type (I wrote one for pawns called "legit_moves_for_pawn"), which returns an array of x/y coordinates that a specific piece (in this case a pawn) can move to. (This could also be used to generate a list of highlighted squares indicating possible target destinations for a piece.) The "is_move_allowed?" function then tests whether the new [x, y] destination coordinates that the player has selected are within that returned array of valid coordinates.

Hopefully this is a good template for writing the piece move logic for other pieces. Thoughts?
Thanks Ken! I was wondering about that too, whether it should go in the
Game or Piece model.

At the moment the code assumes the pieces are unobstructed, but later on
we'll want to evolve these functions to check for other pieces getting in
the way. Since that involves knowing about other pieces in the game, I
opted to initially put it all in the Game model. But I'm definitely open to
suggestions on this.

Do you think it'll still be better putting it all in Piece? Or splitting
the functionality into two, so that the code in the Piece model only
applies the basic move rules, whilst extra code in the Game model checks
for obstructions?

Good idea r.e. abstracting the move logic - will give that some thought.
Although I think this only really applies to the pawns, since all other
pieces can move forwards and backwards, so it may not be much of an issue?

Cheers,

Rehan



On 15 December 2014 at 00:47, ken mazaika <notifications@github.com> wrote:
>
> In app/models/game.rb
> <https://github.com/theFirehoseProject/chess/pull/24#discussion-diff-21801567>
> :
>
> > @@ -230,4 +230,51 @@ def initialize_the_board!
> >  			:image => 'black-rook.gif'
> >  			)
> >  	end
> > +
> > +	def self.is_move_allowed?(piece_id, new_x, new_y)
>
> Instead of having this in the game model, what do you think about moving
> it into the piece model?
>
> piece = Piece.find(1)
> piece.is_move_allowed?(5, 5)
>
> —
> Reply to this email directly or view it on GitHub
> <https://github.com/theFirehoseProject/chess/pull/24/files#r21801567>.
>