Adding more validation.

lib/mjai/action.rb

@@ -33,6 +33,7 @@ class Action < JSONizable
           [:deltas, :numbers],
           [:scores, :numbers],
           [:text, :string],
+          [:message, :string],
         ])
 
     end

lib/mjai/active_game.rb

@@ -1,6 +1,7 @@
 require "mjai/game"
 require "mjai/action"
 require "mjai/hora"
+require "mjai/validation_error"
 
 
 module Mjai
@@ -15,14 +16,20 @@ def initialize(players)
         attr_accessor(:game_type)
 
         def play()
-          do_action({:type => :start_game, :names => self.players.map(){ |pl| pl.name }})
-          @ag_oya = @ag_chicha = @players[0]
-          @ag_bakaze = Pai.new("E")
-          @ag_honba = 0
-          while !self.game_finished?
-            play_kyoku()
+          begin
+            do_action({:type => :start_game, :names => self.players.map(){ |pl| pl.name }})
+            @ag_oya = @ag_chicha = @players[0]
+            @ag_bakaze = Pai.new("E")
+            @ag_honba = 0
+            while !self.game_finished?
+              play_kyoku()
+            end
+            do_action({:type => :end_game})
+            return true
+          rescue ValidationError => ex
+            do_action({:type => :error, :message => ex.message})
+            return false
           end
-          do_action({:type => :end_game})
         end
 
         def play_kyoku()

lib/mjai/game.rb

@@ -1,6 +1,7 @@
 require "mjai/action"
 require "mjai/pai"
 require "mjai/furo"
+require "mjai/validation_error"
 
 
 module Mjai
@@ -66,9 +67,9 @@ def do_action(action)
           responses = (0...4).map() do |i|
             @players[i].respond_to_action(action_in_view(action, i))
           end
-          validate_responses(responses, action)
-
           @previous_action = action
+
+          validate_responses(responses, action)
           return responses
 
         end
@@ -131,17 +132,27 @@ def action_in_view(action, player_id)
         def validate_responses(responses, action)
           for i in 0...4
             response = responses[i]
-            raise("invalid actor") if response && response.actor != @players[i]
-            validate_response_type(response, @players[i], action)
-            validate_response_content(response, action) if response
+            begin
+              if response && response.actor != @players[i]
+                raise(ValidationError, "Invalid actor.")
+              end
+              validate_response_type(response, @players[i], action)
+              validate_response_content(response, action) if response
+            rescue ValidationError => ex
+              raise(ValidationError,
+                  "Error in player %d's response: %s Response: %s" % [i, ex.message, response])
+            end
           end
         end
 
         def validate_response_type(response, player, action)
+          if response && response.type == :error
+            raise(ValidationError, response.message)
+          end
           is_actor = player == action.actor
           if expect_response_from?(player)
             case action.type
-              when :start_game, :start_kyoku, :haipai, :end_kyoku, :end_game,
+              when :start_game, :start_kyoku, :end_kyoku, :end_game, :error,
                   :hora, :ryukyoku, :dora, :reach_accepted
                 valid = !response
               when :tsumo
@@ -176,45 +187,94 @@ def validate_response_type(response, player, action)
               when :log
                 valid = !response
               else
-                raise("unknown action type: #{action.type}")
+                raise(ValidationError, "Unknown action type: '#{action.type}'")
             end
           else
             valid = !response
           end
-          raise("bad response %p for %p" % [response, action]) if !valid
+          if !valid
+            raise(ValidationError,
+                "Unexpected response type '%s' for %s." % [response ? response.type : :none, action])
+          end
         end
 
         def validate_response_content(response, action)
+
           case response.type
+
             when :dahai
-              assert_fields_exist(response, [:pai, :tsumogiri])
-              if !response.actor.possible_dahais.include?(response.pai)
-                raise("dahai not allowed: %p" % response)
-              end
+              validate_fields_exist(response, [:pai, :tsumogiri])
+              validate(
+                  response.actor.possible_dahais.include?(response.pai),
+                  "Cannot dahai this pai.")
               if [:tsumo, :reach].include?(action.type)
                 if response.tsumogiri
                   tsumo_pai = response.actor.tehais[-1]
-                  if response.pai != tsumo_pai
-                    raise("tsumogiri is true but the pai is not tsumo pai: %s != %s" %
-                        [response.pai, tsumo_pai])
-                  end
+                  validate(
+                      response.pai == tsumo_pai,
+                      "tsumogiri is true but the pai is not tsumo pai: %s != %s" %
+                      [response.pai, tsumo_pai])
                 else
-                  if !response.actor.tehais[0...-1].include?(response.pai)
-                    raise("tsumogiri is false but the pai is not in tehais")
-                  end
+                  validate(
+                      response.actor.tehais[0...-1].include?(response.pai),
+                      "tsumogiri is false but the pai is not in tehais.")
                 end
               else  # after furo
-                if response.tsumogiri
-                  raise("tsumogiri must be false on dahai after furo")
-                end
+                validate(
+                    !response.tsumogiri,
+                    "tsumogiri must be false on dahai after furo.")
+              end
+
+            when :chi, :pon, :daiminkan, :ankan, :kakan
+              if response.type == :ankan
+                validate_fields_exist(response, [:consumed])
+              elsif response.type == :kakan
+                validate_fields_exist(response, [:pai, :consumed])
+              else
+                validate_fields_exist(response, [:target, :pai, :consumed])
+                validate(
+                    response.target == action.actor,
+                    "target must be %d." % action.actor.id)
+              end
+              valid = response.actor.possible_furo_actions.any?() do |a|
+                a.type == response.type &&
+                    a.pai == response.pai &&
+                    a.consumed.sort() == response.consumed.sort()
               end
+              validate(valid, "The furo is not allowed.")
+
+            when :reach
+              validate(response.actor.can_reach?, "Cannot reach.")
+
+            when :hora
+              validate_fields_exist(response, [:target, :pai])
+              validate(
+                  response.target == action.actor,
+                  "target must be %d." % action.actor.id)
+              if response.target == response.actor
+                tsumo_pai = response.actor.tehais[-1]
+                validate(
+                    response.pai == tsumo_pai,
+                    "pai is not tsumo pai: %s != %s" % [response.pai, tsumo_pai])
+              else
+                validate(
+                    response.pai == action.pai,
+                    "pai is not previous dahai: %s != %s" % [response.pai, action.pai])
+              end
+              validate(response.actor.can_hora?, "Cannot hora.")
+
           end
+
+        end
+
+        def validate(criterion, message)
+          raise(ValidationError, message) if !criterion
         end
 
-        def assert_fields_exist(response, field_names)
+        def validate_fields_exist(response, field_names)
           for name in field_names
             if !response.fields.has_key?(name)
-              raise("%s missing" % name)
+              raise(ValidationError, "%s missing." % name)
             end
           end
         end

lib/mjai/jsonizable.rb

@@ -19,33 +19,87 @@ def self.define_fields(specs)
 
         def self.from_json(json, game)
           hash = JSON.parse(json)
-          fields = {}
-          for name, type in @@field_specs
-            plain = hash[name.to_s()]
-            next if plain == nil
-            case type
-              when :symbol
-                obj = plain.intern()
-              when :symbols
-                obj = plain.map(){ |s| s.intern() }
-              when :player
-                obj = game.players[plain]
-              when :pai
-                obj = Pai.new(plain)
-              when :pais
-                obj = plain.map(){ |s| Pai.new(s) }
-              when :pais_list
-                obj = plain.map(){ |o| o.map(){ |s| Pai.new(s) } }
-              when :yakus
-                obj = plain.map(){ |s, n| [s.intern(), n] }
-              when :number, :numbers, :string, :strings, :boolean, :booleans
-                obj = plain
-              else
-                raise("unknown type")
+          begin
+            validate(hash.is_a?(Hash), "The response must be an object.")
+            fields = {}
+            for name, type in @@field_specs
+              plain = hash[name.to_s()]
+              next if plain == nil
+              fields[name] = plain_to_obj(plain, type, name.to_s(), game)
             end
-            fields[name] = obj
+            return new(fields)
+          rescue ValidationError => ex
+            raise(ValidationError, "%s JSON: %s" % [ex.message, json])
           end
-          return new(fields)
+        end
+
+        def self.plain_to_obj(plain, type, name, game)
+          case type
+            when :number
+              validate_class(plain, Integer, name)
+              return plain
+            when :string
+              validate_class(plain, String, name)
+              return plain
+            when :boolean
+              validate(
+                  plain.is_a?(TrueClass) || plain.is_a?(FalseClass),
+                  "#{name} must be either true or false.")
+              return plain
+            when :symbol
+              validate_class(plain, String, name)
+              validate(!plain.empty?, "#{name} must not be empty.")
+              return plain.intern()
+            when :player
+              validate_class(plain, Integer, name)
+              validate((0...4).include?(plain), "#{name} must be either 0, 1, 2 or 3.")
+              return game.players[plain]
+            when :pai
+              validate_class(plain, String, name)
+              begin
+                return Pai.new(plain)
+              rescue ArgumentError => ex
+                raise(ValidationError, "Error in %s: %s" % [name, ex.message])
+              end
+            when :yaku
+              validate_class(plain, Array, name)
+              validate(
+                  plain.size == 2 && plain[0].is_a?(String) && plain[1].is_a?(Integer),
+                  "#{name} must be an array of [String, Integer].")
+              validate(!plain[0].empty?, "#{name}[0] must not be empty.")
+              return [plain[0].intern(), plain[1]]
+            when :numbers
+              return plains_to_objs(plain, :number, name, game)
+            when :strings
+              return plains_to_objs(plain, :string, name, game)
+            when :booleans
+              return plains_to_objs(plain, :boolean, name, game)
+            when :symbols
+              return plains_to_objs(plain, :symbol, name, game)
+            when :pais
+              return plains_to_objs(plain, :pai, name, game)
+            when :pais_list
+              return plains_to_objs(plain, :pais, name, game)
+            when :yakus
+              return plains_to_objs(plain, :yaku, name, game)
+            else
+              raise("unknown type")
+          end
+        end
+
+        def self.plains_to_objs(plains, type, name, game)
+          validate_class(plains, Array, name)
+          return plains.map().with_index() do |c, i|
+            plain_to_obj(c, type, "#{name}[#{i}]", game)
+          end
+        end
+
+        def self.validate(criterion, message)
+          raise(ValidationError, message) if !criterion
+        end
+
+        def self.validate_class(plain, klass, name)
+          validate(plain.is_a?(klass), "%s must be %p." % [name, klass])
         end
 
         def initialize(fields)

lib/mjai/pai.rb

@@ -40,7 +40,7 @@ def initialize(*args)
               if str == "?"
                 @type = @number = nil
                 @red = false
-              elsif str =~ /^([1-9])([mps])(r)?$/
+              elsif str =~ /\A([1-9])([mps])(r)?\z/
                 @type = $2
                 @number = $1.to_i()
                 @red = $3 != nil
@@ -49,13 +49,13 @@ def initialize(*args)
                 @number = number
                 @red = false
               else
-                raise(ArgumentError, "unknown pai string: %s" % str)
+                raise(ArgumentError, "Unknown pai string: %s" % str)
               end
             when 2, 3
               (@type, @number, @red) = args
               @red = false if @red == nil
             else
-              raise(ArgumentError, "wrong number of args")
+              raise(ArgumentError, "Wrong number of args.")
           end
         end