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

[project-sequencer-statemachine] MoveNoteStateを追加 #2412

Conversation

sigprogramming
Copy link
Contributor

@sigprogramming sigprogramming commented Dec 12, 2024

内容

以下を行います。

  • MoveNoteStateを追加
  • リファクタリング

関連 Issue

その他

@sigprogramming sigprogramming requested a review from a team as a code owner December 12, 2024 12:30
@sigprogramming sigprogramming requested review from Hiroshiba and removed request for a team December 12, 2024 12:30
Comment on lines -169 to -170
context.storeActions.deselectAllNotes();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

プレビュー開始時のノートの選択・選択解除はIdleStateで行うようにしました。

@voicevox-preview-pages
Copy link

voicevox-preview-pages bot commented Dec 12, 2024

🚀 プレビュー用ページを作成しました 🚀

更新時点でのコミットハッシュ:1574677

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です!!

いくつかコメントしていますが、結構頓珍漢なこと言ってるかもです 🙇
あ、「こうすると良いかも」みたいなコメントもいくつかあるかもですが、リファクタング前と同じコードなのであれば一旦そのままでも良い気がしています。
気が向けば、くらい・・・!

src/sing/stateMachine/sequencerStateMachine.ts Outdated Show resolved Hide resolved
src/sing/stateMachine/sequencerStateMachine.ts Outdated Show resolved Hide resolved
src/sing/stateMachine/sequencerStateMachine.ts 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!!

マージします!!

Comment on lines +404 to +413
for (const note of editedNotes.values()) {
if (note.noteNumber < 0 || note.noteNumber >= keyInfos.length) {
// MIDIキー範囲外へはドラッグしない
return;
}
if (note.position < 0) {
// 左端より前はドラッグしない
return;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

(ただのコメントです)
ここ、可能ならドラッグ処理を打ち切るのではなく、範囲外に行かない範囲でドラッグ処理を実行してあげるとより優しいかもですね!

@Hiroshiba
Copy link
Member

テストが落ちてしまっていますが↓を取り込んだmainブランチを取り込めば解決すると思います!
ってことで問題ないということでマージ!

@Hiroshiba Hiroshiba merged commit a4c4696 into VOICEVOX:project-sequencer-statemachine Dec 19, 2024
9 of 10 checks passed
@sigprogramming sigprogramming deleted the add_move_note_state branch December 20, 2024 18:50
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