phetsims / vegas

Reusable game components for PhET simulations.
MIT License
1 stars 4 forks source link

GameTimer freezes sim on perfect score across multiple sims #126

Closed Luisav1 closed 4 months ago

Luisav1 commented 4 months ago

While working on https://github.com/phetsims/balancing-act/issues/63, I observed an unexpected behavior related to the GameTimer functionality in Balancing Act.

Upon achieving a perfect score in a game while the GameTimer is active, the simulation freezes unexpectedly. I found that this issue is not isolated to Balancing Act only, but rather affects multiple simulations using a game timer, these simulations include:

Additionally, there have been instances where the issue occurs even with imperfect scores, although this is much less frequent.

The assertion message triggering the freeze is: Assertion failed: accessed value outside of dependency tracking. The issue happens in line 90 specifically in GameTimer when accessing the value of the localized string time pattern: VegasStrings.pattern[ '0minutes' ][ '1secondsStringProperty' ].value.

pixelzoom commented 4 months ago

Upon achieving a perfect score in a game while the GameTimer is active, the simulation freezes unexpectedly.

The DerivedProperty that was missing dependencies was in LevelCompletedNode.ts, fixed in https://github.com/phetsims/vegas/commit/857cce097b483e1c716fd6f8f19476366cfce277. @Luisav1 please verify.

Additionally, there have been instances where the issue occurs even with imperfect scores, although this is much less frequent.

@Luisav1 I could not located any other places where GameTimer.formatTime dependencies are missing. It's probably some other dependency that's missing. Can you provide specific examples, preferrably with a stack trace?

pixelzoom commented 4 months ago

One more note... Assertions are not enabled in published versions, so this error will not be encountered. It's also impossible to change the locale while LevelCompletedNode is visible, since it's effectively a modal dialog. So I do not feel that this requires an MR, and does not need to be cherry picked for https://github.com/phetsims/graphing-lines/issues/142 (Graphing Lines 1.4).

pixelzoom commented 4 months ago

Since there are other commits to cherry-pick for https://github.com/phetsims/graphing-lines/issues/142, I'll cherry-pick this one too: https://github.com/phetsims/vegas/commit/857cce097b483e1c716fd6f8f19476366cfce277.

@Luisav1 please assign this issue back to me after verifying.

pixelzoom commented 4 months ago

I went ahead and cherry picked https://github.com/phetsims/vegas/commit/857cce097b483e1c716fd6f8f19476366cfce277 to graphing-lines 1.4 and graphing-slope-intercept 1.2.

Remaining work for @Luisav1:

Luisav1 commented 4 months ago

I encountered a freeze once in one of the simulations with an imperfect score, but unfortunately, I can't remember which one. I've played through all those simulations again without encountering any further issues, whether with perfect or imperfect scores. Apologies for not providing the original stack trace. However, I can confirm that the fix for perfect scores is working well.

Though the issue with imperfect scores occurred only once, I believe it’s not a major concern. Assigning this back to you, @pixelzoom, for a final call.

pixelzoom commented 4 months ago

I think it's safe to close this issue. If we encounter a failure with imperfect scores, we can create a new issue.