moving a lot of logic into the game model to remove duplication with …

…worker

app/helpers/games_helper.rb

@@ -1,9 +1,9 @@
 module GamesHelper
   THUMB_DIR = Rails.root + "public" + "assets" + "games" + "thumbs"
 
-  def color_at(vertex, game = @game)
-    if    game.black_positions_list.include? vertex then "b"
-    elsif game.white_positions_list.include? vertex then "w"
+  def color_at(position, game = @game)
+    if    game.black_positions_list.include? position then "b"
+    elsif game.white_positions_list.include? position then "w"
     else                                                 "e"
     end
   end

app/models/game.rb

@@ -94,7 +94,7 @@ def prepare
       self.black_player = opponent
       self.white_player = creator
     end
-    game_engine do |engine|
+    GameEngine.run(attributes.symbolize_keys) do |engine|
       if handicap.to_i.nonzero?
         self.black_positions = engine.positions(:black)
         self.current_player = white_player
@@ -105,25 +105,13 @@ def prepare
     self.position_changed = true
   end
 
-  def move(vertex, user)
+  # todo: This method needs to be tested better
+  def move(position, user)
     raise GameEngine::OutOfTurn if user.id != current_player_id
-    game_engine do |engine|
-      engine.replay(moves)
-      self.moves = [moves, engine.move(current_color, vertex)].reject(&:blank?).join("-")
-      self.last_move_at = Time.now
-      self.black_positions = engine.positions(:black)
-      self.white_positions = engine.positions(:white)
-      self.current_player = next_player
-      if engine.over?
-        self.finished_at = Time.now
-        self.black_score = engine.black_score
-        self.white_score = engine.white_score
-      else
-        self.black_score = engine.captures(:black)
-        self.white_score = engine.captures(:white)
-        self.position_changed = true
-      end
+    GameEngine.update_game_attributes_with_move(attributes.symbolize_keys, position).each do |name, value|
+      self.send("#{name}=", value)
     end
+    self.position_changed = true # todo: this could be made smarter
     # Check current_player again, fetching from database to async double move problem
     # This should probably be moved into a database lock so no updates happen between here and the save
     raise GameEngine::OutOfTurn if user.id != Game.find(id, :select => "current_player_id").current_player_id
@@ -156,6 +144,10 @@ def next_player
     current_player == black_player ? white_player : black_player
   end
 
+  def next_player_id
+    next_player.id if next_player
+  end
+
   def finished?
     not finished_at.blank?
   end
@@ -174,11 +166,6 @@ def update_thumbnail
 
   def profile_for(color)
     Profile.new(color).tap do |profile|
-      if color.to_sym == :white
-        profile.handicap_or_komi = "#{komi} komi"
-      else
-        profile.handicap_or_komi = "#{handicap} handicap"
-      end
       if color.to_sym == :white
         profile.handicap_or_komi = "#{komi} komi"
       else
@@ -200,12 +187,4 @@ def profile_for(color)
   def profiles
     [profile_for(:white), profile_for(:black)]
   end
-
-  private
-
-  def game_engine
-    GameEngine.run(:board_size => board_size, :handicap => handicap, :komi => komi) do |engine|
-      yield engine
-    end
-  end
 end

app/views/games/show.html.erb

@@ -19,8 +19,8 @@
       <div id="board_spaces">
       <% for y in ('a'..'s').first(@game.board_size) %>
         <% for x in ('a'..'s').first(@game.board_size) %>
-          <% vertex = x + y %>
-          <div class="<%= color_at vertex %><%= ' last' if @game.last_move == vertex %>" id="<%= vertex %>"></div>
+          <% position = x + y %>
+          <div class="<%= color_at position %><%= ' last' if @game.last_move == position %>" id="<%= position %>"></div>
         <% end %>
       <% end %>
       </div>

lib/game_engine.rb

@@ -3,7 +3,7 @@ class Error < StandardError; end
   class IllegalMove < Error; end
   class OutOfTurn < Error; end
 
-  def self.run(options = {}, &block)
+  def self.run(options = {})
     Go::GTP.run_gnugo do |gtp|
       gtp.boardsize(options[:board_size]) unless options[:board_size].to_s.empty?
       gtp.fixed_handicap(options[:handicap]) if options[:handicap].to_i.nonzero?
@@ -12,48 +12,76 @@ def self.run(options = {}, &block)
     end
   end
 
+  def self.update_game_attributes_with_move(game, move = nil)
+    update = {}
+    run(game) do |engine|
+      update.merge!(engine.update_game_attributes_with_move(game, move))
+    end
+    update
+  end
+
   def initialize(gtp, options = {})
     @resigned = nil
     @gtp = gtp
     @board_size = options[:board_size] || 19
-    @handicap = options[:handicap] || 0
+    @handicap = options[:handicap].to_i
+    @current_color = @handicap > 0 ? :white : :black
   end
 
   def replay(moves)
-    colors = [:black, :white].cycle
-    colors.next if first_color.to_sym == :white
     moves.to_s.split("-").each do |move|
-      play(colors.next, (move =~ /[A-Z]/ ? move : move[0..1]))
+      play(move =~ /[A-Z]/ ? move : move[0..1])
     end
   end
 
-  def play(color, vertex)
-    if vertex == "RESIGN"
-      @resigned = color
+  # Play at the given position, nil for computer play
+  def play(position = nil)
+    if position.nil?
+      position = sgf_point(@gtp.genmove(@current_color))
     else
-      @gtp.play(color, gnugo_point(vertex))
+      @gtp.play(@current_color, gnugo_point(position)) unless position == "RESIGN"
+      raise IllegalMove unless @gtp.success?
     end
-    raise IllegalMove unless @gtp.success?
+    if position == "RESIGN"
+      @resigned = @current_color
+    end
+    @current_color = opposite_color
+    position
   end
 
-  def move(color, vertex = nil)
+  # Play the move and include the captured stones afterwards
+  def move(position = nil)
     raise IllegalMove if over?
-    if %w[PASS RESIGN].include? vertex
-      play(color, vertex)
-      vertex
+    if %w[PASS RESIGN].include? position
+      play(position)
     else
-      other_stones = @gtp.list_stones(opposite(color))
-      if vertex
-        play(color, vertex)
-      else
-        vertex = sgf_point(@gtp.genmove(color))
-        @resigned = color if vertex == "RESIGN"
-      end
-      captured = other_stones - @gtp.list_stones(opposite(color))
-      sgf_point(vertex) + captured.map { |v| sgf_point(v) }.join
+      other_color = opposite_color
+      other_stones = @gtp.list_stones(other_color)
+      position = play(position)
+      captured = other_stones - @gtp.list_stones(other_color)
+      position + captured.map { |v| sgf_point(v) }.join
     end
   end
 
+  def update_game_attributes_with_move(game, move = nil)
+    update = {}
+    replay(game[:moves])
+    update[:moves] = [game[:moves].to_s, move(move)].reject(&:empty?).join("-")
+    update[:last_move_at] = Time.now
+    update[:black_positions] = positions(:black)
+    update[:white_positions] = positions(:white)
+    update[:current_player_id] = game["#{@current_color}_player_id".to_sym]
+    if over?
+      update[:finished_at] = Time.now
+      update[:black_score] = score(:black)
+      update[:white_score] = score(:white)
+    else
+      update[:black_score] = captures(:black)
+      update[:white_score] = captures(:white)
+    end
+    update
+  end
+
   def positions(color)
     @gtp.list_stones(color).map { |v| sgf_point(v) }.join
   end
@@ -62,57 +90,45 @@ def captures(color)
     @gtp.captures(color)
   end
 
-  def black_score
-    score_for(:black)
-  end
-
-  def white_score
-    score_for(:white)
-  end
-
   def over?
     @resigned || @gtp.over?
   end
 
-  def first_color
-    @handicap > 0 ? :white : :black
-  end
-
-  private
-
-  def score_for(color)
+  def score(color)
     if @resigned
       @resigned.to_sym == color.to_sym ? 0 : 1
     else
       @gtp.final_score[/^#{color.to_s[0].upcase}\+([\d\.]+)$/, 1].to_f
     end
   end
 
-  def opposite(color)
-    color.to_sym == :black ? :white : :black
+  private
+
+  def opposite_color
+    @current_color == :black ? :white : :black
   end
 
-  def point(vertex)
-    args = [vertex]
-    if vertex =~ /\A[A-HJ-T](?:1\d|[1-9])\z/
+  def point(position)
+    args = [position]
+    if position =~ /\A[A-HJ-T](?:1\d|[1-9])\z/
       args << {:board_size => @board_size}
     end
     Go::GTP::Point.new(*args)
   end
 
-  def gnugo_point(vertex)
-    if %w[PASS RESIGN].include? vertex.to_s.upcase
-      vertex.to_s.upcase
+  def gnugo_point(position)
+    if %w[PASS RESIGN].include? position.to_s.upcase
+      position.to_s.upcase
     else
-      point(vertex).to_gnugo(@board_size)
+      point(position).to_gnugo(@board_size)
     end
   end
 
-  def sgf_point(vertex)
-    if %w[PASS RESIGN].include? vertex.to_s.upcase
-      vertex.to_s.upcase
+  def sgf_point(position)
+    if %w[PASS RESIGN].include? position.to_s.upcase
+      position.to_s.upcase
     else
-      point(vertex).to_sgf
+      point(position).to_sgf
     end
   end
 end

script/worker.rb

@@ -94,22 +94,9 @@ def daemonize
 job "Game.move" do |args|
   id = args["id"]
   game = mysql.query("select * from games where id='#{id}' limit 1", :symbolize_keys => true).first
-  update = {}
-  GameEngine.run(:board_size => game[:board_size].to_i, :handicap => game[:handicap].to_i, :komi => game[:komi].to_f) do |engine|
-    engine.replay(game[:moves])
-    update[:moves] = [game[:moves].to_s, engine.move(args["current_color"])].reject(&:empty?).join("-")
-    update[:last_move_at] = Time.now.utc.strftime("%Y-%m-%d %H:%M:%S")
-    update[:current_player_id] = args["next_player_id"]
-    update[:black_positions] = engine.positions(:black)
-    update[:white_positions] = engine.positions(:white)
-    if engine.over?
-      update[:finished_at] = Time.now.utc.strftime("%Y-%m-%d %H:%M:%S")
-      update[:black_score] = engine.black_score
-      update[:white_score] = engine.white_score
-    else
-      update[:black_score] = engine.captures(:black)
-      update[:white_score] = engine.captures(:white)
-    end
+  update = GameEngine.update_game_attributes_with_move(game)
+  update.each do |name, value|
+    update[name] = value.utc.strftime("%Y-%m-%d %H:%M:%S") if value.kind_of? Time
   end
   values = update.map { |col, val| "#{col}='#{mysql.escape(val.to_s)}'" }.join(", ")
   mysql.query("UPDATE games SET #{values} WHERE id=#{id} AND current_player_id IS NULL AND finished_at IS NULL")