-
Notifications
You must be signed in to change notification settings - Fork 297
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
BUGFIX: Close deleted scripts on install. #1113
base: dev
Are you sure you want to change the base?
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.
I think this change makes sense, but it'll need good testing because this whole area is a bunch of edge cases.
What edge cases are there? This seems like a simple enough function to me, it closes open scripts from deleted servers and updates the currently open script accordingly. The behavior has also not changed at all except for what it does on prestige (install/soft reset/backdooring wd). EDIT: Not trying to argue this doesn't need to be tested, just not sure if it's as complicated as you said. |
I was referring to the game itself, not so much this code. The bug this is fixing is an edge case, and there are just a variety of cases of "you do/don't have certain windows open at certain times" that should be checked. Off the top of my head, the full matrix of:
looked at across installs (or some other reset, they should all act the same), and also whatever it is that triggers the other codepath that calls your function |
The other trigger |
e8d67ec
to
94a60a2
Compare
Note to possibly future self: Based on Discord history, this might be on hold pending Monaco shenanigans discovered during testing |
There was some disagreement on if this fix is necessary, see the related issue for details.
fixes #1050