マルチエンジン：エンジン毎の設定を追加 #1146
Merged
マルチエンジン：エンジン毎の設定を追加 #1146
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
sevenc-nanashi
requested review from
Hiroshiba
and removed request for
a team
Hiroshiba
reviewed
Jan 25, 2023
Hiroshiba
reviewed
Jan 25, 2023
Hiroshiba
reviewed
Jan 25, 2023
Hiroshiba
reviewed
Jan 25, 2023
Hiroshiba
reviewed
Jan 25, 2023
Hiroshiba
reviewed
Jan 25, 2023
Hiroshiba
reviewed
Jan 25, 2023
Hiroshiba
reviewed
Jan 25, 2023
There was a problem hiding this comment.
良いですね！！！
ちょっと全部見れてないのですが、問題が起こりにくいようにするコードの提案をいくつかコメントしました。
UIですが、実装を最速にする形を目指し、時間があればこだわっていくのが良いのかなと思いました！
なので現状からの提案の形はこんな感じになりそうです。
- 「エンジンの選択」を上に持っていく
- 「エンジンの選択」はマルチエンジン時のみ表示する
- こうすると、デフォルトエンジンの個別設定をしたあと、全体設定しかできなくなるが、これはそういう仕様にする
- 解消には「マルチエンジンじゃないときは継承する」処理を書けば良いけど、影響範囲が広いのでたぶんかなり大変
- FIXMEコメントだけ書いておく
- 個々のサンプリングレートは変えられないようにする
- そういうUIを作るより個々のサンプリングレートも変えられるようにしたほうが早いのであれば、そうしても良さそう。
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
|
|
Hiroshiba
reviewed
Jan 27, 2023
Hiroshiba
reviewed
Jan 27, 2023
Hiroshiba
reviewed
Jan 27, 2023
Hiroshiba
reviewed
Jan 27, 2023
Hiroshiba
reviewed
Jan 27, 2023
Hiroshiba
reviewed
Jan 27, 2023
Hiroshiba
reviewed
Jan 27, 2023
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
This reverts commit 880de38.
|
レビューされた部分を修正しましたー。 |
Hiroshiba
reviewed
Jan 27, 2023
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
Hiroshiba
reviewed
Jan 27, 2023
Hiroshiba
reviewed
Jan 27, 2023
| @@ -977,6 +975,7 @@ export type SettingStoreState = { | |||
| splitTextWhenPaste: SplitTextWhenPasteType; | |||
| splitterPosition: SplitterPosition; | |||
| confirmedTips: ConfirmedTips; | |||
| engineSettings: EngineSettings; | |||
There was a problem hiding this comment.
好みかもですが、他の〜sは複数の方を型定義してないので、合わせてこうすると揃うかもとか思いました。
Suggested change
| engineSettings: EngineSettings; | |
| engineSettings: Record<string, EngineSetting>; |
（こうするとEngineSettingsは不要になりそう）
（まあコメントしてますがかなりどっちでも良いかもです！）
Hiroshiba
reviewed
Jan 27, 2023
Comment on lines
+1048
to
+1052
| }): Promise<void>; | ||
| }; | ||
|
|
||
| CHANGE_USE_GPU: { | ||
| action(payload: { useGpu: boolean }): void; | ||
| action(payload: { useGpu: boolean; engineId: string }): Promise<void>; |
There was a problem hiding this comment.
Promise<void>じゃなくてvoidかも（たぶん勝手にPromise型になる）
There was a problem hiding this comment.
けっこう色々なところでPromiseがついてますね
Co-authored-by: Hiroshiba <hihokaruta@gmail.com>
y-chan
approved these changes
Jan 28, 2023
|
PRありがとうございました！！マージします！！ |
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.
内容
タイトル通り。
関連 Issue
スクリーンショット・動画など
（なし）
その他
（なし）