feat(serve): add nicoliveComment plugin (server side) #1459
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xnv
commented
Nov 21, 2024
| @@ -34,6 +35,7 @@ export const onClientInstanceDisappear = new Trigger<ClientInstanceDisappearTest | |||
| export const onBroadcast = new Trigger<any>(); | |||
| export const onDisconnect = new Trigger<void>(); | |||
| export const onPutStartPoint = new Trigger<PutStartPointEvent>(); | |||
| export const onNicoliveCommentPluginStartStop = new Trigger<NicoliveCommentPluginStartStopTestbedEvent>(); | |||
There was a problem hiding this comment.
このトリガーまでは fire していますがこの先の利用者は未実装です。
xnv
commented
Nov 21, 2024
| this.onRunnerPause = new Trigger(); | ||
| this.onRunnerResume = new Trigger(); | ||
| this.onRunnerPutStartPoint = new Trigger(); | ||
| this.onNicoliveCommentPluginStartStop = new Trigger(); |
xnv
commented
Nov 21, 2024
Comment on lines
+74
to
+75
| if (!this.planned) | ||
| this._setupPlan(age); |
There was a problem hiding this comment.
毎フレームの処理を最小限にしておきたく、最初に送信タイミングと内容をまとめて Map に登録しています。(本当はプラグインの start() 時点で行いたいのですが、最初の tick のフレーム番号 (age) が必要なのでこの箇所 (start() で登録される onTick のハンドラが最初に呼ばれたタイミング) で行っています)
yu-ogi
reviewed
Nov 22, 2024
packages/akashic-cli-serve/src/server/domain/nicoliveComment/NicoliveCommentPluginHost.ts
Outdated
Show resolved
Hide resolved
packages/akashic-cli-serve/src/server/domain/nicoliveComment/NicoliveCommentPluginHost.ts
Show resolved
Hide resolved
packages/akashic-cli-serve/src/server/domain/nicoliveComment/NicoliveCommentConfig.ts
Outdated
Show resolved
Hide resolved
…rn invalid comment field
…allback isn't given
yu-ogi
approved these changes
Dec 17, 2024
kamakiri01
reviewed
Dec 17, 2024
| /** | ||
| * コメントデータ。 | ||
| */ | ||
| export interface NicoliveComment { |
There was a problem hiding this comment.
- NicoliveCommentConfigCommentと定義の重複が多いため、片方をextendsしても良いのではないかと思います。
- 名前がプラグインと同じで、他の型名には同名のprefixが付いているので、
NicoliveCommentDataのように「プラグイン名+意味」のような名前にすると扱いやすそうだなと思います。
There was a problem hiding this comment.
うーん、継承するならこちらが祖先だとは思いますが。しかし外形的に同じであっても意味 (特に省略時の) が違うので、いまいち is-a の関係にないような。だからこそ JSDoc コメントの内容が違いますし、実務的にもコメントのために同じプロパティを重複して書くことは避けられないように思います。
また一般論として "data" はできるだけ避けたい単語だと思っています。計算機の扱うものに data でないものは存在しないからです。(ある程度以上抽象的な処理だと data としか呼びようがないことはありますが)
There was a problem hiding this comment.
- そうですね、確かに祖はこちらになると思います。省略時の扱いが違う点は見落としていました。同じプロパティが重複して存在するのを避けるために複雑な型を作るよりは、現状のほうが良さそうです。
- 単語（data）は一例で挙げましたが、他の単語でも良いと思います（自分も積極的にdataは付けないほうが良いと思いますが、コメント時に当意即妙な単語を出せませんでした）。
型の名前が、他の型と同じようにNicoliveComment+Xxxになっていると扱いやすそうです。この型はあくまで「コメントデータ」を表すためのものですが、それが他の多くの型のprefixと同じところが気になりました。
feat(serve): add nicoliveCommentPlugin (client-side) with developer tool support
kamakiri01
approved these changes
Dec 25, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
掲題通り。
仕様挙動いずれも暫定のため詳細は割愛します。