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

Physicist does not appear on repeat visit to north wing #15

Open
rmkubik opened this issue Jul 14, 2022 · 3 comments · Fixed by #17
Open

Physicist does not appear on repeat visit to north wing #15

rmkubik opened this issue Jul 14, 2022 · 3 comments · Fixed by #17
Labels
bug Something isn't working

Comments

@rmkubik
Copy link
Contributor

rmkubik commented Jul 14, 2022

This is a bug reported in Discord:

is it normal to have the js test lab broken and unable to rematerialize the physicist in the north wing in the new version after completing it in a previous version?

Something very similar (or just this same bug) was reported last year in discord too:

I had completed it before the big update, so all of the hacking missions are done, even though some of the story stuff hadn't happened/wasn't complete
Everything was fine until I went to the north, where the two terminals are supposed to reconstitute the scientist--I tried to "complete" it (i.e. I went to the terminals and pressed the "HACK" button on both) but he remains disappeared

We discuss this in Discord in this thread.

and it makes me redo all the lasers... i did laser one the first time, then 2 and 3, but 4 is the physicist, so i can't finish 4 and go back to 1 to fix the ducktypium black hole thing or what that is it created in the middle
i don't have to redo the code or anything, but i have to go to each and click hack for it to proceed
even tho i did those missions long ago

Problem

The users beat the JS Test Lab in an older version. On return visit to the JavaScript Test Lab, the physicist does not rematerialize. The lasers around the central pit were deactivated.

Questions

  • Is this a larger issue with saving state in the JavaScript Test Lab? Perhaps some of the state for these things happens in an async/promise/callback and doesn't get saved to disk correctly.
  • Is this still a possible issue to re-produce? It's possible that this only happened on a previous version of the JS Test Lab and this is now fixed.
  • Related, did I ship a fix for this in a previous version? We should probably find some evidence of this in the commit log. However, I think it would be the twilioquest commit log since this bug was reported before the authoring tools split out all the content.
@rmkubik rmkubik added the bug Something isn't working label Jul 14, 2022
@rmkubik
Copy link
Contributor Author

rmkubik commented Jul 22, 2022

I think this I've found the issue.

Issue

The summoning animation is scheduled here.

This function makes use of an async function in a forEach loop to tween the camera to the physicist.

This async function modifies worldState values after awaiting. Because this function is async though, these changes are not getting saved with the world.setState call at the end of events.js.

Fix

To resolve this, the modification of a worldState value in the async function should be converted to something like:

const worldState = world.getState('js-world-state-key');
worldState.northWing.summonAnimFinished = true;
world.setState('js-world-state-key');

This should allow TQ to actually persist the correct worldState values.

This MIGHT have additional side effects though since we cannot guarantee if this worldState change will take affect in the appropriate place in the rest of the events.js flow. It seems likely that won't be an issue based on a cursory skim of the rest of the event scripting files. We don't seem to rely on this value anywhere else in the same execution flow of events.js.

Migration

Players who have encountered this bug can no longer re-trigger the summon animation since they have a truthy value for worldState.northWing.summonAnimStarted in the main summon trigger conditional.

This value is set synchronously here in the scheduleSummonAnim function.

To allow players to re-trigger this animation and actually summon the scientist (and save that data after the fix above) I think we could check for summonAnimFinished instead of summonAnimStarted on line 84 of events.js.

This MAY accidentally trigger on a later level if you trigger an objectiveDidClose event in a different level. I think this might be fine though since the world.forEachEntities function that searches for physicistViewpoint on line 9 of scheduleSummonAnim will just get no hits and not invoke it's async handler.

@rmkubik
Copy link
Contributor Author

rmkubik commented Jul 28, 2022

Okay, so I wasn't actually able to reproduce this issue the way I expected to above.

Letting the summon animation resolve and then quitting the game or leaving the map does not seem to lead to the softlock state.

I was able to trigger the softlock my quitting the game after the animation started playing but before it resolved. It seems unlikely that this is what the folks who've gotten stuck in this state were doing.


I'm no longer confident in my proposed initial solve for this issue. It doesn't seem like the synchronous nature of setting the worldState is the root cause. Though, I'm not quite sure why that is not also a problem at the moment.

I do think the proposed migration fix should hopefully allow users caught in this situation to re-trigger the animation and get out of their soft lock situation. This might have the same issue as the dev fundamentals telescope in allowing the player to initiate multiple animations at once.

@rmkubik
Copy link
Contributor Author

rmkubik commented Jul 28, 2022

I am still concerned about the underlying cause of this issue. #17 allowed users to get out of the softlock in the North Wing, but I did not successfully identify the root cause of this issue.

@rmkubik rmkubik reopened this Jul 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant