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

巨大なダイスボットファイルの分割 #169

Open
ochaochaocha3 opened this issue Apr 30, 2020 · 3 comments
Open

巨大なダイスボットファイルの分割 #169

ochaochaocha3 opened this issue Apr 30, 2020 · 3 comments
Labels
refactoring 内部構造の改良 要情報 さらなる情報が必要

Comments

@ochaochaocha3
Copy link
Member

ochaochaocha3 commented Apr 30, 2020

ログ・ホライズン(v2.04.00時点で3551行)に代表される、巨大なダイスボットファイルを分割して、処理を探しやすくしたい。

背景

一般に、表が多いダイスボットのファイルは大きくなり、変更時に対象の処理を探しにくい。
対策として、v2.02.73以前のダイスボットについては、extratables ディレクトリを利用して、表のみ別ファイルにされている場合があった。
しかし、「起動時に必ずテーブルを読み出すから処理が重い」などと(処理時間測定なしの)苦情が来たため、v2.02.74(2017年8月)でダイスボット本体にマージせざるを得なくなった。
1ダイスボットにつき1つの(場合によっては巨大な)ファイルとなったことには、このような事情があった。

現在は、以下の理由のため、高速化を目的として無理に「1ダイスボットにつき1ファイル」とする必要性は薄れている。

  • 開発終了に伴い、どどんとふの個別サーバ設置が減っている → どどんとふの起動時間が問題となることが少ない。
  • 表のクラスが作られたため、Rubyのコードで(=追加の構文解析処理なしで)ある程度分かりやすく表を記述できる。
  • そもそも、処理時間測定なしでの苦情だったため、本当に別ファイルの表が原因で遅くなっていたのかが怪しい。

したがって、現在は、大きなダイスボットファイルの分割によって処理が探しやすくなる利点の方が、処理速度低下という欠点より大きいと思われる。

方針案

各項目ともに、採否は未決定。

  • 特殊処理が入るコマンドは今まで通りのファイルに書いて、extratables相当の処理を外だしできるテーブルだけ別ファイルに書く。(@ysakasin 案)
  • 長いテーブルとクラス内クラスを別ファイルに書く。その際、厳密に1個ずつ別ファイルでなくてもよい。各ファイル200〜300行以内を目標にする。(@ochaochaocha3 案)
  • 分割したファイルのパス:diceBot/ゲームシステム名/*.rb@ochaochaocha3 案)
    • 例:diceBot/LogHorizon/MGR.rb
  • テーブルをDSLで書けるようにする(@ysakasin 案)
    • prefixes への登録も自動的に行う。
    • 表がいくつかある程度の普通のダイスボットを、簡潔に書けるようにする。

要決定項目

  • 上の方針案のうち、どれを採用するか?
  • どのダイスボットを分割の対象とするか?
    • ファイル分割によって処理を探しやすくなる具体例がほしい。
  • どのくらいの行数をファイル分割の目安とするか?
    • あまり細かく分けすぎると、逆に処理を見つけにくくなる。
@ochaochaocha3 ochaochaocha3 added 要情報 さらなる情報が必要 refactoring 内部構造の改良 labels Apr 30, 2020
@ysakasin
Copy link
Member

ysakasin commented May 1, 2020

どのくらいの行数をファイル分割の目安とするか?

RuboCopの Metrics/ClassLength によるデフォルト値が100論理行なので、これに引っかからないぐらいにしたい。300行程度?

Ref.

@ysakasin
Copy link
Member

ysakasin commented May 1, 2020

100論理行を超えているクラス一覧

100行以上:112クラス
200行以上:77クラス
300行以上:58クラス
500行以上:29クラス
1000行以上:12クラス

Name Logical Lines Real Lines
LogHorizon.rb 3353 3551
BeginningIdol.rb 2344 2521
BeginningIdol_Korean.rb 2199 2371
GurpsFW.rb 2115 2343
MeikyuKingdom.rb 1973 2258
Dracurouge.rb 1717 1847
FilledWith.rb 1357 1541
KillDeathBusiness_Korean.rb 1333 1503
KillDeathBusiness.rb 1278 1420
LogHorizon_Korean.rb 1200 1392
Elysion.rb 1091 1228
Satasupe.rb 1050 1171
MagicaLogia.rb 965 1142
KanColle.rb 936 1040
FutariSousa.rb 889 1004
OneWayHeroics.rb 856 977
MetalHeadExtream.rb 823 914
StellarKnights.rb 798 915
TwilightGunsmoke.rb 758 852
_InsaneScp.rb 711 822
HatsuneMiku.rb 702 772
Insane_Korean.rb 658 767
Insane.rb 658 767
ShoujoTenrankai.rb 647 674
ShinobiGami.rb 638 674
Garako.rb 607 671
Cthulhu7th.rb 592 740
HuntersMoon.rb 568 647
Amadeus.rb 551 614
BloodCrusade.rb 496 552
DemonParasite.rb 482 520
ColossalHunter.rb 482 558
Villaciel.rb 468 554
GranCrest.rb 464 514
ShinkuuGakuen.rb 449 527
DarkDaysDrive.rb 448 503
DeadlineHeroes.rb 443 520
YankeeYogSothoth.rb 440 478
Amadeus_Korean.rb 438 493
MonotoneMuseum.rb 430 484
MeikyuDays.rb 427 510
GardenOrder.rb 412 448
BloodMoon.rb 401 449
BadLife.rb 385 435
TorgEternity.rb 382 426
SwordWorld.rb 379 493
BlindMythos.rb 372 468
SterileLife.rb 367 449
DiceBot.rb 365 584
Kamigakari_Korean.rb 356 429
BeastBindTrinity.rb 342 435
LiveraDoll.rb 341 375
Kamigakari.rb 337 396
Gurps.rb 337 398
MeikyuKingdomBasic.rb 334 400
BattleTech.rb 318 531
Peekaboo.rb 311 350
TrinitySeven.rb 308 361
Torg.rb 291 368
Warhammer.rb 280 334
Cthulhu7th_ChineseTraditional.rb 274 384
Cthulhu7th_Korean.rb 272 381
Paradiso.rb 267 317
Nuekagami.rb 263 290
StratoShout.rb 255 312
MonotoneMuseum_Korean.rb 254 299
GundogRevised.rb 247 295
Dracurouge_Korean.rb 241 310
Alter_raise.rb 235 278
DetatokoSaga.rb 233 313
DetatokoSaga_Korean.rb 233 313
KemonoNoMori.rb 225 255
GundogZero.rb 219 259
BladeOfArcana.rb 218 246
Skynauts.rb 213 319
CardRanker.rb 213 256
WitchQuest.rb 212 273
EarthDawn3.rb 195 272
ParasiteBlood.rb 193 230
TunnelsAndTrolls.rb 189 256
MetalHead.rb 188 245
EmbryoMachine.rb 184 221
HouraiGakuen.rb 184 257
Strave.rb 182 220
LostRoyal.rb 179 238
EarthDawn4.rb 178 269
Cthulhu_ChineseTraditional.rb 174 240
DoubleCross.rb 172 417
Cthulhu_Korean.rb 172 238
Cthulhu.rb 172 237
DarkBlaze.rb 163 216
NinjaSlayer.rb 163 271
SwordWorld2_0.rb 162 222
HarnMaster.rb 156 199
EarthDawn.rb 150 222
BarnaKronika.rb 150 205
SRS.rb 145 343
PulpCthulhu.rb 144 181
Ryutama.rb 141 196
Oukahoushin3rd.rb 135 156
DiceBotLoaderList.rb 134 152
Postman.rb 134 177
NightWizard.rb 128 180
Raisondetre.rb 127 166
Chill.rb 124 158
Nechronica_Korean.rb 113 162
BattleTech.rb 111 531
ArsMagica.rb 108 142
Utakaze.rb 107 158
RecordOfSteam.rb 105 151
Nechronica.rb 105 147
ScreamHighSchool.rb 103 125

@ysakasin
Copy link
Member

ysakasin commented May 1, 2020

300論理行くらいは許容しても良さそう

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring 内部構造の改良 要情報 さらなる情報が必要
Projects
None yet
Development

No branches or pull requests

2 participants