search

phetsims / projectile-motion

"Projectile Motion" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
15 stars 13 forks source link

Clean up view code given one ProjectileObject per Trajectory #313

Closed zepumph closed 1 year ago

zepumph commented 1 year ago

Now given the work done in https://github.com/phetsims/projectile-motion/issues/291, we can cleanup TrajectoryNode and ProjectileObjectNode quite a bit. @matthew-blackman and I on are the case!!!!

zepumph commented 1 year ago

I found 3 other bug fixes while finishing this up with @matthew-blackman:

  1. min opacity values were set to 0, so actually the MAX_NUM_TRAJECTORIES constant was off by one in a visual sense, since the last was invisible.
  2. There was a bug in the default parameter for the TrajectoryNode constructor still pointing to ProjectileMotionConstants.MAX_TRAJECTORY_COUNT.
  3. the apex point was never having its opacity reset, so in the published version, the apex point was bright green always. Oop!

I wanted to have the opacity just set on the TrajectoryNode itself, but that doesn't work since dots max a .5 and the path maxes at 1.

Mostly the work involved getting rid of container nodes that weren't necessary, and that @matthew-blackman and I found to be complicating the implementation. We also renamed many variables and documented code as we saw fit.

@matthew-blackman could you take another look through our changes above. Do you see any trouble? Do you see anything to be improved upon?

matthew-blackman commented 1 year ago

@zepumph and I noticed that the projectile is currently being drawn in front of the trajectory, whereas it should be the other way around.

matthew-blackman commented 1 year ago

I am not longer able to reproduce this issue, and believe is has been fixed by https://github.com/phetsims/projectile-motion/commit/cd31d599d11d63b4c081aaa9a80a82388bb1904f. Closing this issue.