fix(gen): auto-persist sprite gen when content phase starts#3027
fix(gen): auto-persist sprite gen when content phase starts#3027xgopilot[bot] wants to merge 1 commit intodevfrom
Conversation
Previously, ongoing sprite generation was only added to editor generation state (and thus saved with the project) when the user clicked Minimize. This meant the task was lost if the project was saved before the user ever clicked Minimize. Now, the gen is automatically registered into project generation state as soon as contentPreparingState.status becomes 'finished', so save/load correctly preserves the task without requiring a prior Minimize action. The Minimize button still triggers keepAlive so the gen survives modal close, but persistence no longer depends on it. Adds test coverage for save/reload after content phase without Minimize. Generated with [codeagent](https://github.com/qbox/codeagent) Co-authored-by: nighca <1492263+nighca@users.noreply.github.com>
There was a problem hiding this comment.
The fix is conceptually correct and the hasBeenAddedToGenState guard logic is sound for preventing double-registration. A few issues worth addressing before merge:
handleModalClose dialog (not in diff, but affected by this change): The confirmation dialog shown when a user closes the modal via the X button currently warns "Current progress will not be saved." when props.gen == null. After the watcher fires and auto-registers the gen, this warning is incorrect — the gen has already been persisted. The guard should also check !hasBeenAddedToGenState.value to suppress the confirmation once auto-registration has occurred.
Test coverage gap: The new test (should persist sprite gen that entered content phase without Minimize) calls genState.addSprite(gen) directly — it bypasses the watcher in SpriteGenModal entirely and only exercises the pre-existing export/load round-trip. The actual bug path (watcher firing → genCollapseHandler called → addSprite) has no test coverage. A component-level test (or a unit test with a mock genCollapseHandler) is needed to verify the new watcher behavior.
| if (gen == null) return | ||
| hasBeenAddedToGenState.value = true | ||
| props.genCollapseHandler(gen, true) | ||
| } |
There was a problem hiding this comment.
Bug: keepAlive is not called here. In handleGenCollapse, keepAlive(gen) is called before registering the gen to prevent internalGen's cleanup handler from calling gen.cancel() / gen.dispose() on modal close. This watcher path skips that, so if the user closes the modal after the watcher fires but before clicking Minimize, the gen is disposed while still registered in genState.
Also, the Promise returned by genCollapseHandler is not awaited and has no .catch(). If the handler rejects, hasBeenAddedToGenState stays true but the gen was never actually added — there is no recovery path.
Suggested fix:
hasBeenAddedToGenState.value = true
keepalive(gen)
await props.genCollapseHandler(gen, true).catch(() => {
hasBeenAddedToGenState.value = false
})| const hasBeenAddedToGenState = ref(false) | ||
|
|
||
| // Auto-register the gen into editor genState when it enters content phase, | ||
| // so it is persisted with the project without requiring a Minimize action. |
There was a problem hiding this comment.
Slight inaccuracy: the watcher fires when status === 'finished', which is the end of the content-preparing phase (i.e., the gen has transitioned into the content phase, not begun preparing). Consider: // Auto-register the gen into editor genState when content-preparing finishes,
| const genState = new GenState(i18n, project) | ||
|
|
||
| // Create a gen that has entered the content phase. | ||
| // This simulates the auto-persist watcher in SpriteGenModal firing when |
There was a problem hiding this comment.
This comment is misleading: the test does not simulate the SpriteGenModal watcher at all. It calls genState.addSprite(gen) directly with a pre-constructed SpriteGen that already has status: 'finished'. What is actually being tested is that GenState can export and reload a gen in the finished-content-phase state — which is pre-existing behavior, not the new watcher logic.
|
See #3033 |
Requested by @nighca
Fixes #3024
Summary
watchinSpriteGenModal.vuethat fires whencontentPreparingState.statusbecomes'finished'and auto-registers the gen into the editor'sgenState(viagenCollapseHandler) — no Minimize click requiredhandleGenCollapseto skip re-registering the gen if it was already auto-registered, preventing double-addgen.test.tsverifying that a sprite gen in content phase survives a save/load cycle without any prior Minimize actionRoot cause
The gen was previously only added to
genStateinsidehandleGenCollapse(the Minimize path). SincegenState.export()is what drives project persistence, any gen that hadn't been through Minimize would be omitted from saved project files.How the fix works
SpriteGenModal.vuenow watchesactiveGen.value?.contentPreparingState.status. When it transitions to'finished'and the gen is newly created (not passed via props), it immediately callsprops.genCollapseHandler(gen, true)to add the gen togenState. AhasBeenAddedToGenStateflag prevents duplicate registration if the user later also clicks Minimize.