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

エンジン追加・削除時の再起動要求を極力なくす #1189

Closed
3 tasks
nmori opened this issue Feb 8, 2023 · 6 comments
Closed
3 tasks

エンジン追加・削除時の再起動要求を極力なくす #1189

nmori opened this issue Feb 8, 2023 · 6 comments

Comments

@nmori
Copy link
Contributor

nmori commented Feb 8, 2023

内容

  • エンジンを追加するたびにエディタの再起動を求められるのはユーザ体感が悪そうです。
  • できるだけエディタを再起動させないことで、エンジン追加後にユーザがすぐ作業に入れる状況を作ります。
  • 操作頻度としては少ないので、これを実装する方がよいかは議論する方がよいかと思います。

Pros 良くなる点

  • 追加操作対してのユーザ体感がよくなる

Cons 悪くなる点

  • 起動に失敗するケースなどに対応する必要があり、内部遷移状態が少し複雑になる
  • エンジン再起動自体は待つ必要が発生する。

実現方法

  • エンジンは単体で再起動可能な状態にあることから、エンジン追加・削除時にEngineInfoを再生成する

  • そのうえで、エンジンを再起動すれば、エディタ側を再起動する必要はなさそうです。

  • 再起動を求める部分に、下記コードを差し込むことで、最低限の形は実現できそうです。(エラー処理やUI周りの部分で追加調整は必要)

          await store.dispatch("GET_ENGINE_INFOS");
          const engineIds = computed(() => store.state.engineIds);
          await store.dispatch("RESTART_ENGINES", {
            engineIds: engineIds.value,
          });
  • テストケースとしてエンジン削除時や、エンジンの追加/削除が入り乱れるケースなどを考慮する必要がありそうです。

VOICEVOXのバージョン

0.14.2

OSの種類/ディストリ/バージョン

  • Windows
  • macOS
  • Linux

その他

@Hiroshiba
Copy link
Member

Hiroshiba commented Feb 8, 2023

issue化ありがとうございます!!
洗い出しもすごく助かります!

@nmori さんが実現方法にあげてくださったEngineInfoのリロード方法も課題ですが、他にも2つほどポイントがあるかもと思いました。

まずEngineInfoのリロード方法ですが、これはエンジン追加・消去が終わった後にどこかでGET_ENGINE_INFOSを呼べば良いかなと思います。

2つ目のポイントはしばらくの間エンジンが未起動な状態になるので、この間もUIがエラーが出ないようにする必要がありそうです。
いたるところにエンジンが起動している前提だったり、キャラ情報などが不変であることを前提にしたコードがあるかもと思ってます。

3つ目がどうやってエンジンを停止したあとにengineInfosを更新するかも重要そうです。
windowsは起動中のexeを消せないので、消去するときはエンジン停止→消去→engineInfos更新になるはずで、このロジックは現状だと再起動以外ないかもです。

という感じで、操作頻度に比べてかなりヘビーな改修が必要なので、とりあえず後回しにしている感じです 😇

@nmori
Copy link
Contributor Author

nmori commented Feb 18, 2023

検討用にかいてみました。

  • 追加、削除はcomputedなので何とかなっているように見える。
  • エンジンの削除は現実装があるので、この削除機能でなんとかなっているようにみえるが、
     複数のエンジンを削除した場合をケアしないといけなさそう。
  • メイン画面で選ばれているキャラのエンジンが消されたとき、「有効なエンジンを選んで」と出る。
     落ちることはないが、ありたい姿は「使えるエンジンがなにか1つ選定されセットされる」?
    ・シナリオがある状態の場合、キャラが一括で崩れる部分をどう対応すべきか
     「このキャラがいないので代替を設定してください」とすべきか、 
     「空欄にしておいて、そのままユーザに選んでもらうまでそのまま」にするか。

ということで、やはりヘビーですね… 

ちなみに、ステップゴールをくぎるとしたら、
・追加したときに毎回再起動を求めず、エンジン管理ページを閉じるときに聞く
・追加・削除したエンジンは、起動しているものとは別にステートをもって再起動を促す表示をだす
をまずは実装すると、最小限の変更でユーザ体感を上げられるかもしれません。

@Hiroshiba
Copy link
Member

削除に関してはたしかに現状でもなんとかなっている気がしました!

エンジンを追加するとき、再起動しなくても追加したエンジンは起動するんでしたっけ・・・?
そこも変更が必要かもと思いました!

@nmori
Copy link
Contributor Author

nmori commented Feb 19, 2023

追加画面を閉じるときにこれを実行すれば全再起動するのでよさそうですが、
リスクとしてはデフォルトエンジンの再起動に失敗するとUIが「起動待ち」で止まってしまいます。
(もしかしたら、デフォルト以外を再起動する、とかにすればリスクは最低限にできるのかも?ですね)

            const engineIds = computed(() => store.state.engineIds);
            await store.dispatch("RESTART_ENGINES", {
              engineIds: engineIds.value,
            });

@Hiroshiba
Copy link
Member

Hiroshiba commented Feb 19, 2023

なるほどです、なんとなく掴めてきました!

エンジン管理画面を閉じたときに全処理を行うのは良さそうに思いました!!
消去すべきものをすべて消去し、追加すべきものをすべて追加すれば良さそう。

RESTART_ENGINESはRESTARTなので、プロセスをkillしようとするはずです。
なので、追加したエンジンは「再起動」ではなく「起動」だけするようにするか、あるいはRESTART_ENGINESの仕様を「起動していないエンジンはkillを飛ばす」と変えることになるかなと思いました。
(後者は結構不思議な処理かなと思いました)

あと、現状の「消去」はおそらく単に見ないようにしているだけでエンジンは起動しっぱなし(たぶん)なので、消した後に追加したときの挙動もバグりやすそうに感じました。
(portが使われてるので起動しない、ということなど)

素直に実装するなら、こうなるかなーと思いました・・・!

  • エンジンがなくても正しくUIが表示されるようにする
  • 「エンジンを追加して起動する」処理を書く
  • 「エンジンを停止して消去する」処理を書く
  • エンジン管理画面を閉じたときに順次実行していく

@Hiroshiba
Copy link
Member

こちらの課題ですが、↓のプルリクエストで解決したのでクローズします!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants