Backmerge hotfixes #292
Merged
Backmerge hotfixes #292
Conversation
This file contains 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
### Request For Comment Hotfix to master, will require back-merge to develop. Apologies for wall of text. This hotfixes at least one cause of black screen (seen at least in ~2~ ~3~ ~4~ 5 player reports) and adds a best-effort `try/finally` to save end-of-raid progress. **Hotfix** See stacktrace below. Basically, one UI element (trader rep section) wants to get called when player levels up (like at the end of a raid) to update its "next trader level requirements" UI. Thing is, that piece of UI is not around anymore/yet at the end of the raid, so it crashes. I'm not sure why the hook does not get properly unregistered (presumably on scene unload). I hate having to use a patch for that, but the hook is so deep, I haven't found a better approach. Open to suggestions. Note that issues with black screen at end-of-raid have been reported in vanilla and SPT as well. **Best-effort save end-of-raid progress** On top of fixing the above, I added a try/finally around the end-of-raid code (C1) to make sure we make the network call to the Aki backend to save progression. Two worries about doing that if an exception is thrown in C1: - some important code could have been skipped, maybe some profile-related stuff, so there's a risk that what gets sent to Aki is not what we want, depending on what `Player.OnGameSessionEnd`, `CoopSITGame.CleanUp` and `BackEndSession.OfflineRaidEnded` do as of today, and in the future - the client probably needs to be restarted because a lot of UI consistency/cleanup stuff is happening in C1, like unregistering hooks, etc. even if the progression properly gets saved. **Choices to make** @paulov-t 1) regarding saving progression, do we really want to do a best-effort to make the backend request? On the one hand, it's unlikely that profiles get corrupted from that, and hopefully people make backups (or the launcher could do it daily). On the other hand, if an end-of-raid logic exception is thrown, we now have nice logs sent by users + a hotfix-friendly master/develop branch system so we could just get a fix out quickly. Both are fine with me, the latter has less catastrophic failure modes and avoids UI consistency issues. 2) if we do go with the best-effort try/finally and an exception is thrown, should we display a fatal error popup explaining what happened, or just hope that the UI isn't too wonky even without the proper cleanup? Think popup would be the way to go. Error was: ``` NullReferenceException at (wrapper managed-to-native) UnityEngine.Component.get_gameObject(UnityEngine.Component) at EFT.UI.UIElement.ShowGameObject () [0x00000] in <305d2033b20e43a1983675b0792a75c5>:0 at EFT.UI.RankPanel.Show (System.Int32 rankLevel, System.Int32 maxRank) [0x00000] in <305d2033b20e43a1983675b0792a75c5>:0 at TraderCard.method_0 () [0x0005a] in <305d2033b20e43a1983675b0792a75c5>:0 at (wrapper delegate-invoke) <Module>.invoke_void() at (wrapper dynamic-method) EFT.Profile+TraderInfo.DMD<EFT.Profile+TraderInfo::UpdateLevel>(EFT.Profile/TraderInfo) at EFT.Profile+TraderInfo.method_1 (System.Int32 _, System.Int32 __) [0x00000] in <305d2033b20e43a1983675b0792a75c5>:0 at (wrapper delegate-invoke) System.Action`2[System.Int32,System.Int32].invoke_void_T1_T2(int,int) at ProfileInfo.set_Experience (System.Int32 value) [0x00054] in <305d2033b20e43a1983675b0792a75c5>:0 at AStatisticsManagerForPlayer.EndStatisticsSession (EFT.ExitStatus exitStatus, System.Single pastTime) [0x00181] in <305d2033b20e43a1983675b0792a75c5>:0 at EFT.Player.OnGameSessionEnd (EFT.ExitStatus exitStatus, System.Single pastTime, System.String locationId, System.String exitName) [0x0005d] in <305d2033b20e43a1983675b0792a75c5>:0 at StayInTarkov.Coop.SITGameModes.CoopSITGame+<>c__DisplayClass96_0.<Stop>b__0 () [0x00042] in <88c5c3ff81d84133ad3383979cbeddd0>:0 at EFT.UI.PreloaderUI+Class2571.MoveNext () [0x000a9] in <305d2033b20e43a1983675b0792a75c5>:0 at UnityEngine.SetupCoroutine.InvokeMoveNext (System.Collections.IEnumerator enumerator, System.IntPtr returnValueAddress) [0x00026] in <ca21460feb9c47d0ac337b9893474cc6>:0 ```
Co-Authored-By: Mihai <[email protected]>
Hotfix for now, but the whole extract+quit out logic will need to be simplified/unified in the next major release. Sequence of events triggering the bug: - guest extracts (regardless of exfil type/map) - guest's collider is disabled so that existing players do not get bodyblocked - no more collision with exfil point means `OnCancelExtraction` is called, which has new "runner" detection logic, setting the exit status to mia (restoring "default" value) - host quits (i.e. pressed F8) - host's `profileId` is before guest's `profileId` in the `Players` list - host "kicks" all players including itself before it "kicks" the guest - nodejs server sees host left, sends a "everybody leave now!" message to all remaining connected players, including guest - guest leaves without recomputing its exit status, which was set to MIA by `OnCancelExtraction` above - player sad
dev2master
- got player report that it was still broken - rewrote the fix after lots of testing - looked at SPT for something else - saw they had the exact same fix - cried a little - imported their stuff with proper credit to make sure we can stay up-to-date and even contribute back
|
Build Successful! You can find a link to the downloadable artifact below. |
paulov-t
approved these changes
Apr 30, 2024
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.
No description provided.