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

Moon is returned relative to earth rather than initial position #241

Closed
jessegreenberg opened this issue Nov 3, 2016 · 8 comments
Closed
Assignees

Comments

@jessegreenberg
Copy link
Contributor

For the moon + earth system, the 'return object' behavior seems strange. For most bodies, the return object button places the body back at the initial position. For the moon, the moon is placed in the initial position relative to the earth. This creates situations like this:

recording 1

It makes it seem like the 'return' button is broken. @arouinfar is this correct?

@jessegreenberg
Copy link
Contributor Author

Hmm, seems like this was considered fixed in #156? But I can't find more information about this.

@jessegreenberg
Copy link
Contributor Author

Yes, this is not a bug.

        // Restore the moon near the earth and with the same relative velocity vector
          var relativePosition = this.positionProperty.initialValue.minus( earth.positionProperty.initialValue );
          this.positionProperty.set( earth.positionProperty.get().plus( relativePosition ) );
          var relativeVelocity = this.velocityProperty.initialValue.minus( earth.velocityProperty.initialValue );
          this.velocityProperty.set( earth.velocityProperty.get().plus( relativeVelocity ) );

@arouinfar do you know why it should behave like this?

@arouinfar
Copy link
Contributor

I find this behavior a bit odd @jessegreenberg. In general, it seems like return object takes the object back to its initial state. Instead, I think it would make more sense for all objects to return to their saved state. Basically, return object would behave like the rewind button.

Consider this situation. First we save a new state, by pausing and dragging the sun elsewhere.
screen shot 2016-11-07 at 12 27 46 pm

After hitting play, the earth will eventually make its way off screen.
screen shot 2016-11-07 at 12 28 15 pm

The return object button pauses the sim, and returns the earth to its initial position (leaving the moon in the sun's orbit).
screen shot 2016-11-07 at 12 29 33 pm

This doesn't seem useful at all. Instead, I think the return object button should restore the saved state (like the rewind button). If no state has been saved, all objects would return to their initial position.

Even though it's redundant, I think we should keep the return object button because it serves as a visual cue to the users. They may not intuitively click on rewind or reset after an object is lost off-screen. However, let's rename it "Return Objects".

@arouinfar
Copy link
Contributor

To summarize:

  • Return object button will behave exactly like the rewind button.
  • Rename it "Return Objects" since multiple objects may be moved.

@jessegreenberg
Copy link
Contributor Author

Sounds good @arouinfar. Should the 'Return Objects' button pause the sim like before, or should the sim continue playing once objects are returned?

@jessegreenberg
Copy link
Contributor Author

Commit above should fix this, and if we need to we can easily pause the sim once the 'Return Objects' button is pressed. At the moment, it does not pause the sim because the rewind doesn't either. But the 'Return Objects' button used to pause the sim, so perhaps this is where it should differ from the 'Rewind' button.

@arouinfar
Copy link
Contributor

But the 'Return Objects' button used to pause the sim, so perhaps this is where it should differ from the 'Rewind' button.

I agree @jessegreenberg. I wasn't thinking about this difference earlier. Can you please update the Return Objects button so that it pauses the sim?

@arouinfar arouinfar removed their assignment Nov 10, 2016
@jessegreenberg
Copy link
Contributor Author

The system is now paused when the objects are returned. Thanks @arouinfar, I think your recommendations in #241 (comment) have improved the consistency of this feature.

Closing.

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

No branches or pull requests

2 participants