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

エモクロア 文字列”成功数を追加 #505

Merged
merged 1 commit into from
Oct 16, 2021

Conversation

rassi0429
Copy link
Contributor

  • 判定値の結果に成功数の表記を追加しました。

@codecov
Copy link

codecov bot commented Oct 8, 2021

Codecov Report

Merging #505 (29abfa8) into master (a6ab6a9) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #505   +/-   ##
=======================================
  Coverage   95.43%   95.43%           
=======================================
  Files         310      310           
  Lines       18236    18236           
=======================================
  Hits        17403    17403           
  Misses        833      833           
Impacted Files Coverage Δ
lib/bcdice/game_system/Emoklore.rb 95.00% <100.00%> (ø)

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 a6ab6a9...29abfa8. Read the comment docs.

@GenKuzumochi
Copy link
Contributor

(1DM<=5) > [5] > 1 > 成功数1 成功!
この形にするなら、真ん中の> 1 >は冗長に思えます。略していいのではないでしょうか。
(1DM<=5) > [5] > 成功数1 成功!

(このPRには無関係ですが、成功数1は「シングル!」ではなく「成功!」と表示されるんですね。ルルブに合わせたほうが自然かも……)

Copy link
Member

@ysakasin ysakasin left a comment

Choose a reason for hiding this comment

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

直接話を聞いたのですが、エモクロアでは成功数に依存する処理をすることがあり、多くのオンセツールで結果概要に 区切りの最後の部分が表示されるのを利用したいということですね。たしかに成功数が重複表示されていますが、値と結果が十分に分離されていると思いますので、このままで大丈夫だと思います。

@ysakasin ysakasin merged commit 0ab5b50 into bcdice:master Oct 16, 2021
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.

3 participants