Skip to content

Connect four #23

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ source 'http://rubygems.org'

gem 'rspec'
gem 'pry'
gem 'debugger'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can lose this -- that's what pry is for! Just use binding.pry where you would use debugger. plop right into a running ruby interpreter! :)


# Specify your gem's dependencies in percival.gemspec
gemspec
4 changes: 2 additions & 2 deletions Rakefile
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ task :start do
c.plugins.plugins = [ClockPlugin,
LoggerPlugin,
ChannelChangerPlugin,
NameChangerPlugin]
NameChangerPlugin,
ConnectFourPlugin]
end
end

bot.start
end
1 change: 1 addition & 0 deletions lib/percival.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
require 'percival/logger'
require 'percival/channel_changer'
require 'percival/name_changer'
require 'percival/connect_four'

PERCIVAL_ROOT = File.dirname(File.dirname(__FILE__))

3 changes: 3 additions & 0 deletions lib/percival/connect_four.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
require 'percival/connect_four/plugin'
require 'percival/connect_four/connect_four'
require 'percival/connect_four/board_score'
109 changes: 109 additions & 0 deletions lib/percival/connect_four/board_score.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
class BoardScore

WIN_VALUE= 10000
DEPTH = 4
attr_accessor :score, :win

def initialize board
@board = board
@win = false
@score = 0
score_board
end

def terminal
return true if @win
end


def score_board
@board.each_with_index do |column,i|
column.each_with_index do |color, j|
# up
count = counter(color) do |k|
@board[i][j+k]
end
return if won? count, color
@score += color * (count ** 2)

#up diag
count = counter(color) do |k|
i+k < @board.size ? @board[i+k][j+k] : nil
end
return if won? count, color
@score += color * (count ** 2)

#right
count = counter(color) do |k|
i+k < @board.size ? @board[i+k][j] : nil
end
return if won? count, color

@score += color * (count ** 2)

#down right diag
count = counter(color) do |k|
j-k >= 0 and i+k < @board.size ? @board[i+k][j-k] : nil
end
return if won? count, color
@score += color * (count ** 2)

#down
count = counter(color) do |k|
j-k >= 0 ? @board[i][j-k] : nil
end
return if won? count, color
@score += color * (count ** 2)

#down left diag
count = counter(color) do |k|
j-k >= 0 and i-k >= 0 ? @board[i-k][j-k] : nil
end
return if won? count, color
@score += color * (count ** 2)

# left
count = counter(color) do |k|
i-k >= 0 ? @board[i-k][j] : nil
end
return if won? count, color
@score += color * (count ** 2)

#up left diag
count = counter(color) do |k|
i-k >= 0 ? @board[i-k][j+k] : nil
end
return if won? count, color
@score += color * (count ** 2)

end
end
end

def counter e_color
count = 0
(0..3).each do |k|
color = yield k
if color == - e_color
count = 0
break
end
count += 1 if color == e_color
end
count
end

def won? count, color
if count == 4
@win = color
@score = @win * WIN_VALUE
return true
end
false
end
end





104 changes: 104 additions & 0 deletions lib/percival/connect_four/connect_four.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
require 'debugger'

class ConnectFour
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably have some tests for this class in isolation of the plugin functionality. That might give us some opportunity to refactor this so that we have some better MVC in here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried writing this TDD style but it was tough and I gave up and did it the old way. That being said I spent 4 hours or so trying to figure out why it was the worst player ever and had no idea how to track down the bug. It was a minus sign that should have been a plus.

I think I'm getting more comfortable with tests now though. I'm actually doing what looks to me like real TDD in the blackjack plugin and the LPMC Casino api plugin. However I'm still not sure how to test this. By it's nature it's so open ended I don't really know what to test and how. Do I test that it chooses the right move? I don't even know what the right move is. I can test that the board scoring is working but there are thousands of possible boards and I don't really know what boards to test that would provide enough coverage. (those aren't really specific questions I'm asking. They're just illustrative examples of the kind of issues I'm having with the concept of testing)

At the very least we can provide tests for some of the smaller helper methods though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule #1 of TDD, if it's hard to test, you've probably got a problem in the code.

Rule #2 of TDD, most code is hard to test.

Lemma #1 : Most code has problems (follows from Rule #2 and Rule #1).


I haven't looked at this code thoroughly yet, my guess, however, is that you
have only two classes for a problem which probably desires more. I have a set of
Maxims about programming I use often, but one of the most important is one I
stole from the greats, it's called "SOLID" -- and the important bit for us to
consider is the "S"; the "Single Responsibility Principle"

The idea of the SRP is that each class should have precisely one responsibility
(but crucially, not necessarily one role) in the system. Here, we have three
classes(ish) representing all of the code necessary to play connect four. To
see why this is a problem, let's look at a breakdown of a problem in prose:


To play Connect Four, I first need a board, a board has many positions,
arranged in a grid. Each position has neighbors. There are precisely two
players, and each have many positions. The positions on the board specify a
subset of "fillable" positions, starting with an arbitrary player, each takes
turns choosing one of the fillable positions and adding a position to it's
position-set. The board then recalculates it's position set according to the
new set of "taken" positions by adding the position directly above the taken
position, and adding the taken position to it's set of "Filled" positions. Any
position that is not Filled is Empty. An arbitrator can see that one of the
players has won when they see that one of the player owns four positions in a
horizontal, vertical, or diagonal row.


So, we can try to parse that, and read out the actors of the system -- an Actor
is a particular responsibility that needs filling, I'll go through and highlight
the actors now:


To play Connect Four, I first need a board, a board has many positions,
arranged in a grid. Each position has neighbors. There are precisely two
players, and each have many positions. The positions on the board specify a
subset of "fillable" positions, starting with an arbitrary player, each
takes turns choosing one of the fillable positions and adding a position to it's
position-set. The board then recalculates it's position set according to the
new set of "taken" positions by adding the position directly above the taken
position, and adding the taken position to it's set of "Filled" positions. Any
position that is not Filled is Empty. An arbitrator can see that one of the
players has won when they see that one of the player owns four positions in a
horizontal, vertical, or diagonal row.


So we can see that the Actors of the system are:

  • The Board
  • The Positions
    • A subconcept of "Fillable" positions
    • A subconcept of "Filled" positions
    • A subconcept of "Empty" positions
  • The Players
    • A subconcept of the AI-player
  • The Arbitrator

So far, I'm counting 4 or 5 (I'm not sure how I want to model Fillable positions
yet) actors. Since we're building an AI, we can treat it as a special case of
the Player model.

At this point, We can go back and look at the relationships between models, I
won't highlight them as before, but they look a bit like:

  • The Board
    • Has-many positions
      • Has-subset of "Filled" positions
    • Belongs-to two players and an arbitrator
  • The Positions
    • Has-many Neighbors
    • Belongs-to a Player or noone
  • The Players
    • Has-many positions
    • Can see the board -- and thus other player's positions.
  • The Arbitrator
    • Can see the whole board -- and thus all player positions.

After looking at the relationships, we can start to think about responsibilities
for each model -- We can see that the board will be primarily an information
"Source" -- where we pull information about what positions are
fillable/empty/filled etc.

A position is a bit different, players will be manipulating them often, we will
need to know if a position is filled or fillable or empty, we'll need to have
the ability to 'take' a filled position, and we'll need to be able to see which
player owns the position.

The players are our "primary" actors, most of the interaction with the program
is through them, they should probably be able to identify the set of fillable
positions, and give us a method for choosing one and adding it to the owned-set
of positions they have. For the AI player, we might want to model a "Brain"
which makes the decision to choose from the given set of fillable positions and
the state of the board which position to choose.

Finally, the Arbitrator is really just encapsulating the "win" condition, he
shoud have a method like #check! which returns either false or the player who
won the game[1].

So, already we can see there is a little more apparent complexity (inasmuch as
there are probably 5 or 6 classes here), but less actual complexity, since
each object is only doing one thing. I might therefore start w/ tests that just
lay out the API, like:

# in board_spec.rb
describe Board do
  it { should respond_to :positions }
  it { should respond_to :fillable_positions }
  it { should respond_to :players } 
end

#in position_spec.rb
describe Position do
  it { should repsond_to :filled? }
  it { should respond_to :empty? }
  it { should respond_to :owner }
  it { should respond_to :fillable } 

  describe "a filled/taken position" do
    #intentionally blank
  end

  describe "an empty position" do
    #intentionally blank
  end
end

#etc

These respond_to tests are largely just notes, they help me to remember what
the API is as I'm developing, sometimes I leave them in, there are a lot of
arguments as to whether or not they're useful/useless/harmful. I like them.
Specifically, I like to keep them at the top of my specs, so that I can
at-a-glance see what the API of my object is, which helps later when mocking or
even just using my library/app.

Now we can start speccing. Let's look at the position spec

#in position_spec.rb
describe Position do
  it { should repsond_to :filled? }
  it { should respond_to :empty? }
  it { should respond_to :owner }
  it { should respond_to :fillable? } 

  let (:position) { Position.new } 
  let (:player) { double("Player 1") } 
  let (:board) { double("Board") }

  subject { position } 

  describe "a filled/taken position" do
    before do
      position.stub(:owner).and_return(player) 
    end

    it { should_not be_empty } 
    it { should_not be_fillable } 
    it { should be_filled } 
  end

  describe "an empty position" do
    it { should be_empty } 
    its(:owner) { should be_nil } 

    context "an empty, unfillable position" do
      before do
        #appropriate mocking to make the board specify this position as
        #not-fillable
      end

      it { should_not be_fillable }  
    end

    context "an empty, fillable position" do
      before do
        #appropriate mocking to make the board specify this position as
        #fillable
      end

      it { should be_fillable } 
    end
  end
end

Now hopefully you can see how easy it is to spec that model, and further, the
implementation of those things should be pretty obvious -- I used mocks to avoid
instantiating a dependency, instead just noting that in one context, the
#owner method returns something, and in another it returns nothing. I used
context to specify the sub-condition of fillability. Importantly -- I haven't
written any code yet, just specified how this class should behave, and what API
it should support. I can now build the Position class in total isolation of
everything else. Rinse and repeat to build up the other classes, and vois la, a
really nice OO design that will be easy to maintain and well-tested! A FOSS
Maintainers Dream!


Hopefully that helps elucidate a bit, I want to add the disclaimer that I
didn't run any of this through an interpreter or anything, so there might be
syntax bugs and stuff. Take everything with a grain of salt.

/Joe


[1] This is a pretty common technique, in Haskell they call it the "Maybe"
datatype, elsewhere it's called the "Nullable Value" pattern. The idea is that
you can return either "Nothing" -- which means nothing happened, or a specific
sort of "Something", which acts-as-true, and also carries information about what
kind of truth that is.

INF = 10000
DEPTH = 5
attr_accessor :board

def initialize width, height
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Parens

@width, @height = width, height
@my_board = []
@your_board = []
@width.times { @my_board.push [] }
end

def your_move move
@your_board = new_board @my_board, move, -1
rendering = "
You dropped a piece at #{move + 1}
#{board_to_string(@your_board)}"

bs = BoardScore.new(@your_board)
if bs.win
return "You win" + rendering
end
return rendering
end

def my_move
my_move = get_move(@your_board)
@my_board = new_board(@your_board, my_move, 1)
rendering = "
I dropped a piece at #{my_move + 1}
#{board_to_string(@my_board)}"

bs = BoardScore.new(@my_board)
if bs.win
return "I win!!\n\n" + rendering
end
return rendering
end

def you_won
"You Won!!! #can't happen"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that hash cause the rest of the line to be a comment? I guess it must not, ruby is so clever.

end

def i_won
"I Won!!!"
end

def get_move board
idx = 0
min = INF
(0..board.size - 1).each do |i|
nb = new_board board, i, 1
val = negamax nb, DEPTH, -INF, INF, -1

if val < min
min = val
idx = i
end
end
return idx
end

#negamax alpha beta pruning http://en.wikipedia.org/wiki/Negamax
#much better than my mickey mouse implementation of minimax
def negamax board, depth, alpha, beta, color
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to extract the minimax algorithm here into it's own class/module?

bs = BoardScore.new board
if depth == 0 or bs.terminal
return color * bs.score
end
(0..(board.size - 1)).each do |i|
nb = new_board board, i, color
val = - negamax( nb, depth - 1, - beta, - alpha, - color)
return val if val >= beta
alpha = val if val >= alpha
end
return alpha
end

def new_board board, move, color
nb = Marshal.load(Marshal.dump(board))
nb[move].push color
nb
end

def board_to_string b
l = []
(0..@height-1).to_a.reverse.each do |i|
e = []
(0..@width - 1).each do |j|
if b[j][i].nil?
e << ' '
elsif b[j][i] > 0
e << 'X'
elsif b[j][i] < 0
e << '0'
end
end
l << e.join(' ')
end
return '|' + l.join("|\n|") + "|\n|1 2 3 4 5 6 7|"
end
end
23 changes: 23 additions & 0 deletions lib/percival/connect_four/plugin.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
class ConnectFourPlugin
include Cinch::Plugin

def initialize *args
super
@games = { }
end

match /cf\s+(\S+)?/, :method => :connect_four

def connect_four m, command
if /new/.match command
@games[m.user.name] = ConnectFour.new 7,6
m.reply "Awaiting your move sir"
elsif /[1-7]/.match command
@games[m.user.name] ||= ConnectFour.new 7,6
m.reply @games[m.user.name].your_move(command.to_i - 1)
m.reply @games[m.user.name].my_move
else
m.reply "command must be in [new|[1-7]]"
end
end
end