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

[StellarKnights] 【アタック判定】のダイス数に 0 が指定された場合を考慮 #455

Merged

Conversation

ViVi-shark
Copy link
Contributor

内容

 【アタック判定】のダイス数に 0 が指定された場合、メッセージでその旨を表示する。

背景

 今までは結果文字列が "(0SK) > " のようになって、不自然かつ不親切だったので。

従来は結果文字列が `"(0SK) > "` のようになって、不自然かつ不親切だったので。
@codecov
Copy link

codecov bot commented May 3, 2021

Codecov Report

Merging #455 (7ca23e2) into master (d5dffa9) will increase coverage by 0.01%.
The diff coverage is 99.21%.

❗ Current head 7ca23e2 differs from pull request most recent head 8f30a3b. Consider uploading reports for the commit 8f30a3b to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #455      +/-   ##
==========================================
+ Coverage   95.24%   95.26%   +0.01%     
==========================================
  Files         288      289       +1     
  Lines       18679    18760      +81     
==========================================
+ Hits        17791    17871      +80     
- Misses        888      889       +1     
Impacted Files Coverage Δ
lib/bcdice/game_system/BattleTech.rb 98.95% <97.50%> (-0.45%) ⬇️
lib/bcdice/game_system.rb 100.00% <100.00%> (ø)
lib/bcdice/game_system/Satasupe.rb 98.02% <100.00%> (ø)
lib/bcdice/game_system/StarryDolls.rb 100.00% <100.00%> (ø)
lib/bcdice/game_system/SteamPunkers.rb 95.12% <100.00%> (+0.38%) ⬆️
lib/bcdice/game_system/StellarKnights.rb 98.76% <100.00%> (+0.03%) ⬆️

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 d5dffa9...8f30a3b. Read the comment docs.

@ysakasin
Copy link
Member

ysakasin commented May 4, 2021

ありがとうございます。

エラーメッセージの内容は「ダイス数は1以上としてください」と正常な範囲を示す方が良さそうだと感じましたがどうでしょうか。例えば、新クトゥルフ神話TRPGのボーナスダイスではそのように表示しています。
https://github.com/bcdice/BCDice/blob/master/lib/bcdice/game_system/Cthulhu7th.rb#L209-L211

また、ダイス数が0である場合には成功数を表示する必要はないと思っています。
(0SK4,1>6) > ダイス数は1以上としてください と表示するのが良いだろうと感じましたが、いかがでしょう?

@ViVi-shark
Copy link
Contributor Author

エラーメッセージの内容は「ダイス数は1以上としてください」と正常な範囲を示す方が良さそうだと感じましたがどうでしょうか。例えば、新クトゥルフ神話TRPGのボーナスダイスではそのように表示しています。
https://github.com/bcdice/BCDice/blob/master/lib/bcdice/game_system/Cthulhu7th.rb#L209-L211

 「新クトゥルフ神話TRPGのボーナスダイス」の例は「プログラムとしての便宜上の制限」と見受けられ、本件の「【アタック判定】のダイスが0個」は「現実のゲームプレイで発生する処理の一種」にすぎないので、メッセージの性格がことなるとおもっています。
 前者で「想定されている範囲に収めてください」というのは妥当だとおもいますが、後者は(ユーザーはゲームプレイ上の合理的経緯によって0を入力しているはずなので)「1以上にして入力し直すことをうながす」のはユーザー心理に反するとかんがえます。

@ViVi-shark
Copy link
Contributor Author

また、ダイス数が0である場合には成功数を表示する必要はないと思っています。

 これについては、Q&Aによると、

【Q.】スキルなどの効果でアタック判定のダイス数が0となった場合、「タイミング:アタック判定直後」のスキルを使用できますか?(19/11/28追加)
【A.】アタック判定のダイスが0となった場合、判定自体が行われません。

 らしいので、たしかに成功数を導出するべきではありませんでした。のちほどそのようにしておきます。

@ysakasin
Copy link
Member

ysakasin commented May 8, 2021

ユーザーはゲームプレイ上の合理的経緯によって0を入力しているはずなので

わかりました。でしたら ダイスが 0 個のためダイスロールされませんでした のように、ダイスロールが実行されなかったことを明示した方が良いかもしれないと思いましたが、どうでしょう?

@ViVi-shark
Copy link
Contributor Author

でしたら ダイスが 0 個のためダイスロールされませんでした のように、ダイスロールが実行されなかったことを明示した方が良いかもしれないと思いましたが、どうでしょう?

 たしかにユーザーにとって不親切だったので、メッセージを変更しました。

 上( #455 (comment) )で述べた「ダイスが0個ならアタック判定が行われない」ルールにもとづき、その旨を記述しました。 7d3b756

@ysakasin ysakasin merged commit ff0cc55 into bcdice:master May 12, 2021
@ysakasin
Copy link
Member

ありがとうございます! こちらで少しだけ手直ししてマージしました

@ViVi-shark ViVi-shark deleted the features/stellarknights-support_zero_dice branch May 18, 2021 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants