Skip to content

Conversation

@sigprogramming
Copy link
Collaborator

@sigprogramming sigprogramming commented Dec 23, 2023

内容

重なっているノートを検出して色を変えて表示する機能を追加します。
また、以下も行います。

  • action、mutationを整理
    • ADD_NOTEからADD_NOTESに変更(複数ノートに対応)
    • UPDATE_NOTEからUPDATE_NOTESに変更(複数ノートに対応)
      • REPLACE_ALL_NOTESを削除
    • REMOVE_NOTEからREMOVE_NOTESに変更(複数ノートに対応)
    • SET_SELECTED_NOTE_IDSのactionを削除
      • SELECT_NOTESを追加
      • CLEAR_SELECTED_NOTE_IDSからDESELECT_ALL_NOTESに変更
    • REMOVE_SELECTED_NOTESのmutationを削除(REMOVE_NOTESを呼び出すように変更)
  • REPLACE_ALL_NOTESを削除したので、それに合わせてドラッグ周りの処理を修正
  • レンダリング時の重なっているノートを削除する処理を修正
    • state.overlappingNoteIdsを元に削除するように変更
  • リファクタリング

重なっているノートを検出する処理は計算量が$O(n^2)$になっています。

関連 Issue

VOICEVOX/voicevox_project#15

その他

@sigprogramming sigprogramming requested a review from a team as a code owner December 23, 2023 06:27
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!!

dev環境じゃない状態で起動する方法を調べてみました。
いろいろややこしいので実際にビルドしちゃうのが手っ取り早そうでした!

ビルドコマンド↓

npm run electron:build -- --dir

ビルド完了後はdist_electron/win-unpacked/VOICEVOX.exeができてるはずです。
エンジンが同梱されないので、実行するとエンジン起動エラーが表示されると思います。
別でエンジンrun.exeを起動してあげればUIが表示されるかなと!

(ただ製品のVOICEVOXと同じconfigファイルを見るので、configの状況によっては正しく表示されないかもです)

});
const selectedNoteIds = computed(() => state.selectedNoteIds);
const selectedNotes = computed(() => {
const selectedNoteIds = new Set(state.selectedNoteIds);
Copy link
Member

Choose a reason for hiding this comment

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

selectedNoteIdsはもともと`Setにしておいても良いかもですね!

Comment on lines +107 to 115
const classNamesStr = computed(() => {
if (state.selectedNoteIds.includes(props.note.id)) {
return "sequencer-note selected";
}
if (state.overlappingNoteIds.includes(props.note.id)) {
return "sequencer-note overlapping";
}
return "sequencer-note";
});
Copy link
Member

Choose a reason for hiding this comment

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

文字列の配列を返してもいけるかも・・・?
https://ja.vuejs.org/guide/essentials/class-and-style#binding-to-arrays

つまりこんな感じも。(sequencer-noteのコピペを抑えられる)

const classes = ["sequencer-note"]
if(selected) { classes.push("selected") }
else if(overlapped) { classes.push("overlapping") }
return classes

Comment on lines +727 to +729
const scoreNotes = state.score.notes;
const existingIds = new Set(scoreNotes.map((value) => value.id));
const isValidNoteIds = noteIds.every((value) => existingIds.has(value));
Copy link
Member

Choose a reason for hiding this comment

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

この処理結構見かけるのでGETTER辺りに置いても良いかもと思いました!

@Hiroshiba Hiroshiba merged commit b7e4e54 into VOICEVOX:project-s Dec 23, 2023
@sigprogramming sigprogramming deleted the overlapping_notes branch January 14, 2024 12:38
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.

2 participants