zepumph
commented
1 year ago @matthew-blackman and I went over this today. In general things are great! I am so happy with how this has been improving. I recommended 2 further potential improvements to look into.
- It seems like all that is needed for creating a DataPoint is the previous point and a DT value. The step function does this in three separate, duplicated code chunks (when still flying, when landed, and for the apex) and I think it could be improved on. What about a function called
this.getNextDataPoint( previousDataPoint, dt). I think you could argue that all three of these should behave the same way in terms of how we calculate the "next" point, though it seems like the apex is getting extra fudging logic at this time. If this was the case, then the step function is just (simplification) 1. calculate the right DT to start, 2. handle apex if needed, 3. create data point 4. add data point. - There is still a smaller piece or two that continues to be duplicated, like the calculation for accelleration. If we choose not to go all in on the first suggestion, we at least should factor out a couple more smaller chunks of code as was done (and done well) for
dragForceForVelocity.
matthew-blackman
The Trajectory step function is about 300 lines long and includes some handling of edge casses (https://github.com/phetsims/projectile-motion/issues/286). We should review and consider modularizing the physics in this algorithm, including the handling of discrete calculation issues before republishing Projectile Motion.