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

solve issue #2370 (deeply clone MotionEvent path) #2390

Merged
merged 3 commits into from
Mar 19, 2025

Conversation

stephengold
Copy link
Member

Currently a work in progress.

@stephengold stephengold marked this pull request as draft March 6, 2025 21:51
@stephengold stephengold linked an issue Mar 6, 2025 that may be closed by this pull request
@stephengold
Copy link
Member Author

This PR was created to address issue #2370. Here's what it does:

  • It adds code to MotionEvent.cloneFields() to clone the path field. While implementing this change, I discovered that the lookAt and rotation fields also needed to be cloned. (The direction and upVector fields are cloned in the jmeClone() method, which is irregular but suffices for most use cases.)

  • It makes MotionPath and Spline deeply cloneable, adding jmeClone() and cloneFields() methods. Unfortunately, Cloner.clone() does not work on ArrayList<Float> because Float is not cloneable---see issue Cloner fails on enums and wrapped primitives #879. In Spline.cloneFields() I used copy constructors to work around this limitation.

  • It adds automated cloning tests for all 3 classes to jme3-core/src/test. The cloning tests for Spline use the same test splines as the existing serialization tests. To avoid duplicating code, I refactored the generation of test splines to private methods.

  • The copyright notices in the affected source files were updated to include the year 2025.

@stephengold stephengold marked this pull request as ready for review March 7, 2025 21:30
@yaRnMcDonuts yaRnMcDonuts added this to the v3.8.0 milestone Mar 9, 2025
@yaRnMcDonuts
Copy link
Member

Since no one else has reviewed this yet, I did my best to look through the code and I don't see any apparent issues.

So I will leave this open for another day or two and then I plan to merge this in ~48 hours.

@yaRnMcDonuts yaRnMcDonuts merged commit 985ac2c into master Mar 19, 2025
15 checks passed
@stephengold stephengold deleted the sgold/issue/2370 branch March 19, 2025 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MotionEvent.cloneFields() should deeply clone the path field
2 participants