-
Notifications
You must be signed in to change notification settings - Fork 312
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-s] Phraseをストアで持つようにする #1663
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
いろいろコメントしていますが、このPRでマストで変更がいる感じではないのでそのままマージさせていただきます!
type PhraseData = { | ||
blob?: Blob; | ||
source?: Instrument | AudioPlayer; // ひとまずPhraseDataに持たせる | ||
sequence?: Sequence; // ひとまずPhraseDataに持たせる | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
おそらくこのデータは、Vuex内で閉じていて他で使ってほしくない値というニュアンスなのかなと思いました!
ローカル変数の概念が無いのでそう分けられたのかなと。
となると、コメントでこの型の説明付けておくとニュアンス伝わりやすいかもです。こんな感じとか?
type PhraseData = { | |
blob?: Blob; | |
source?: Instrument | AudioPlayer; // ひとまずPhraseDataに持たせる | |
sequence?: Sequence; // ひとまずPhraseDataに持たせる | |
}; | |
// Phraseのstore内用データ | |
type PhraseData = { | |
blob?: Blob; | |
source?: Instrument | AudioPlayer; // ひとまずPhraseDataに持たせる | |
sequence?: Sequence; // ひとまずPhraseDataに持たせる | |
}; |
const allPhrases = new Map<string, Phrase>(); | ||
const phraseDataMap = new Map<string, PhraseData>(); | ||
const phraseAudioBlobCache = new Map<string, Blob>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PhraseDataにもblobがあって、それとは別にphraseAudioBlobCache
もあるのでちょっとややこしいかもと思いました!
commit("SET_AUDIO_QUERY_TO_PHRASE", { phraseKey: key, audioQuery }); | ||
|
||
const startTime = calcStartTime(phrase.score, audioQuery); | ||
commit("SET_START_TIME_TO_PHRASE", { phraseKey: key, startTime }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
audioQueryとstartTimeは片方が存在すればもう片方は必ず存在する気がするので、それを型に入れてあげるとundefinedチェック用のifを二回実行しなくて済むかもと思いました!
つまり、こうでも良いかも、と。
type Phrase = {
otherData?: {
audioQuery: AudioQuery,
startTime: number
}
}
まあこのあたり今はどういう形が最高かわからないのですが、こういう考え方もあるよという感じで・・・!
内容
Phrase
をストア(VuexのState)で持つようにし、ストアで持てないBlob
などはPhraseData
で持つようにします。また、以下も行います。
Phrase
のstartTime
の計算をAudioQuery
生成・編集時に行うようにするAudioQuery
のハッシュを計算・比較する処理を一旦削除exportingAudio
をnowAudioExporting
に変更EXPORT_WAVE_FILE
にレンダリング未完了の場合の処理が残っていたので削除関連 Issue
VOICEVOX/voicevox_project#15
その他