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

Improve: 複数エンジン登録時の起動を高速化 #1015

Merged
merged 3 commits into from
Nov 15, 2022

Conversation

sevenc-nanashi
Copy link
Member

@sevenc-nanashi sevenc-nanashi commented Nov 5, 2022

内容

複数エンジン登録時の起動を高速化します。
旧実装:

graph LR
    T1[ ]:::empty

    A[起動] --- T1
    T1 --> B1 & B2
    subgraph START_WAITING_ENGINE_ALL
      B1[エンジンAの起動待機]
      B2[エンジンBの起動待機]
    end
    
    T2[ ]:::empty
    T2p[ ]:::empty

    B1 & B2 --- T2p
	T2p --- T2
    T2 --> C1 & C2

    subgraph FETCH_AND_SET_ENGINE_MANIFESTS_ALL
      C1[エンジンAのManifest読み込み]
      C2[エンジンBのManifest読み込み]
    end

    T3[ ]:::empty
    T3p[ ]:::empty

    C1 & C2 --- T3p
	T3p --- T3
    T3 --> D1 & D2

    subgraph LOAD_CHARACTER_ALL
      D1[エンジンAのキャラクター読み込み]
      D2[エンジンBのキャラクター読み込み]
    end

    T4[ ]:::empty
    T4p[ ]:::empty

    D1 & D2 --- T4p
	T4p --- T4
    T4 --> E[UNLOCK_UI]
    classDef empty width:0px,height:0px;
Loading

新実装:

graph LR
    t1[ ]:::empty

    a[起動] --- t1
    t1 --> b1 & b2
    subgraph  
      b1[エンジンAの起動待機] --> c1
      c1[エンジンAのManifest読み込み] --> d1
      d1[エンジンAのキャラクター読み込み]
    end
    subgraph  
      b2[エンジンBの起動待機] --> c2
      c2[エンジンBのManifest読み込み] --> d2
      d2[エンジンBのキャラクター読み込み]
    end

    t2[ ]:::empty
    d1 & d2 --- t2
    t2 --> e[UNLOCK_UI]
    classDef empty width:0px,height:0px;
Loading

関連 Issue

(なし)

スクリーンショット・動画など

(なし)

その他

(なし)

src/views/EditorHome.vue Outdated Show resolved Hide resolved
Comment on lines +241 to +244
state.engineManifests = {
...state.engineManifests,
[engineId]: engineManifest,
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

たしか普通にこうで良いはず!

Suggested change
state.engineManifests = {
...state.engineManifests,
[engineId]: engineManifest,
};
state.engineManifests[engineId] = engineManifest;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

それだとなぜかcomputedのところが更新されなかったんですよね。
MenuBarのアイコンのところが埋まりませんでした。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

うーん、なるほどです。なんでだろう。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

state.xxx =のところにcomputedとかの更新処理が挟まってて、[engineId] = だとそれが行われないのかも(自分の勝手な予想)

src/store/type.ts Outdated Show resolved Hide resolved
src/views/EditorHome.vue Outdated Show resolved Hide resolved
Copy link
Member

@Hiroshiba Hiroshiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!!

@raa0121 さん
すみません、また実行チェックなどお願いできると嬉しいです!!

@raa0121
Copy link
Contributor

raa0121 commented Nov 15, 2022

実行チェックできました。
セーフモードのボタンが出てこないくらいには早く起動してると思います。

@Hiroshiba
Copy link
Member

engine_manifestのポート番号を実際のと変えたりすると、接続不可になってセーフモードのチェックもできるかもです!

@sevenc-nanashi
Copy link
Member Author

そもそもセーフモードのボタンはこのブランチに無いような…?

@raa0121
Copy link
Contributor

raa0121 commented Nov 15, 2022

そもそもセーフモードのボタンはこのブランチに無いような…?

そうですね、未マージの状態でした。
手元でマージして試してみます。

@raa0121
Copy link
Contributor

raa0121 commented Nov 15, 2022

手元でマージしたら、conflict しましたね…
セーフモードのブランチの上に載せたほうが良かったかもですね。

@Hiroshiba
Copy link
Member

あ!!すみません、セーフモードのプルリクエストだと勘違いしました!!!
まあつまり大丈夫ということですね、検証ありがとうございます!!

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

Successfully merging this pull request may close these issues.

3 participants