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

Add: UtaFormatixを導入してインポートできる対応形式を増やす #2104

Merged
merged 55 commits into from
Jun 15, 2024

Conversation

sevenc-nanashi
Copy link
Member

内容

utaformatixを入れます。
とりあえず「外部プロジェクトファイル」/「外部プロジェクト」と呼んでいますがもっといい名前があるかも。

関連 Issue

(なし)

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

(なし)

その他

(なし)

@sevenc-nanashi sevenc-nanashi requested a review from a team as a code owner May 27, 2024 11:28
@sevenc-nanashi sevenc-nanashi requested review from Hiroshiba and removed request for a team May 27, 2024 11:28
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.

良いですね!!!!

まだstore/singing.ts追えてないですが、一旦コメントまで・・・!

外部プロジェクトファイル

コード内で書くときは歌限定なことが伝わる名前にすると無難かもですね!
あとユーザーに露出するとこだと、MIDIファイルやMusicXMLはプロジェクトファイルに見えないかもです。

src/components/Dialog/ImportExternalProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/AllDialog.vue Outdated Show resolved Hide resolved
src/components/Sing/menuBarData.ts Outdated Show resolved Hide resolved
src/components/Dialog/ImportExternalProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportExternalProjectDialog.vue Outdated Show resolved Hide resolved
@sevenc-nanashi sevenc-nanashi marked this pull request as draft June 1, 2024 13:26
@sevenc-nanashi sevenc-nanashi marked this pull request as ready for review June 1, 2024 22:14
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.

良い感じだと思います!!

ちょっとコードのメンテナンス性を上げたくいくつかコメントしました!
あとUXも相談したくいくつかコメントしました。もしよければ 🙏

src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/helpers/fileReader.ts Outdated Show resolved Hide resolved
src/store/type.ts Outdated Show resolved Hide resolved
tests/unit/lib/midi/midi.spec.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/singing.ts Outdated Show resolved Hide resolved
src/store/singing.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.

設計が難しくて右往左往してしまってすみません。。
とりあえず見ました!

src/sing/utaformatixProject/common.ts Outdated Show resolved Hide resolved
src/sing/utaformatixProject/import.ts Outdated Show resolved Hide resolved
src/store/singing.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です!!!

src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/sing/utaformatixProject/fromVoicevox.ts Show resolved Hide resolved
src/sing/utaformatixProject/fromVoicevox.ts Show resolved Hide resolved
src/sing/utaformatixProject/toVoicevox.ts Outdated Show resolved Hide resolved
src/sing/utaformatixProject/toVoicevox.ts Outdated Show resolved Hide resolved
src/store/singing.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!!

いくつかコメントしましたが、こちらで書き加えようと思います!
roundの部分だけどうすべきか @sevenc-nanashi さんの意見をお待ちします!

スナップショットテスト良いですね!!
こういうときにはバンバン使っていって良い気もしますね。

あとImportSongProjectDialog.vueのStorybook興味あったらぜひチャレンジしてみてください。
めっちゃ面白い&健全になると思うのですが、手が出せてないだけなので。。

src/store/singing.ts Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
src/components/Dialog/ImportSongProjectDialog.vue Outdated Show resolved Hide resolved
@Hiroshiba
Copy link
Member

あ、utaformatixを使って"いろんなソングプロジェクトをimport可能にする"ってタイトルに変えたほうが良いかもですね。
exportはまだなので。そんな感じに変えさせていただきます!

@Hiroshiba Hiroshiba changed the title Add: utaformatixを入れる Add: UtaFormatixを導入してインポートできる対応形式を増やす Jun 9, 2024
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.

変えてみたけど、なんかいびつなコードになりました 😇
940826d

エラーメッセージにエラーメッセージ以外の意味を持たせてますね・・・
{error: Error, message: string}型にするのが正しそうだけど、まあ良いかな。。。

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

お疲れ様でした!!
エクスポート機能も期待しています・・・!!
(あとマルチトラックも・・・!!!)

ツイート文でUtaFormatixへの感謝を伝えたい気持ちがありますが、UtaFormatix公式提供みたいな見え方になっちゃうと迷惑かけちゃうかもですねぇ・・・

まあUtaFormatixを導入し、様々なソングプロジェクトファイル(.ccs/.svp/.vsqなど)のインポートに対応しました🎉とかが無難ですかね。
そうします!

@Hiroshiba Hiroshiba merged commit 8c944eb into VOICEVOX:main Jun 15, 2024
9 checks passed
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