Stop stock toolbar buttons being duplicated on scene switch - #426
Open
djungelorm wants to merge 1 commit into
Open
Stop stock toolbar buttons being duplicated on scene switch#426djungelorm wants to merge 1 commit into
djungelorm wants to merge 1 commit into
Conversation
An app now gets one trip to the toolbar at a time, and only while it is not already on it. A genuine second instance still destroys itself as it always did.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When switching game scenes quickly, stock toolbar icons can become duplicated. This patch fixes that.
UIApp.AddToAppLauncher()keeps the button it is given in a single field,appLauncherButton, whichUIApp.OnDestroy()uses to give it back. Nothing stops an app going through it twice:onGUIApplicationLauncherReadyfires more than once for a scene loaded while another is still being torn down, and since the coroutine waitsAppStartFrameDelayframes before doing anything, both runs are in flight before either has a button. The second overwrites the field, soOnDestroygives back only the second button and the first is left on the toolbar with nothing that can remove it.The apps' own duplicate checks compound it. All of them (
ResourceDisplay,ActionGroupsApp,ContractsApp,MessageSystem,KnowledgeBase,MissionsAppandApplicationLauncher) testInstance != nullrather thanInstance != this, andAddToAppLauncherrunsOnAppInitialized, so an app added twice finds the instance it set itself and destroys itself. Its singleton is left pointing at a destroyed object, which causes a flood ofNullReferenceExceptionin the log.With this patch, an app now gets one trip to the toolbar at a time, and only while it is not already on it. A genuine second instance still destroys itself exactly as before, and an app whose button went with a torn-down launcher is still free to add itself again.
Tested in KSP 1.12.5 over repeated scene switches:
ResourceDisplaybuttons went from growing by two per entry into flight (past ten and still climbing) to peaking at two and settling at one, and theKSP.UIexceptions above went from ~15 per scene change to none.Found via the kRPC mod, to drive rapid scene changes and so hits this on nearly every transition, but can accumulate in ordinary play too.