Skip to content
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

State oddly saved if ball bumped off screen #163

Closed
arouinfar opened this issue Aug 25, 2020 · 4 comments
Closed

State oddly saved if ball bumped off screen #163

arouinfar opened this issue Aug 25, 2020 · 4 comments
Assignees
Labels

Comments

@arouinfar
Copy link
Contributor

For phetsims/qa#537 and related to #100.

It's generally rare to completely lose a ball when it is bumped away from its neighbors. However, after an inelastic collision, balls are grouped by a wall. Clicking on a middle ball can cause it to be bumped out of bounds. When trying to recover it with the restart button, the lost ball will return to its previously saved state, but the other balls will remain where they were with the lost ball was clicked.

@brandonLi8 this behavior seems odd to me. Can all balls return to their previously saved states? If not, I'm ok with the partially saved state since it allows the user to easily retrieve the lost ball.

Steps to reproduce:

  1. Explore 1D
  2. 3 Balls, Elasticity 0%
  3. Play
  4. Balls will end up grouped at the left wall
    image
  5. Click (and hold) Ball 2, and it will snap a bit to the left (closest tick mark)

Screen Shot 2020-08-25 at 5 15 54 PM

6. Release Ball 2, and it will disappear. (Likely bumped to the left of Ball 1.)

image

  1. Press restart button, and the state is oddly restored

image

@arouinfar arouinfar added the type:bug Something isn't working label Aug 25, 2020
@brandonLi8
Copy link
Contributor

brandonLi8 commented Aug 25, 2020

@arouinfar I confirmed that the ball is being "bumped" to the left of ball1.

But I don't believe that this is actually a bug due to how restart/states work; at the END of when any ball is manipulated, every balls state is saved, if and only if it is in-bounds. This decision was described in #90 (comment), where @arouinfar said:

but only saving the state if the balls are in bounds sounds reasonable to me.

So essentially what is happening in this scenario is:

  1. Ball2 is snapped out of bounds.
  2. Since the user just finished manipulating the system, every ball that is in bounds is saved, which is ball 1 and 3. Notice that ball2's state is not saved to the out-of-bounds position, and its most recent save is the starting state.
  3. Restart then sets the properties of the balls to the most recent state. For balls 1 and 3, it is already in the most recent state. For ball2, its most recently saved state is the starting state (when the sim was loaded).

However, if you think the current behavior is incorrect, we could slightly alter this behavior; the states are always grouped as one, so when the user is finished manipulating AND none of the balls are out-of-bounds, ALL of the states of every ball is saved. If any of the balls are out-of-bounds, NONE of the states are saved. If that makes sense.

@brandonLi8 brandonLi8 assigned arouinfar and unassigned brandonLi8 Aug 25, 2020
@arouinfar
Copy link
Contributor Author

we could slightly alter this behavior; the states are always grouped as one, so when the user is finished manipulating AND none of the balls are out-of-bounds, ALL of the states of every ball is saved. If any of the balls are out-of-bounds, NONE of the states are saved. If that makes sense.

I would prefer this all-or-nothing approach to saving the state @brandonLi8.

@arouinfar arouinfar assigned brandonLi8 and unassigned arouinfar Aug 26, 2020
brandonLi8 added a commit that referenced this issue Aug 26, 2020
@brandonLi8
Copy link
Contributor

OK. Done in the commit above. Back to @arouinfar.

@arouinfar
Copy link
Contributor Author

Looks good in master @brandonLi8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants