add AccentPhraseに個別idを設定 #1879

P0ngCh4ng · 2024-02-27T06:22:28Z

AccentPhraseを継承したEditorAccentPhraseという型を作成し、EditorAudioQueryのプロパティとした。

AudioDetail.vueファイルの中でaccentPhrases変数の変更のタイミングで割り振るように

内容

  SET_ACCENT_PHRASES_EDITORID: {
    mutation(
      state,
      {
        audioKey,
      }: {
        audioKey: AudioKey;
      }
    ) {
      const accentPhrases = state.audioItems[audioKey].query?.accentPhrases;
      if (accentPhrases)
        accentPhrases.map((elem) => {
          if (!elem.editorID)
            elem.editorID = uuidv4();
        });
    },
    action({ commit }, { audioKey }: { audioKey: AudioKey }) {
      commit("SET_ACCENT_PHRASES_EDITORID", { audioKey });
    },
  },

このようなactionとmutationを作成し、AudioDetail.vueの中で呼ぶように。

その他

動作的には問題なさそうですが、作成したSET_ACCENT_PHRASES_EDITORIDを呼び出す場所はここで良いでしょうか？
src/openapi/models/AccentPhrase.tsファイル内と迷いましたが、まずは実装が簡単そうだった方から選びました。

2024-02-27.15.17.16.mov

AccentPhraseを継承したEditorAccentPhraseという型を作成し、EditorAudioQueryのプロパティとした。 AudioDetail.vueファイルの中でaccentPhrases変数の変更のタイミングで割り振るように

Hiroshiba

プルリクエストありがとうございます！！
動作は完璧だと思います、素晴らしいです！！

動作に関しては完璧だったのですが、ちょっと将来的に課題になりそうな点に思い当たったので相談コメントを書きました 🙇

Hiroshiba · 2024-02-27T18:32:55Z

src/store/type.ts

+export interface EditorAccentPhrase extends AccentPhrase {
+  editorID?: string;
+}


すみません、ちょっとご相談です！
editorID?をオプショナルじゃなくするか、あるいはAudioDetail.vue内でだけeditorIDが現れる形にするか、どちらかに倒す必要があるかもです 🙇

以下詳細です：

editorID?とオプショナルにするの、なるほどです！！
ただこうすると、プロジェクトファイルを保存した時にこの ID が保存されたりされなかったりしちゃうかもです。

実際にオプショナルをやめる設計を考えていたのですが、実際試してみると膨大な量の変更が必要になりました 😇
実際に実装してみたのが↓です。まだmerge/splitしたときにアニメーションしてしまう問題が残ってます。
Hiroshiba#2

あるいは、どうにかしてAudioDetail.vue内でだけアクセント句ごとにeditorIDを持たせるのも良さそうです。
ただその方法ずっと考えてみてたんですが、思いつきませんでした････････。もし思いついたらぜひ知りたいです。。

ということで、2通りの方法どちらかできそうだったりしますか･･･？
前者は大変ですが、僕が書いたとこから続きをやっていただければ･･･！
後者の方は方法が思いついたらぜひお願いしたいです 🙇

長くなってしまってすみません、何かあれば気軽に聞いていただければ！！

ご提案ありがとうございます。
こちら了解しました！

後者は私の方でも検討してみますが、難しそうであればこのまま前者を実装してみます！

ありがとうございます！！
入り組み始めるかもなので、不明な点などあればお気軽にお聞きください！！

…ba/voicevox into stop-accentphrases-animation

P0ngCh4ng · 2024-02-29T23:42:11Z

これマージコミットって発生しても大丈夫ですか？
何も考えずにgit pull --rebaseしてしまった...

Hiroshiba · 2024-03-01T06:26:07Z

@P0ngCh4ng マージコミット大丈夫です！

ちなみにrebaseはgithubコメントとコミットの順番が変わるのでどちらかというとマージのが嬉しい感じです！（細かいのでどこにもこの案内書いてません🙇）
今回はまだ序盤なのでどっちでも大丈夫だったかなと！

P0ngCh4ng · 2024-03-05T01:56:55Z

先日いただいた修正をもとに、とりあえず同じようにaudioDetails.tsの中でactionを走らせてみました。
動作はしているように見えますが、課題点の解決には至っていますかね?

P0ngCh4ng · 2024-03-10T14:58:24Z

@Hiroshiba
こちらお手すきの際にご確認していただけると幸いです 🙏

Hiroshiba · 2024-03-12T15:58:31Z

@P0ngCh4ng おまたせしてしまっていて申し訳ありません 🙇 🙇 🙇
もう少しだけお待ちいただけると････ 🙇 🙇 🙇

Hiroshiba · 2024-03-18T18:38:24Z

すみませんお待たせしました！

先日いただいた修正をもとに、とりあえず同じようにaudioDetails.tsの中でactionを走らせてみました。動作はしているように見えますが、課題点の解決には至っていますかね?

課題点は解決していますが、実はちょっとだけ危ない点があるかもと思いました！

vueの中でUI操作以外（Vuex.stateのwatch）からVuexの状態を変更してる点です。
ここだけだったら把握しやすいかもですが、至るところでこのような操作がされてしまうと、どこで誰がどのデータをどう変更しているのかが複雑に入り組んでしまうかなと！
（ちなみにそういう複雑化を解消するために「データの流れを一方向にして分かりやすくしよう」とした設計思想がVuexが参考にしてるFlux思想･･･なはず！）

実際、テキスト欄を削除するとエラーが出るのを発見しました！
もちろん例外処理をすれば大丈夫ですが、いくつも重なるとかなり大変になっていくかも･･･？

そもそもmerge/splitするときにidを変更してしまっても問題なさそうなことに気づきました。
Hiroshiba#2 (comment)

merge/splitは多分この辺なのですが、ここでidを変更するように処理の書き換えをお願いする事ってできたりしますか･･･？ 🙇
https://github.com/P0ngCh4ng/voicevox/blob/d058e47dab9e4da2acda7c01aca7e8ef1d4a6338/src/store/audio.ts#L2224-L2271

何度も何度もすみません。。わからないとこがあれば何でも聞いてください！！ 🙇

P0ngCh4ng · 2024-03-19T01:34:29Z

了解しました
1から10まですみません 🙏
書き換えやってみます！

P0ngCh4ng · 2024-03-29T06:47:55Z

@Hiroshiba
完了したので確認いただいてもよろしいでしょうか？

Segu-g

方針は良いと思います！

修正が必要な点として、過去のバージョンで作られたプロジェクトファイルを読み込む際にkeyを追加するマイグレーションが書かれていません。project.tsにマイグレーションのコードがあるので追加をお願いします🙏

src/store/audio.ts

src/store/utility.ts

src/store/type.ts

src/store/proxy.ts

P0ngCh4ng · 2024-04-11T03:05:42Z

レビューありがとうございます！
ご指摘のところ修正します。

Segu-g

修正ありがとうございます！
project.ts の修正についてなのですが、このPR以前のバージョンで作られたプロジェクトファイルではkeyプロパティが含まれていないので、それを追加する処理を書いてください。
具体的にはproject.tsの他のマイグレーション処理の後に、他のマイグレーションと同じようにsemverでバージョンを判別しマイグレーションを実行するコードを書いてください。

if (semver.satisfies(projectAppVersion, "<0.17.0", semverSatisfiesOptions)) {
	... // アクセント句にkeyプロパティを追加する処理を書く
}

Segu-g · 2024-04-13T15:17:41Z

src/store/project.ts

@@ -298,7 +300,7 @@ export const projectStore = createPartialStore<ProjectStoreTypes>({
                    engineId,
                    styleId: audioItem.characterIndex,
                  })
-                  .then((accentPhrases: AccentPhrase[]) => {


申し訳ございません、説明不足でした！
このコードはvoicevoxの 0.5より前のバージョンで作られたプロジェクトファイルを編集している箇所なので、keyが含まれていないAccentPhraseのままで大丈夫だと思います！

P0ngCh4ng · 2024-04-13T17:25:43Z

ご指摘ありがとうございます。
現在の最新バージョンに合わせればいいでしょうか？

Segu-g · 2024-04-14T01:10:49Z

マイグレーションのコードは リリースするバージョン未満 で実行するべきなので、とりあえず <0.19.0 の条件で書いておけばいいと思います。
また、コンフリクトが発生しているようなのでその解消もお願いします。

@Hiroshiba マイグレーションのバージョン判定を埋め込んでいるのでメンションしておきます。

P0ngCh4ng · 2024-05-02T07:15:21Z

コンフリクトの解消の方法については、

mainをPRブランチに取り込み
ローカルでコンフリクトを解消
マージコミットを作成

こちらの方法で問題ないでしょうか？

日にちだいぶ空いてしまいましたが、GW中は時間があるので、作業再開します 🙇

Segu-g · 2024-05-02T07:27:47Z

@P0ngCh4ng
はい！その方法で大丈夫です！

また、先日0.19.0がリリースされたため、バージョン判定は'<0.20.0'でよろしくお願いします！

P0ngCh4ng · 2024-05-05T09:40:39Z

@Segu-g
ご返信ありがとうございます！
コンフリクトの修正と、マイグレーションコードの作成を行いましたが、
・マージ直後にもかかわらず、linterとfomtterを起動させると大量の変更が発生する
・src/store/project.tsの93行目に発生したエラーの解決ができない

これらの問題が残っています。レビューしていただいてもよろしいでしょうか？
よろしくお願いします。

sevenc-nanashi · 2024-05-05T12:16:26Z

・マージ直後にもかかわらず、linterとfomtterを起動させると大量の変更が発生する

#1992 かも？多分フォーマットかければ治ると思います

P0ngCh4ng · 2024-05-05T13:08:29Z

npm ciをかけなおしたところ、想定通りの動作になりました。

Segu-g

src/store/project.tsの93行目に発生したエラーの解決ができない

すみません、エラーの件見逃してました！
ここのエラーはプロジェクトファイルの型定義である LatestProjectType の更新のし忘れによって起こっているエラーです。

src\domain\project\schema.ts にある accentPhraseSchema に key: accentPhraseKeySchema を追加すれば良いと思います！

src/store/project.ts

accentPhraseKeySchema追加し忘れ EditorAccentPhraseからAccentPhraseへ戻し忘れ

P0ngCh4ng · 2024-05-08T07:35:13Z

エラーの修正完了しました！
お手隙の際レビューお願いします🙇

Segu-g · 2024-05-11T14:48:08Z

すみません！レビューが遅くなってしまいました🙇‍♂️

PRの修正内容は問題ないと思います！LGTMです！
申し訳ないのですが、またConflictが発生してしまったので修正してもらえると!!🙇‍♂️

add AccentPhraseに個別idを設定

7606c95

AccentPhraseを継承したEditorAccentPhraseという型を作成し、EditorAudioQueryのプロパティとした。 AudioDetail.vueファイルの中でaccentPhrases変数の変更のタイミングで割り振るように

P0ngCh4ng requested a review from a team as a code owner February 27, 2024 06:22

P0ngCh4ng requested review from Hiroshiba and removed request for a team February 27, 2024 06:22

AccentPhrase全てにidを振ろうとした

4747be3

Hiroshiba mentioned this pull request Feb 27, 2024

AccentPhrase全てにidを振ろうとした Hiroshiba/voicevox#2

Open

Hiroshiba reviewed Feb 27, 2024

View reviewed changes

P0ngCh4ng and others added 2 commits March 1, 2024 08:05

Merge branch 'VOICEVOX:main' into stop-accentphrases-animation

f3f7e35

Merge branch 'AccentPhrase全てにidを振ろうとした' of https://github.com/Hiroshi…

af7799e

…ba/voicevox into stop-accentphrases-animation

change editorID用に書いたコードをEditorKeyに対応させた

d058e47

refactor accentKeyを書き換える処理をaudio.tsの中に

211986d

Segu-g requested changes Apr 6, 2024

View reviewed changes

src/store/audio.ts Outdated Show resolved Hide resolved

src/store/audio.ts Show resolved Hide resolved

src/store/utility.ts Outdated Show resolved Hide resolved

src/store/type.ts Outdated Show resolved Hide resolved

src/store/proxy.ts Outdated Show resolved Hide resolved

P0ngCh4ng added 2 commits April 11, 2024 13:54

refact 不要なコードや引数の方を変更

7c721ef

faet プロジェクトファイルからインポートするときにkeyを追加するマイグレーションを追加

0cec632

Segu-g requested changes Apr 13, 2024

View reviewed changes

Merge branch 'main' into stop-accentphrases-animation

7dc130c

P0ngCh4ng requested a review from a team as a code owner May 5, 2024 08:40

P0ngCh4ng requested review from Hiroshiba and removed request for a team May 5, 2024 08:40

<0.20.0 のマイグレーションを作成

d503d4f

フォーマットの掛け直し

b4fff84

Segu-g requested changes May 5, 2024

View reviewed changes

src/store/project.ts Outdated Show resolved Hide resolved

src/store/project.ts Outdated Show resolved Hide resolved

[fix] エラーの修正

c0589e7

accentPhraseKeySchema追加し忘れ EditorAccentPhraseからAccentPhraseへ戻し忘れ

P0ngCh4ng and others added 3 commits May 16, 2024 11:30

Merge branch 'main' into stop-accentphrases-animation

c98f46a

merge main into stop-accentphrases-animation

e9d3e99

インポートの順番の問題を修正

c60cc6b

Segu-g approved these changes May 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add AccentPhraseに個別idを設定 #1879

add AccentPhraseに個別idを設定 #1879

P0ngCh4ng commented Feb 27, 2024

Hiroshiba left a comment •

edited

Hiroshiba Feb 27, 2024

P0ngCh4ng Feb 29, 2024

Hiroshiba Feb 29, 2024

P0ngCh4ng commented Feb 29, 2024

Hiroshiba commented Mar 1, 2024

P0ngCh4ng commented Mar 5, 2024

P0ngCh4ng commented Mar 10, 2024

Hiroshiba commented Mar 12, 2024

Hiroshiba commented Mar 18, 2024 •

edited

P0ngCh4ng commented Mar 19, 2024

P0ngCh4ng commented Mar 29, 2024

Segu-g left a comment

P0ngCh4ng commented Apr 11, 2024

Segu-g left a comment

Segu-g Apr 13, 2024

P0ngCh4ng commented Apr 13, 2024

Segu-g commented Apr 14, 2024 •

edited

P0ngCh4ng commented May 2, 2024

Segu-g commented May 2, 2024

P0ngCh4ng commented May 5, 2024

sevenc-nanashi commented May 5, 2024

P0ngCh4ng commented May 5, 2024

Segu-g left a comment •

edited

P0ngCh4ng commented May 8, 2024

Segu-g commented May 11, 2024

add AccentPhraseに個別idを設定 #1879

Are you sure you want to change the base?

add AccentPhraseに個別idを設定 #1879

Conversation

P0ngCh4ng commented Feb 27, 2024

内容

関連 Issue

その他

Hiroshiba left a comment • edited

Choose a reason for hiding this comment

Hiroshiba Feb 27, 2024

Choose a reason for hiding this comment

P0ngCh4ng Feb 29, 2024

Choose a reason for hiding this comment

Hiroshiba Feb 29, 2024

Choose a reason for hiding this comment

P0ngCh4ng commented Feb 29, 2024

Hiroshiba commented Mar 1, 2024

P0ngCh4ng commented Mar 5, 2024

P0ngCh4ng commented Mar 10, 2024

Hiroshiba commented Mar 12, 2024

Hiroshiba commented Mar 18, 2024 • edited

P0ngCh4ng commented Mar 19, 2024

P0ngCh4ng commented Mar 29, 2024

Segu-g left a comment

Choose a reason for hiding this comment

P0ngCh4ng commented Apr 11, 2024

Segu-g left a comment

Choose a reason for hiding this comment

Segu-g Apr 13, 2024

Choose a reason for hiding this comment

P0ngCh4ng commented Apr 13, 2024

Segu-g commented Apr 14, 2024 • edited

P0ngCh4ng commented May 2, 2024

Segu-g commented May 2, 2024

P0ngCh4ng commented May 5, 2024

sevenc-nanashi commented May 5, 2024

P0ngCh4ng commented May 5, 2024

Segu-g left a comment • edited

Choose a reason for hiding this comment

P0ngCh4ng commented May 8, 2024

Segu-g commented May 11, 2024

Hiroshiba left a comment •

edited

Hiroshiba commented Mar 18, 2024 •

edited

Segu-g commented Apr 14, 2024 •

edited

Segu-g left a comment •

edited