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

feat: [project-sequencer-statemachine] mainにマージする #2555

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

sigprogramming
Copy link
Contributor

内容

project-sequencer-statemachineブランチをmainブランチにマージします。

関連 Issue

その他

@romot-co @sevenc-nanashi
プレビュー周り(矩形選択と歌詞編集含む)が大きく変わっているので、
挙動(クリックの判定やカーソルの切り替わりなど)が変わっていないか、ご確認いただけると助かります…!

sigprogramming and others added 26 commits August 9, 2024 02:29
* ステートマシンを追加

* IdleStateとAddNoteStateを追加

* noImplicitOverrideを有効にするとエラーが出るので無効にして、抽象クラスをインターフェースにした

* メソッドの引数のところを分割代入に変更

* StateをIStateに変更し、StatesをStateに変更

* ファイルを移動、ファイル名を変更

* Dispatcherを無くした

* ドキュメントコメントを追加

* 現在使われていないことをコメントで説明

* 初期ステートのonEnterを呼んでいなかったので修正

* コンポーザブル化した

* sequencerBodyは使用していなかったので削除

* 修正

* nextStateをインスタンス変数からローカル変数にする

* innerContextとしてまとめた
…ート追加の処理を行うようにする (#2540)

Co-authored-by: Hiroshiba Kazuyuki <[email protected]>
@sigprogramming sigprogramming requested a review from a team as a code owner February 22, 2025 02:16
@sigprogramming sigprogramming removed the request for review from a team February 22, 2025 02:16
@voicevox-preview-pages
Copy link

voicevox-preview-pages bot commented Feb 22, 2025

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

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

Copy link
Contributor

@romot-co romot-co left a comment

Choose a reason for hiding this comment

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

外形的に軽くみた限りでは問題なさそうです…!
ややこしかった状態がすっきりしていてすごいです。

すごく細かいところですが一応見つけたこととしては
複数ノート選択+ドラッグでシーケンサ左端より移動させようとすると、
左端に達したノート以外が端に達するまで動いてしまうかも

test-state-machine-1.mp4

とはいえ大きな問題じゃなさそうに思えます!

@sigprogramming
Copy link
Contributor Author

@romot-co
挙動の確認ありがとうございます!
これは確か、一番左のノートが端に達してもドラッグできた方が便利かも、というので変更した感じだったと思います!
(ノートリサイズの方も同様に変更しています)
#2412 (comment)#2463 (comment)

@romot-co
Copy link
Contributor

@sigprogramming
なるほど!それであれば大丈夫そうです!実際その方が便利そうです

@Hiroshiba
Copy link
Member

Hiroshiba commented Feb 22, 2025

あと他に把握している差としては、ピッチ編集中にctrlを押して削除できる状態になると、消しゴムボタンが選択されたようなUIになっていたのが、ピッチ編集ボタンが選択されたままのようになっています!

ピッチ編集の消しゴムボタン、これです。
image

この仕様で良いかは一度 @romot-co さん的な意見聞けると嬉しいかも。
(個人的には、消しゴムモードになっていることがわかりやすい一方で、モード変更がされたように表示するのはちょっと不自然かもではあるので、このプルリクの仕様が良いのかな~と思ってます。
今はマウスカーソルが変わらないけど、マウスカーソルが消しゴムになれば結構自然なはず・・・?)

@romot-co
Copy link
Contributor

romot-co commented Feb 22, 2025

@Hiroshiba

このプルリクの仕様が良いのかな~と思ってます。

私的にも問題ないです!一時的なツール変更なので消しゴムカーソルになれば=変更されているのがわかれば必要十分かと…!
(あとメンテが面倒そう&状態管理が複雑なわりに得られるメリットが少なそう)

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