Altering certain aspects of the sim with balls offscreen causes buggy behavior related to state
Closed this issue · 10 comments
Test device
Windows 10 Laptop
Operating System
Windows 10 1909
Browser
Firefox 84.0.2 (64-bit)
Problem description
For phetsims/qa#599
Related to #183
Turning the borders off causes a variety of buggy behavior when certain aspects of the sim are altered.
- When Velocity or Size is changed, which includes the use of the constant size checkbox, the balls are returned to the edge of the screen which @arouinfar confirmed was an unintended behavior.
- Noticed by @KatieWoe. When Elasticity is changed while the balls are off screen, stepping back only goes to the point where elasticity was changed, which is an off screen location, while the blue reset button returns the balls in a manner similar to the "Return Balls" button, which is inconsistent with its usual behavior of returning balls to the last altered state.
Steps to reproduce
To reproduce the velocity and size returning balls issues
- Uncheck the reflecting border box
- Unpause the simulation
- Allow balls to leave screen
- Check the constant size box, or alter the mass or velocity by clicking on the values at the bottom of the screen
To reproduce the elasticity and step back issue
- Uncheck the reflecting border box
- Unpause the simulation
- Allow balls to leave screen
- Alter elasticity
- Pause the sim and hold the step back button until grey
- Compare with the results of the blue reset button
Visuals
A gif of the issues caused with changing velocity and mass while off screen
A gif demonstrating the failure of step back to synchronize with the locations of the blue reset all button
Troubleshooting information:
!!!!! DO NOT EDIT !!!!!
Name: Collision Lab
URL: https://phet-dev.colorado.edu/html/collision-lab/1.1.0-rc.1/phet/collision-lab_en_phet.html
Version: 1.1.0-rc.1 2021-01-20 01:05:28 UTC
Features missing: touch
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:84.0) Gecko/20100101 Firefox/84.0
Language: en-US
Window: 1920x966
Pixel Ratio: 1/1
WebGL: WebGL 1.0
GLSL: WebGL GLSL ES 1.0
Vendor: Mozilla (Mozilla)
Vertex: attribs: 16 varying: 30 uniform: 4096
Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32)
Max viewport: 32767x32767
OES_texture_float: true
Dependencies JSON: {}
Addition: Adding balls seems to show similar buggy behavior.
A few more things I noticed about this change of state with the reflecting wall:
When a ball is halfway through the wall and the reflected wall is turned on, the ball still continues through.
When the balls are on the "stick" setting, they become one entity so if one ball goes through then the other ball goes through as well, even though it is fully behind the border at the time.
Behavior when stepping back past reflecting border.
When Velocity or Size is changed, which includes the use of the constant size checkbox, the balls are returned to the edge of the screen which @arouinfar confirmed was an unintended behavior.
Does that mean I'll need to identify whether it was inside the play area BEFORE a user behavior? Presumably if a ball is inside of the play area (small, but on the edge of the play area) and the user increases the size, it would usually now be partially outside of the play area (and we bump it back in).
So... if a ball is outside the play area BEFORE editing, it should STAY outside? Does that sound correct @arouinfar?
When Elasticity is changed while the balls are off screen, stepping back only goes to the point where elasticity was changed, which is an off screen location, while the blue reset button returns the balls in a manner similar to the "Return Balls" button, which is inconsistent with its usual behavior of returning balls to the last altered state.
This seems consistent with the stated goals of "don't ever save ball positions for the restart-balls button when one is outside the play area" and "don't let the user rewind past the point of an elasticity change". In that specific case, it seems impossible to find a single instant back in time where rewinding and restarting can use the same instant in time AND satisfy those conditions.
What should the desired behavior here be?
Addition: Adding balls seems to show similar buggy behavior.
This is where the sim would otherwise save the state when the ball is added. However because one ball is outside the play area, this doesn't happen (as specified).
Perhaps the assumptions baked into the codebase aren't matching up with expectations?
When a ball is halfway through the wall and the reflected wall is turned on, the ball still continues through.
This seems like by design, according to the simulation's code. Is there another behavior that is desired?
When the balls are on the "stick" setting, they become one entity so if one ball goes through then the other ball goes through as well, even though it is fully behind the border at the time.
That definitely seems correct, and describes accurately how the code is handling it. This would be fairly tricky to change.
@arouinfar let me know your thoughts on the above? Happy to discuss over a call if it would help.
Thanks @loganbraywork @KatieWoe @brooklynlash! You've documented some really interesting edge cases here. I've added my comments to each observation below, but the tl;dr is that off-screen balls are weird sometimes, but I don't think we need to make any changes.
When Velocity or Size is changed, which includes the use of the constant size checkbox, the balls are returned to the edge of the screen which @arouinfar confirmed was an unintended behavior.
Does that mean I'll need to identify whether it was inside the play area BEFORE a user behavior? Presumably if a ball is inside of the play area (small, but on the edge of the play area) and the user increases the size, it would usually now be partially outside of the play area (and we bump it back in).
So... if a ball is outside the play area BEFORE editing, it should STAY outside? Does that sound correct @arouinfar?
When @loganbraywork demonstrated this for me it looked really buggy, but after @jonathanolson's explanation I understand what the sim is doing now. Let's leave the behavior as-is. It's a bit of an edge case, anyway, and doesn't hurt anything.
When Elasticity is changed while the balls are off screen, stepping back only goes to the point where elasticity was changed, which is an off screen location, while the blue reset button returns the balls in a manner similar to the "Return Balls" button, which is inconsistent with its usual behavior of returning balls to the last altered state.
This seems consistent with the stated goals of "don't ever save ball positions for the restart-balls button when one is outside the play area" and "don't let the user rewind past the point of an elasticity change". In that specific case, it seems impossible to find a single instant back in time where rewinding and restarting can use the same instant in time AND satisfy those conditions.
What should the desired behavior here be?
I don't think we should be saving the state of off-screen things. Now that we've decided in #183 to disable the step back button for elasticity =/= 0 the latter goal of "don't let the user rewind past the point of an elasticity change" is moot. I don't think we need to do anything here @jonathanolson.
Addition: Adding balls seems to show similar buggy behavior.
This is where the sim would otherwise save the state when the ball is added. However because one ball is outside the play area, this doesn't happen (as specified).
We don't need to save the state of off-screen balls. I'll be sure to document this in the Teacher Tips.
When a ball is halfway through the wall and the reflected wall is turned on, the ball still continues through.
This seems like by design, according to the simulation's code. Is there another behavior that is desired?
Correct @jonathanolson this is by design and will be documented in the Teacher Tips.
When the balls are on the "stick" setting, they become one entity so if one ball goes through then the other ball goes through as well, even though it is fully behind the border at the time.
That definitely seems correct, and describes accurately how the code is handling it. This would be fairly tricky to change.
This is the correct behavior and consistent with the point above.