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

『アルケミア・ストラグル』のダイスボット AlchemiaStruggle を追加 #341

Merged
merged 23 commits into from
Dec 26, 2020

Conversation

ViVi-shark
Copy link
Contributor

@ViVi-shark ViVi-shark commented Dec 25, 2020

ゲームについて

 Role&Roll のサポートがまだ存在しないのですが、上記の新紀元社のページにあるとおり、 2020/12/24 に発行されたタイトルです。

コマンド一覧(コード内から転載)

        ■ ダイスロール( XAS )
          XDをロールします。
          例) 5AS
        
        ■ ダイスロール&最大になるようにピック( XASY )
          XDをロールし、そこから最大になるようにY個をピックします。
          例) 4AS3

        ■ 表
          ・奇跡の触媒
            ・エレメント (CELE, CElement)
            ・アルケミア (CALC, CAlchemia)
            ・インフォーマント (CINF, CInformant)
            ・イノセンス (CINN, CInnocence)
            ・アクワイヤード (CACQ, CAcquired)
          ・携行品
            ・Sサイズ (ARS, ArticleS)
            ・Mサイズ (ARM, ArticleM)
            ・Lサイズ (ARL, ArticleL)
          ・PC情報獲得表 (PCI, PCInformation)
          ・理由表 (REA, Reason)
          ・交流表 (ASS, Associate)
          ・接触のきっかけ表 (CON, Contact)

ダイスロール機能

 現時点ではそこまで意味のある(=標準のコマンドと差異のある)機能ではありませんが、タイトル独自性の高い機能が追加されたときに、エンドユーザーにとってのコマンド体型が現在と一貫しなくなるようでは困ると考えて、独自ロールコマンドを用意してあります。

 よくある表です。
 「D66 の左側を偶奇しか見ない」表がいくつかあります。

@codecov
Copy link

codecov bot commented Dec 25, 2020

Codecov Report

Merging #341 (224d1c2) into master (2b48bdc) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #341      +/-   ##
==========================================
+ Coverage   94.41%   94.43%   +0.01%     
==========================================
  Files         260      262       +2     
  Lines       18073    18047      -26     
==========================================
- Hits        17063    17042      -21     
+ Misses       1010     1005       -5     
Impacted Files Coverage Δ
lib/bcdice/dice_table.rb 100.00% <100.00%> (ø)
lib/bcdice/dice_table/d66_parity_table.rb 100.00% <100.00%> (ø)
lib/bcdice/game_system.rb 100.00% <100.00%> (ø)
lib/bcdice/game_system/AlchemiaStruggle.rb 100.00% <100.00%> (ø)
lib/bcdice/game_system/KanColle.rb 87.65% <0.00%> (-0.90%) ⬇️
lib/bcdice/game_system/KillDeathBusiness.rb 96.49% <0.00%> (-0.25%) ⬇️
lib/bcdice/common_command/add_dice/parser.rb 97.16% <0.00%> (-0.02%) ⬇️
lib/bcdice/preprocessor.rb 100.00% <0.00%> (ø)
lib/bcdice/game_system/CardRanker.rb 100.00% <0.00%> (ø)
lib/bcdice/game_system/ShinobiGami.rb 100.00% <0.00%> (ø)
... and 9 more

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 2b48bdc...224d1c2. Read the comment docs.

@ViVi-shark
Copy link
Contributor Author

ちょっと不備を見つけたので

@ViVi-shark ViVi-shark closed this Dec 25, 2020
@ysakasin
Copy link
Member

ysakasin commented Dec 25, 2020

@ViVi-shark 不備修正のコミットは同じブランチにpushしてください。自動的にこのPRに反映されます。

@ysakasin ysakasin reopened this Dec 25, 2020
@ysakasin ysakasin changed the title 『アルケミア・ストラグル』のダイスボット AlchemiaStruggle を追加 [WIP]『アルケミア・ストラグル』のダイスボット AlchemiaStruggle を追加 Dec 25, 2020
@ViVi-shark ViVi-shark force-pushed the features/alchemia_struggle branch from e1f51cc to 9907930 Compare December 25, 2020 03:59
@ViVi-shark
Copy link
Contributor Author

@ysakasin 修正しました

@ysakasin ysakasin self-requested a review December 25, 2020 05:10
@ysakasin ysakasin changed the title [WIP]『アルケミア・ストラグル』のダイスボット AlchemiaStruggle を追加 『アルケミア・ストラグル』のダイスボット AlchemiaStruggle を追加 Dec 25, 2020
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.

寄稿ありがとうございます!
気になる点を指摘しましたので、確認/修正お願いします。

lib/bcdice/game_system/AlchemiaStruggle.rb Outdated Show resolved Hide resolved
lib/bcdice/game_system/AlchemiaStruggle.rb Outdated Show resolved Hide resolved
lib/bcdice/game_system/AlchemiaStruggle.rb Outdated Show resolved Hide resolved
lib/bcdice/game_system/AlchemiaStruggle.rb Outdated Show resolved Hide resolved
lib/bcdice/game_system/AlchemiaStruggle.rb Outdated Show resolved Hide resolved
lib/bcdice/game_system/AlchemiaStruggle.rb Outdated Show resolved Hide resolved
lib/bcdice/game_system/AlchemiaStruggle.rb Outdated Show resolved Hide resolved
lib/bcdice/game_system/AlchemiaStruggle.rb Outdated Show resolved Hide resolved
lib/bcdice/game_system/AlchemiaStruggle.rb Outdated Show resolved Hide resolved
lib/bcdice/game_system/AlchemiaStruggle.rb Outdated Show resolved Hide resolved
@ViVi-shark ViVi-shark force-pushed the features/alchemia_struggle branch from a6668a7 to 4dcde02 Compare December 25, 2020 07:19
Co-authored-by: SAKATA Sinji <ysakasin@gmail.com>
@ViVi-shark ViVi-shark force-pushed the features/alchemia_struggle branch from 4dcde02 to baffdd9 Compare December 25, 2020 07:27
ViVi and others added 6 commits December 25, 2020 16:27
…try_roll_alchemia`

Co-authored-by: SAKATA Sinji <ysakasin@gmail.com>
 - 変数部分を小文字表記に
 - 英数を半角に

ルールブック内で `XD`, `YD` などと書かれているのでそれを踏襲した表記だったが、現実問題として判読しづらいのは事実なので。

Co-authored-by: SAKATA Sinji <ysakasin@gmail.com>
@ViVi-shark ViVi-shark force-pushed the features/alchemia_struggle branch from f37a2f5 to 1292451 Compare December 25, 2020 07:44
@ysakasin
Copy link
Member

思ったいたのよりだいぶ機能が増えたD66クラスが作成されましたが、奇遇判定を上も下も両対応するのは需要があるんでしょうか? 少なくとも寄稿してもらった範囲では最初のダイス1つを奇遇判定できればいい気がします。

両対応だとデータ構造が記述しづらかったり思いの外複雑になるので、できるだけシンプルに保ちたいです。

@ViVi-shark ViVi-shark force-pushed the features/alchemia_struggle branch from f2bb4c0 to 561ed8d Compare December 26, 2020 04:37
@ViVi-shark
Copy link
Contributor Author

少なくとも寄稿してもらった範囲では最初のダイス1つを奇遇判定できればいい気がします。

 これはそのとおりです。

データ構造が記述しづらかったり

 これは Hash でも Array でも受けられるようにすればよかったかと思いつつ、

 それはそれとして、左ダイスと右ダイスでキーの解決方法を変えるほうが(コード上・クラス仕様として)複雑になるという考えであのようにしたのですが、 YAGNI 的に今回のダイスボットで必要なふるまいだけに変更しました。 a411f65

@ysakasin
Copy link
Member

左ダイスと右ダイスでキーの解決方法を変えるほうが(コード上・クラス仕様として)複雑になる

おっしゃる通りだと思います。その代償として、表の煩雑になるというトレードオフがありそうな気がします。

加えて質問ですが、コンストラクタを def initialize(name, odd:, even:) とせずに def initialize(name, items)とHashを受け取るようにしているのは何か思惑があるのでしょうか?

oddとevenは必要不可欠な要素なので、コンストラクタの引数として要求した方が、コンストラクタの引数情報から表作成に必要な情報が分かり、良いのではないかと感じました。

@ViVi-shark
Copy link
Contributor Author

加えて質問ですが、コンストラクタを def initialize(name, odd:, even:) とせずに def initialize(name, items)とHashを受け取るようにしているのは何か思惑があるのでしょうか?

 ほかのテーブル系( DiceTable::Table, DiceTable::D66Table )とインタフェースの傾向を合わせる意図です。

oddとevenは必要不可欠な要素なので、コンストラクタの引数として要求した方が、コンストラクタの引数情報から表作成に必要な情報が分かり、良いのではないかと感じました。

 これもそのとおりだとは思うので、変更は可能です。

@ysakasin
Copy link
Member

def initialize(name, odd:, even:) に変更をお願いします。

def initialize(name, items)と書くとコンストラクタのインターフェースは一見同じように見えますが、実際は受け付けられるitemsの値が異なっているので、同じインターフェースにはなっていません。引数の数を合わせてもダックタイピングが成立しないので、読みやすさに舵を切った方が得られる恩恵が大きそうです。

先例をあげると、RangeTableもテーブル系のダックタイピングができるように各種メソッドが完備されていますが、コンストラクタの引数は必要な分だけ増えています

https://github.com/bcdice/BCDice/blob/master/lib/bcdice/dice_table/range_table.rb

@ViVi-shark
Copy link
Contributor Author

@ysakasin

 変更しました。 224d1c2

@ysakasin ysakasin merged commit acdb6ec into bcdice:master Dec 26, 2020
@ysakasin
Copy link
Member

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

多数の指摘にも根気強く対応してくださりありがとうございます。

@ViVi-shark ViVi-shark deleted the features/alchemia_struggle branch March 28, 2021 10:16
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