Fix Potential Hang in AnimatedPart.SetFrameWrap #1116
Merged
Conversation
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
twpol
pushed a commit
that referenced
this pull request
Jun 27, 2025
- Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1107 at f5eb3dc: Fix DPMode when last remote is moved to front. - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1114 at 9a4a873: Fix color of DB in DrivingInfo when in setup with DPU fenced. - Pull request #1115 at 01c0838: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1104 at f9691cf: Handle simple adhesion within the axle module - Pull request #1081 at 689494b: Brake cuts power unification
twpol
pushed a commit
that referenced
this pull request
Jun 27, 2025
- Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1107 at f5eb3dc: Fix DPMode when last remote is moved to front. - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1114 at 9a4a873: Fix color of DB in DrivingInfo when in setup with DPU fenced. - Pull request #1115 at 01c0838: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1104 at 128520d: Handle simple adhesion within the axle module - Pull request #1081 at 689494b: Brake cuts power unification
twpol
pushed a commit
that referenced
this pull request
Jun 27, 2025
- Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1107 at f5eb3dc: Fix DPMode when last remote is moved to front. - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1114 at 9a4a873: Fix color of DB in DrivingInfo when in setup with DPU fenced. - Pull request #1115 at 01c0838: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1104 at 03ccdc6: Handle simple adhesion within the axle module - Pull request #1081 at 689494b: Brake cuts power unification
twpol
pushed a commit
that referenced
this pull request
Jun 27, 2025
- Pull request #1104 at 03ccdc6: Handle simple adhesion within the axle module - Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1107 at f5eb3dc: Fix DPMode when last remote is moved to front. - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1114 at 9a4a873: Fix color of DB in DrivingInfo when in setup with DPU fenced. - Pull request #1115 at 01c0838: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1081 at 689494b: Brake cuts power unification
twpol
pushed a commit
that referenced
this pull request
Jun 27, 2025
- Pull request #1104 at 03ccdc6: Handle simple adhesion within the axle module - Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1107 at f5eb3dc: Fix DPMode when last remote is moved to front. - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1114 at 9a4a873: Fix color of DB in DrivingInfo when in setup with DPU fenced. - Pull request #1115 at 270f22f: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1081 at 689494b: Brake cuts power unification
twpol
pushed a commit
that referenced
this pull request
Jun 28, 2025
- Pull request #1104 at 03ccdc6: Handle simple adhesion within the axle module - Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1114 at 9a4a873: Fix color of DB in DrivingInfo when in setup with DPU fenced. (r1.6) - Pull request #1115 at 270f22f: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1081 at 689494b: Brake cuts power unification
twpol
pushed a commit
that referenced
this pull request
Jun 28, 2025
- Pull request #1104 at 03ccdc6: Handle simple adhesion within the axle module - Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1114 at 9a4a873: Fix color of DB in DrivingInfo when in setup with DPU fenced. (r1.6) - Pull request #1115 at 270f22f: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1118 at f1e0f35: Fix DPMode when last remote is moved to front. (r1.6) - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1081 at 689494b: Brake cuts power unification
twpol
pushed a commit
that referenced
this pull request
Jun 30, 2025
- Pull request #1104 at f15bb6b: Handle simple adhesion within the axle module - Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1114 at 9a4a873: Fix color of DB in DrivingInfo when in setup with DPU fenced. (r1.6) - Pull request #1115 at 270f22f: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1118 at f1e0f35: Fix DPMode when last remote is moved to front. (r1.6) - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1081 at 689494b: Brake cuts power unification
twpol
pushed a commit
that referenced
this pull request
Jun 30, 2025
- Pull request #1104 at f15bb6b: Handle simple adhesion within the axle module - Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1114 at 9a4a873: Fix color of DB in DrivingInfo when in setup with DPU fenced. (r1.6) - Pull request #1115 at 270f22f: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1118 at f1e0f35: Fix DPMode when last remote is moved to front. (r1.6) - Pull request #1119 at 57243f0: Re-add missing PR #1103 - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1081 at 689494b: Brake cuts power unification
twpol
pushed a commit
that referenced
this pull request
Jul 1, 2025
- Pull request #1104 at f15bb6b: Handle simple adhesion within the axle module - Pull request #1057 at 1c2bcb4: Switchable brake system - Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1114 at 9a4a873: Fix color of DB in DrivingInfo when in setup with DPU fenced. (r1.6) - Pull request #1115 at 270f22f: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1118 at f1e0f35: Fix DPMode when last remote is moved to front. (r1.6) - Pull request #1119 at 57243f0: Re-add missing PR #1103 - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1081 at 689494b: Brake cuts power unification
twpol
pushed a commit
that referenced
this pull request
Jul 1, 2025
- Pull request #1104 at f15bb6b: Handle simple adhesion within the axle module - Pull request #1057 at 1c2bcb4: Switchable brake system - Pull request #1086 at e10390b: Add Settings Exporter tool (copy settings to INI, etc) - Pull request #1091 at 2391bc0: Automatic speed control - Pull request #1110 at 387388e: Fix Activity Runner persists after loading exception - Pull request #1115 at 270f22f: Do not activate ETS switch if no suitable cars are attached - Pull request #1116 at 721d118: Fix Potential Hang in AnimatedPart.SetFrameWrap - Pull request #1118 at f1e0f35: Fix DPMode when last remote is moved to front. (r1.6) - Pull request #1082 at 5845a1a: Allow variable water level in glass gauge - Pull request #1081 at 689494b: Brake cuts power unification
twpol
approved these changes
Jul 1, 2025
Member
|
Thanks for the PR description with more lines than the code change, genuinely useful for review 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
AnimatedPart.SetFrameWrap uses a while loop to gradually step a frame that's gone negative back to the appropriate positive range. While this works fine for frames that are negative by small amounts, I encountered a game freeze when an animation frame was set to an extremely negative value (due to wheel slip, it seems) and the while loop used all processing time to attempt to move the animation frame from a value in the negative billions back to a positive value 12 at a time (ie: requiring hundreds of millions of iterations).
While loops should be used very sparingly and other options that can't hang the entire process should replace while loops where possible. In this case, a simple modulus operation can achieve the same goal of "restore variable to a given range" in a constant number of steps, regardless of how extremely negative the variable originally was.
As pseudocode, the old method was this:
And the new method is this:
The new algorithm produces identical results in dramatically less time, needing to add
MaxFrameonly once regardless of the magnitude offrame. Whenframeis negative,frame %= MaxFrameimmediately movesframeto the range of -1 to -(MaxFrame - 1) while maintaining the proper "wrap" of the frame counter, so adding onMaxFrameone more time is all that's required to finally restoreframeto the range of 0 to (MaxFrame - 1).For example:
For slightly negative values, this doesn't save much time but I saw frame values in the negative billions, in which case avoiding a while loop is dramatically faster.
Another minor change is that if
MaxFrame = 0, the old code would produceframe = NaNas it would runframe %= MaxFrameeven in this case, but now that calculation is skipped instead, avoiding the NaN which could cause other problems.I'm also wondering if this caused the problem mentioned in this bug report as I encountered this when wheel slipping, and an adhesion change was introduced before the bug was reported.