BCDice icon indicating copy to clipboard operation
BCDice copied to clipboard

[SwordWorld2.0] レーティング表部分の解析結果のリファクタリング

Open ochaochaocha3 opened this issue 3 years ago • 0 comments

ソード・ワールド2.0、2.5のレーティング表部分の解析結果(rating_parsed.rb)の冗長な部分を簡潔にする。

現状

rating_parsed.rbでは、各インスタンス変数を nil で初期化しており、またそれらを正規化するメソッド(nil ならば 0 等の既定値に変換したり、範囲内に収めたりするメソッド)を用意している。

https://github.com/bcdice/BCDice/blob/c833464bf4e9db8d35b4adc170a9f662a762842b/lib/bcdice/game_system/sword_world/rating_parsed.rb#L34-L41

正規化メソッドの例:

https://github.com/bcdice/BCDice/blob/c833464bf4e9db8d35b4adc170a9f662a762842b/lib/bcdice/game_system/sword_world/rating_parsed.rb#L55-L58

以下に示すように、これらはうまく使われていない。

rating_parsed.rbの #to_s には、必要なパーツを文字列に追加する処理が書かれているが、必要性の判断は nil との比較ではなく、0 等の既定値との比較で行われている。また、条件判定と式展開で正規化メソッドが2回呼び出されているため、インスタンス変数と既定値の比較が最大で3回行われている。

https://github.com/bcdice/BCDice/blob/c833464bf4e9db8d35b4adc170a9f662a762842b/lib/bcdice/game_system/sword_world/rating_parsed.rb#L81-L95

レーティング表の処理はSwordWorld.rbに書かれているが、ここでも必要性の判断は nil との比較ではなく、0 等の既定値との比較で行われている。

例1:

https://github.com/bcdice/BCDice/blob/c833464bf4e9db8d35b4adc170a9f662a762842b/lib/bcdice/game_system/SwordWorld.rb#L82-L95

例2:

https://github.com/bcdice/BCDice/blob/c833464bf4e9db8d35b4adc170a9f662a762842b/lib/bcdice/game_system/SwordWorld.rb#L325-L341

以上の例のように、各インスタンス変数の nil は使われておらず、0 等の既定値を直接設定すればよくなっている。

おそらく、構文解析器の #parsed にある、値をまとめて設定する処理との整合性をとるために、このような形になったのではないだろうか。

https://github.com/bcdice/BCDice/blob/ca56617908046f8432479f194e2f995b64ba7988/lib/bcdice/game_system/sword_world/rating_parser.y#L215-L229

変更案

RatingParsedの各インスタンス変数を原則として 0 等の既定値で初期化するようにする。そうすれば、正規化メソッドは不要になり、直接インスタンス変数を参照すればよくなる。nil での初期化のままにし、SwordWorld.rb側も含めて処理を書き換えることも一案だが、範囲内に収める等の処理もあるため、数値で表現しておく方が問題が生じにくいと思われる。

class RatingParsed
  def initialize
    @critical = nil
    @kept_modify = 0
    @first_to = 0
    @first_modify = 0
    @greatest_fortune = false
    @rateup = 0
  end

  # @return [Boolean]
  def half
    @modifier_after_half != 0
  end

  # @return [String]
  def to_s()
    sequence = ["KeyNo.#{@rate}"]

    sequence << "c[#{@critical}]" if @critical < 13
    sequence << "m[#{Format.modifier(@first_modify)}]" if @first_modify != 0
    sequence << "m[#{@first_to}]" if @first_to != 0
    sequence << "r[#{@rateup}]" if @rateup != 0
    sequence << "gf" if @greatest_fortune
    sequence << "a[#{Format.modifier(@kept_modify)}]" if @kept_modify != 0

    sequence << Format.modifier(@modifier) if @modifier != 0

    return sequence.join
  end
end

RatingParsedに値を範囲内に収める正規化処理のメソッドを用意する。これは構文解析後に1回行えばよい。

# rating_parsed.rb
class RatingParsed
  def normalize!
    # クリティカル値を3以上に設定する
    @critical = (@critical || (half ? 13 : 10)).clamp(3..)
    # ...

    self
  end
end

# SwordWorld.rb
class SwordWorld < Base
  def rating(string) # レーティング表
    command = rating_parser.parse(string).normalize!
    # ...
  end
end

構文解析器では、#parsed において、オプションが設定されたかを if で判定するように変更する必要があるが、構文解析中の判定にも重複箇所が多くあるため、専用のBuilderを作るのがよいかもしれない。#515 の変更にも合わせる。

ochaochaocha3 avatar Nov 24 '21 19:11 ochaochaocha3