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

各ダイスボットにゲームシステム名の読みがなを設定できるようにする #128

Merged
merged 13 commits into from
Mar 18, 2020

Conversation

ochaochaocha3
Copy link
Member

@ochaochaocha3 ochaochaocha3 commented Feb 29, 2020

各ダイスボットにゲームシステム名の読みがなを設定できるようにします。この変更の目的は、ダイスボットを自然な順序で並び替えられるようにすることです。

方法

ゲームシステム名の定義および取得

以下に示すように、ダイスボットのクラスに SORT_KEY という定数を作り、ゲームシステム名の読みがなを定義します。

class KariDice < DiceBot
  # ...

  # ゲームシステム名の読みがな
  SORT_KEY = 'かりたいす'

  # ...
end

定義した読みがなは、DiceBot#sort_key メソッドで取得できるようにします。また、DiceBot#info で返される基本情報にキー "sortKey" を新設し、読みがなを格納します。

以上に合わせて、説明文書も更新します。既存の文書については、「ダイスボットのつくりかた」(docs/how_to_make_dicebot.md)の該当箇所を更新します。また、読みがなの設定方法について説明する文書「ゲームシステム名の読みがなの設定方法」(docs/dicebot_sort_key.md)を追加します。

ダイスボットの基本情報の定義・取得方法の変更

さらに、今回の変更では、v3移行作業のひとつである、ダイスボットの基本情報(ゲームシステムの識別子、ゲームシステム名、ダイスボットの使い方の説明文)を定義、取得するためのインターフェースの変更を同時に行います。目的は、今後必要となる、読みがなの周辺にある基本情報の定義部分の変更の手間を削減することです。ダイスボットの基本情報の項目が変更される今回の改修がちょうどよい機会ではないかと判断しました。

以下をクラス定数として定義します:

  • ゲームシステムの識別子:IDDiceBot#id で取得可能)
  • ゲームシステム名:NAMEDiceBot#name で取得可能)
  • ダイスボットの使い方の説明文:HELP_MESSAGEDiceBot#help_message で取得可能)

参考資料:https://gist.github.com/ysakasin/1464a15828f4beeb2988b61f924bacb9

確認・調整が必要なこと

  • ゲームシステム名の読みがなを含む、ダイスボットの基本情報の定義・取得インターフェースが妥当か。
  • DiceBotLoader.collectDiceBots でダイスボットの一覧を返すときに、ゲームシステム名の読みがなを基準としてソートするか。
    • ソートするようにすると、BCDice APIや各オンセツールにおいてソートが不要になる可能性があります。
  • 今回の変更をどのタイミングで導入するか。
    • BCDice外部に公開している部分(DiceBot#info)について、改名や削除は行われないため、インターフェースの変更としては軽微です。
    • BCDice全体への変更ではあるため、個人的にはマイナーバージョンアップ(X.Y.ZのYが増加)が適当と考えました。

一箇所にまとめて、クラスメソッド定義の範囲を分かりやすくする
以下を定数化し、v3のダイスボットのインターフェースに近づける。

* ID:ゲームシステムの識別子
* NAME: ゲームシステム名
* HELP_MESSAGE: ダイスボットの使い方
ダイスボットを自然な順序で並べ替えられるようにするため。

* 各ダイスボットクラスに定数 `SORT_KEY` を作成する。
* `DiceBot#sort_key` で各ダイスボットクラスの `SORT_KEY` を返す。
* `DiceBot#info` にキー `sortKey` を新設し、読みがなを格納する。

各ダイスボットの読みがなは、以下の作業用シートから取得した。
https://docs.google.com/spreadsheets/d/1kPzwAdcEL4KtmwkbqDB4TgycL75BrrxWHOyIBKsRtsw
@ysakasin
Copy link
Member

ysakasin commented Mar 2, 2020

Nice Work!

ゲームシステム名の読みがなを含む、ダイスボットの基本情報の定義・取得インターフェースが妥当か。

妥当だと思います。システム一覧表示のときに、頭文字で分類したいときに便利です

DiceBotLoader.collectDiceBots でダイスボットの一覧を返すときに、ゲームシステム名の読みがなを基準としてソートするか。
ソートするようにすると、BCDice APIや各オンセツールにおいてソートが不要になる可能性があります。

ソートして良いと思います。エンドユーザーの不利益にはならないと思います。

今回の変更をどのタイミングで導入するか。
BCDice外部に公開している部分(DiceBot#info)について、改名や削除は行われないため、インターフェースの変更としては軽微です。

時期が時期なので、どどんとふ離れを促すためにも入れてしまっていいと思っています。
ただ、これを入れてしまうと各ダイスボットの変更を個人鯖のどどんとふに取り入れようとしたときに、適切な知識が必要という問題は

BCDice全体への変更ではあるため、個人的にはマイナーバージョンアップ(X.Y.ZのYが増加)が適当と考えました。

どどんとふ2系はそもそもSem. Verを採用できていないので、これはどちらでもいい気がします

@ysakasin
Copy link
Member

ysakasin commented Mar 2, 2020

ただ、せっかくなのでダイスボットの命名ルールを統一してから採用したいなと思っています。

@codecov-io
Copy link

codecov-io commented Mar 6, 2020

Codecov Report

Merging #128 into master will increase coverage by 0.53%.
The diff coverage is 99.19%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #128      +/-   ##
==========================================
+ Coverage   86.30%   86.84%   +0.53%     
==========================================
  Files         192      193       +1     
  Lines       22266    21889     -377     
==========================================
- Hits        19217    19009     -208     
+ Misses       3049     2880     -169     
Impacted Files Coverage Δ
src/diceBot/DiceBotLoaderList.rb 100.00% <ø> (ø)
src/bcdiceCore.rb 71.46% <50.00%> (ø)
src/test/testDiceBotPrefixesCompatibility.rb 88.23% <75.00%> (ø)
src/diceBot/DiceBot.rb 93.52% <78.78%> (-1.20%) ⬇️
src/diceBot/AFF2e.rb 98.38% <100.00%> (+1.51%) ⬆️
src/diceBot/AceKillerGene.rb 100.00% <100.00%> (+5.00%) ⬆️
src/diceBot/Airgetlamh.rb 100.00% <100.00%> (+2.04%) ⬆️
src/diceBot/Alsetto.rb 100.00% <100.00%> (+1.88%) ⬆️
src/diceBot/Alshard.rb 100.00% <100.00%> (ø)
src/diceBot/Alter_raise.rb 100.00% <100.00%> (+0.90%) ⬆️
... and 170 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 0de2911...e48e9b6. Read the comment docs.

@ochaochaocha3
Copy link
Member Author

1c81f7a: DiceBotLoader#collectDiceBots の結果が読みがなでソートされるようにしました。以下は、オンセツールでのゲームシステム一覧を想定した実行例です。

bcdice-sort_key_sort

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.

元の表で間違っていた表記を修正して欲しいです

src/diceBot/Airgetlamh.rb Outdated Show resolved Hide resolved
src/diceBot/Alsetto.rb Outdated Show resolved Hide resolved
src/diceBot/Avandner.rb Outdated Show resolved Hide resolved
src/diceBot/StellarKnights.rb Outdated Show resolved Hide resolved
src/diceBot/HatsuneMiku.rb Outdated Show resolved Hide resolved
* 朱の孤塔のエアゲトラム(あけのことうのえあけとらむ)
* 詩片のアルセット(うたかたのあるせつと)
* 黒絢のアヴァンドナー(こつけんのあうあんとなあ)
* 初音ミクTRPG ココロダンジョン(はつねみくTRPGこころたんしよん)
* 銀剣のステラナイツ(きんけんのすてらないつ)

Co-Authored-By: SAKATA Sinji <[email protected]>
@ochaochaocha3
Copy link
Member Author

5つのダイスボットの読みがなを修正しました。

@ochaochaocha3 ochaochaocha3 changed the title WIP: 各ダイスボットにゲームシステム名の読みがなを設定できるようにする 各ダイスボットにゲームシステム名の読みがなを設定できるようにする Mar 11, 2020
@ochaochaocha3 ochaochaocha3 added the 新機能 新機能の実装やリクエスト label Mar 13, 2020
@ysakasin ysakasin mentioned this pull request Mar 14, 2020
@ysakasin
Copy link
Member

コンフリクトの解消と、ADVANCED FIGHTING FANTASY 2nd Edition の取り込みが終わり次第、マージしようと思います。

@ochaochaocha3
Copy link
Member Author

以下を行いました:

  • 72978ca: コンフリクトの解消
  • 6ce7fbb: ADVANCED FIGHTING FANTASY 2nd Editionの基本情報の定数化
  • e48e9b6: 「ダイスボットのつくりかた」に読みがなの設定方法の説明へのリンクを追加

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.

LGTM!

@ysakasin ysakasin merged commit e451d61 into master Mar 18, 2020
@ysakasin ysakasin deleted the sort_key branch March 22, 2020 04:07
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
新機能 新機能の実装やリクエスト
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants