クトゥルフテック：リファクタリング #165
Merged
クトゥルフテック：リファクタリング #165
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* シークレットロール * 修正値あり * ファンブル
特に DiceBot#changeDiceValueByDiceText を使わないようにするのが目的。
クトゥルフテックのリファクタリングによって、不要になったため。
DiceBot#changeDiceValueByDiceText は削除された。
ochaochaocha3
force-pushed
the
cthulhu_tech-refactor
branch
from
f72c39d to
52b26e6
Compare
ysakasin
requested changes
Apr 27, 2020
There was a problem hiding this comment.
1. CthulhuTech::Compare
CthulhuTech::Test を現在の CthulhuTech::Compare 相当のクラスにし、 class Contest < Test とするのはどうでしょう。 CthulhuTech::Compare と CthulhuTech::Test の差分があまりにもないのと、Compareという名前は正直しっくりきません。構造は変えない場合、少なくともクラス名は変更して欲しいです。 私の思いつく限りでは、Nodeが適切かと思います。
2. CthulhuTech#execute
これは全てCthulhuTech::TestおよびCthulhuTech::Compareの内容から算出しているので、CthulhuTechのメソッドではなくCthulhuTech::Testのメソッドにした方が良いと思いました。
|
あぁ |
|
@ysakasin どちらも作っている途中で迷ったところでした。1については、そのようにしたいと思います。2については、dicebotを渡せばできそうかな、と考えていたところだったので、試してみます。 |
|
判定処理をひとつのクラスにまとめました。ダイスロールは、判定ノードの |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
クトゥルフテックのダイスボットをリファクタリングしました。
主な目的は、「クトゥルフテック専用」と書かれていた
DiceBot#changeDiceValueByDiceTextを使わないようにすることです。クトゥルフテックのコマンドには、動作が加算ロールとは大きく異なる
nD10があります。従来は、これを無理やり
dice/AddDice.rb内の加算ロールの処理で扱おうとしていたため、DiceBot#changeDiceValueByDiceTextのような不自然なメソッドが必要になっていました。今回、
nD10を加算ロールではなくダイスボット固有のコマンドとして処理するように変えたため、ダイスボット外にある上記の不自然な処理が不要となり、消すことができました。そのほかに、対抗判定成功時に表示されるダメージロールのコマンドの意味が分かりやすくなるように、出力を少し変えました。
例：