Skip to content
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

カオスフレアの専用コマンドを追加 #213

Merged
merged 3 commits into from
Jun 12, 2020

Conversation

Nokiyen
Copy link
Contributor

@Nokiyen Nokiyen commented Jun 5, 2020

こんにちは。いつもBCDiceをありがたく使わせていただいております。
今回カオスフレアのDicebotに、処理を追加したため、レビューしていただきたく思います。

①カオスフレアの判定専用コマンドを追加しました。
今までは2d6についてのみ処理が書かれておりましたが、種々のケースに対応した専用の判定コマンド"CF"を追加しました。2d6の処理は、特に触らずにそのまま残してあります。
カオスフレアの判定は(ご存知かもしれませんが)、基本2d6で、クリティカルだと30扱い、ファンブルだと-20扱いになります。また、対抗判定で差分値を求めることがあります。
そのため、クリティカル値、ファンブル値を指定でき、かつ差分値を計算するコマンドとして実装しました。(あと修正値も加減算のみできます。)

②因縁表を振るコマンドを追加しました。
他のルールで言うところの感情表に値するものを、ランダムに触れるコマンドを用意しました。

普段Rubyは使用しないため、実装に問題のある個所や、リファクタリングの必要な個所があればご教示いただけますと幸いです。
お忙しい中恐縮ですが、よろしくお願いいたします。

@codecov-commenter
Copy link

codecov-commenter commented Jun 5, 2020

Codecov Report

Merging #213 into master will increase coverage by 0.01%.
The diff coverage is 92.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #213      +/-   ##
==========================================
+ Coverage   87.26%   87.28%   +0.01%     
==========================================
  Files         216      216              
  Lines       22410    22479      +69     
==========================================
+ Hits        19556    19620      +64     
- Misses       2854     2859       +5     
Impacted Files Coverage Δ
src/utils/command_parser.rb 96.38% <88.23%> (-2.13%) ⬇️
src/diceBot/ChaosFlare.rb 92.10% <94.33%> (+5.14%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d6fa8b1...f861b57. Read the comment docs.

@ysakasin
Copy link
Member

ysakasin commented Jun 5, 2020

@Nokiyen 寄稿ありがとうございます! 十分に書けていると思います。

CFの書式ですが、他のシステムのコマンドとの共通化のために以下のどちらかにしたいのですが、どうですか。

  • [ダイスの数]CF[@クリティカル値][#ファンブル値][修正値][>=目標値]
  • [ダイスの数]CF[@クリティカル値][#ファンブル値][修正値][@クリティカル値][#ファンブル値][>=目標値]

@ysakasin
Copy link
Member

ysakasin commented Jun 5, 2020

どちらを採用するにしろ、最近作ったモジュールを使ってコマンドの書式を分解するので、以後の修正はこちらでさせていただきます。
#207

@Nokiyen
Copy link
Contributor Author

Nokiyen commented Jun 5, 2020

ご確認ありがとうございます! 書式としては、2番目の方が、クリティカル値とファンブル値を前にも後ろにもかける、ということでしょうか?
選択肢は多い方が良いかと思いますので、2番目が好ましいように感じます。

修正は行っていただけるとのことで、ありがとうございます。(コマンドの書式分解とは、すごい良いですね……。)
よろしくお願い致します。

@ysakasin
Copy link
Member

ysakasin commented Jun 5, 2020

了解です。CFの後ろか修正値の後ろにクリティカルファンブルを書けるようにします

@ysakasin ysakasin changed the title カオスフレアのdicebotに処理を追加しました。 カオスフレアの専用コマンドを追加 Jun 6, 2020
@ysakasin ysakasin added the new dicebot 新システムの対応 label Jun 8, 2020
@ysakasin
Copy link
Member

@Nokiyen FTコマンドの数字ありのときに、どのような挙動にしたいのかがコメントから分からなかったのですが、教えていただけますか?

@Nokiyen
Copy link
Contributor Author

Nokiyen commented Jun 11, 2020

大変失礼しました……。

FT [値]で、D66表の数字を直接指定して、結果を得ることを想定していました。例えば11なら「純愛」と返す、などです。D66表の中に66で選べない値として、「0」の「腐れ縁」と「7」の「任意」が設定されているため、何らかの手段でこれらをコマンドから取得できればと思っていました。

ただ、実際に値付きでFTコマンドを打つ機会はかなり少ない (気分で数字を選んで結果を得たいとき? あるいはGMのギミック)と思われますので、実装から削除いただいても良いかと思います。

ゲームシステム上、FTコマンドに値が必要なわけではありません。

@ysakasin
Copy link
Member

なるほど、そうしたら以下のような処理で問題ないですか?

FT0 -> 腐れ縁
FT66 -> 利用
FT7 -> 任意

@Nokiyen
Copy link
Contributor Author

Nokiyen commented Jun 11, 2020

そちらの処理で、意図通りとなります。ありがとうございます。

Copy link
Member

@ochaochaocha3 ochaochaocha3 left a comment

Choose a reason for hiding this comment

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

大丈夫だと思います!

@ysakasin
Copy link
Member

@Nokiyen 他のダイスボットと表記を合わせるため、出力の形式を変更しました。これで問題ないか確認お願いします。

@Nokiyen
Copy link
Contributor Author

Nokiyen commented Jun 12, 2020

確認させていただきました。シークレットダイスまで対応していただきありがとうございます。こちらで問題ないかと思います。

@ysakasin ysakasin merged commit 80559e0 into bcdice:master Jun 12, 2020
@ysakasin
Copy link
Member

@Nokiyen マージしました! 寄稿ありがとうございます!

@Nokiyen
Copy link
Contributor Author

Nokiyen commented Jun 13, 2020

マージありがとうございます。こちらこそ、ご丁寧に対応・修正いただきありがとうございました!

ysakasin added a commit that referenced this pull request Aug 30, 2020
カオスフレアの専用コマンドを追加
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new dicebot 新システムの対応
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants