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

Fix デフォルトプリセットの再登録が機能してないバグ(#1996)を修正 #2053

Merged
merged 7 commits into from
May 6, 2024

Conversation

madosuki
Copy link
Contributor

@madosuki madosuki commented May 3, 2024

内容

  • GET_ALL_VOICESをUSER_ORDERED_CHARACTER_INFOSのようにしてトークスタイルのみでデフォルトプリセットを生成するようにしました。
  • config.jsonのdefaultPresetKeysのkeyにスタイルIDが記録されているのでそれを使ってプリセットKeyを取得して記録されているプリセットからハミング/ソングの物を削除する処理をConfigManagerのマイグレーション機能ところに実装しました。

マイグレーションのところのバージョン指定、とりあえず0.21にしてあります。動作確認は999.999.999にして行いました。

関連 Issue

close #1996

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

その他

@madosuki madosuki requested a review from a team as a code owner May 3, 2024 15:27
@madosuki madosuki requested review from Hiroshiba and removed request for a team May 3, 2024 15: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!!!

実装ありがとうございます、とても良い実装だと思いました!!!
実際に動かしてみてプリセットがちゃんと消えてそうなことを確認しました!

ちょっとコメントの追加と関数の移動だけこちらでしてからマージさせていただこうと思います!!

あ、あとPRの名称も文脈わからなくてもわかるようにさせていただきます!

src/backend/common/ConfigManager.ts Outdated Show resolved Hide resolved
src/backend/common/ConfigManager.ts Outdated Show resolved Hide resolved
src/backend/common/ConfigManager.ts Outdated Show resolved Hide resolved
src/backend/common/ConfigManager.ts Show resolved Hide resolved
@Hiroshiba Hiroshiba changed the title Fix #1996 Fix デフォルトプリセットの再登録が機能してないバグ(#1966)を修正 May 6, 2024
@Hiroshiba Hiroshiba changed the title Fix デフォルトプリセットの再登録が機能してないバグ(#1966)を修正 Fix デフォルトプリセットの再登録が機能してないバグ(#1996)を修正 May 6, 2024
@Hiroshiba
Copy link
Member

マージします!

テストがあるとなお安心できるかなと思い、issueを作ってみました!
#2062
(バグってるconfig.jsonを作る部分だけやっておきました。)

@madosuki テスト周りの実装とかもしご興味あれば・・・!!

@Hiroshiba
Copy link
Member

Hiroshiba commented May 16, 2024

@madosuki ちょっと突発的なのですがご相談が!
ポストにXアカウントを言及させていただいても良いでしょうか? 🙇

ニーズのある機能が実装されたときにSNSで言及しておりまして、今回のプルリクエストもツイートしたいと思っています。
https://twitter.com/search?q=%23VOICEVOX%E9%96%8B%E7%99%BA%E7%8A%B6%E6%B3%81
もしよかったらそこで @madosuki さんのXアカウントをツイート文に含めて紹介させていただきたいのですが、いかがでしょうか・・・?

(RTされたりリプライで届く感謝の言葉をお届けできればという意図と、あと開発者が分かるので新規コミッターの方がOSS開発に興味を持ってくださる導線を増やせればという意図があります・・・!)

こんな感じを予定しています・・・!

#VOICEVOX開発状況 
デフォルトプリセットが再登録できないバグが直りました🎉(今後のアップデートで実装されます。)
【開発者: @madosuki】
https://github.com/VOICEVOX/voicevox/pull/2056

(ちなみにアカウントあってますか・・・?)

@madosuki
Copy link
Contributor Author

madosuki commented May 17, 2024

紹介問題ありません。
アカウントはそれで合っています。

@madosuki madosuki deleted the fix/duplicate-default-presets branch July 5, 2024 15:26
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