Skip to content

Conversation

Hiroshiba
Copy link

の変更提案プルリクです!

mergeしていただければ自動的に元のプルリクに反映されるはずです。
もちろんmergeせず自由に変更いただいても問題ないです!

Copy link
Author

@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.

ちょっと解説コメントをしてみました!

@@ -160,6 +160,7 @@ onMounted(async () => {
isAcceptTermsDialogOpen:
import.meta.env.MODE !== "development" &&
store.state.acceptTerms !== "Accepted",
isInitialSettingsDialogOpen: store.state.openedEditor == undefined,
Copy link
Author

@Hiroshiba Hiroshiba Dec 29, 2024

Choose a reason for hiding this comment

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

ここでopenedEditorが設定されているかどうかを見て、isInitialSettingsDialogOpenをtrueにしてます!

App.vueが表示されている直後のタイミングだと必ずstate.openedEditorundefinedになってるので、エンジンが起動した後にダイアログを開くかどうかを判定してます。
利用規約ダイアログとかも開くかどうかここで判定しているので、同じ機構にしています。

(ちなみに、これは正直既存の実装がちょっと微妙なのですが、リファクタリングができていなくてそのままにって感じです。。。)

Comment on lines +150 to +154
!store.state.isAcceptTermsDialogOpen &&
!store.state.isCharacterOrderDialogOpen &&
!store.state.isDefaultStyleSelectDialogOpen &&
!store.state.isAcceptRetrieveTelemetryDialogOpen &&
store.state.isInitialSettingsDialogOpen,
Copy link
Author

Choose a reason for hiding this comment

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

利用規約ダイアログ、キャラクター並び替えダイアログなどを設定し終わってからこのダイアログが開かれるようにしてます。
UX次第だけど、順番は変えてもいいかも。

openedEditor: z.enum(["talk", "song"]).default("talk"),
openedEditor: z.enum(["talk", "song"]).optional(),
Copy link
Author

Choose a reason for hiding this comment

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

openedEditorはconfig.json設定においてデフォルトでtalkになるようになっていたのですが、デフォルトがundefinedになるようにしてます。
これで今まで開いたことがある人には「どっちにしますか?」ダイアログが開かれず、初めて起動した人はundefinedになってダイアログが開かれるはず!

@jdkfx jdkfx merged commit 6f7c945 into jdkfx:feature/#2011 Jan 20, 2025
5 of 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.

2 participants