Fix interpolator being lost when moving keyframe to the end (fixes #62)

src/integration-test/java/com/replaymod/core/ReplayModIntegrationTest.java

@@ -1,6 +1,7 @@
 package com.replaymod.core;
 
 import com.replaymod.core.regression.RegressionTest60;
+import com.replaymod.core.regression.RegressionTest62;
 import com.replaymod.extra.DownloadOpenEye;
 import com.replaymod.online.SkipLogin;
 import com.replaymod.recording.CreateSPWorld;
@@ -44,6 +45,7 @@ public class ReplayModIntegrationTest {
                 new LoadReplay(),
 
                 new RegressionTest60(),
+                new RegressionTest62(),
 
                 // new AbstractTask() {}, // Uncomment to not exit on success (useful for writing more tests)
                 new ExitReplay()

src/integration-test/java/com/replaymod/core/regression/RegressionTest62.java

@@ -0,0 +1,28 @@
+package com.replaymod.core.regression;
+
+import com.replaymod.core.CompositeTask;
+import com.replaymod.core.Task;
+import com.replaymod.replay.overlay.OverlayGui;
+import com.replaymod.simplepathing.GuiPathingTasks;
+
+/**
+ * Regression test: #62 Swapping the only two existing keyframes causes NPE
+ */
+public class RegressionTest62 extends CompositeTask {
+    public RegressionTest62() {
+        super(new Task[]{
+                OverlayGui.whileOpened(
+                        // Place first keyframe
+                        Task.click(130, 50),
+                        new GuiPathingTasks.ClickPositionKeyframeButton(),
+                        // Place second keyframe
+                        Task.click(150, 50),
+                        new GuiPathingTasks.ClickPositionKeyframeButton(),
+                        // Move first keyframe past second keyframe
+                        Task.click(130,50),
+                        Task.drag(170,50)
+                ),
+                new GuiPathingTasks.ClearKeyframeTimeline()
+        });
+    }
+}

src/main/java/com/replaymod/simplepathing/SPTimeline.java

@@ -340,13 +340,15 @@ public class SPTimeline implements PathingRegistry {
         Optional<Interpolator> firstInterpolator =
                 path.getSegments().stream().findFirst().map(PathSegment::getInterpolator);
 
-        // The interpolator of the previous segment
+        // The interpolator that will be lost once we remove the old keyframe and has to be restored afterwards
-        Optional<Interpolator> interpolatorBefore =
+        Optional<Interpolator> lostInterpolator = path.getSegments().stream().filter(s -> {
-                path.getSegments().stream().filter(s -> s.getEndKeyframe() == keyframe).findFirst().map(PathSegment::getInterpolator);
+            // If this is the last keyframe,
-
+            if (Iterables.getLast(path.getKeyframes()) == keyframe) {
-        // The interpolator of the following segment
+                return s.getEndKeyframe() == keyframe; // the previous interpolator will be lost
-        Optional<Interpolator> interpolatorAfter =
+            } else { // otherwise
-                path.getSegments().stream().filter(s -> s.getStartKeyframe() == keyframe).findFirst().map(PathSegment::getInterpolator);
+                return s.getStartKeyframe() == keyframe; // the following interpolator will be lost
+            }
+        }).findFirst().map(PathSegment::getInterpolator);
 
         // First remove the old keyframe
         Change removeChange = create(path, keyframe);
@@ -369,18 +371,27 @@ public class SPTimeline implements PathingRegistry {
         Keyframe newKf = path.getKeyframe(newTime);
         if (Iterables.getLast(path.getKeyframes()) != newKf) { // Unless this is the last keyframe
             // the interpolator of the following segment has been lost and needs to be restored
-            restoreInterpolatorChange = interpolatorAfter.<Change>flatMap(interpolator ->
+            restoreInterpolatorChange = lostInterpolator.<Change>flatMap(interpolator ->
                     path.getSegments().stream().filter(s -> s.getStartKeyframe() == newKf).findFirst().map(segment ->
                             SetInterpolator.create(segment, interpolator)
                     )
             ).orElseGet(CombinedChange::create);
         } else { // If it is the last keyframe however,
             // the interpolator of the previous segment has been lost and needs to be restored
-            restoreInterpolatorChange = interpolatorBefore.<Change>flatMap(interpolator ->
+            restoreInterpolatorChange = path.getSegments().stream().filter(s -> s.getEndKeyframe() == newKf)
-                    path.getSegments().stream().filter(s -> s.getEndKeyframe() == newKf).findFirst().map(segment ->
+                    .findFirst().flatMap(segment -> lostInterpolator.map(interpolator -> {
-                            SetInterpolator.create(segment, interpolator)
+                        // additionally, if the interpolation of this keyframe was set to explicit, that property
-                    )
+                        // has to be transferred to the start keyframe of the new segment
-            ).orElseGet(CombinedChange::create);
+                        if (newKf.getValue(ExplicitInterpolationProperty.PROPERTY).isPresent()) {
+                            return CombinedChange.create(
+                                    SetInterpolator.create(segment, interpolator),
+                                    UpdateKeyframeProperties.create(path, segment.getStartKeyframe())
+                                            .setValue(ExplicitInterpolationProperty.PROPERTY, ObjectUtils.NULL).done()
+                            );
+                        } else {
+                            return SetInterpolator.create(segment, interpolator);
+                        }
+            })).orElseGet(CombinedChange::create);
         }
         restoreInterpolatorChange.apply(timeline);
 

src/test/java/com/replaymod/simplepathing/SPTimelineTest.java

@@ -473,6 +473,27 @@ public class SPTimelineTest {
         impl.setInterpolatorToDefault(0);
     }
 
+    @Test
+    public void testMoveKeyframeSimple() {
+        addPosition(0, 0);
+        addPosition(1, 1);
+        setInterpolator(0, new LinearInterpolator(), 1);
+        assertIsLinear(0);
+
+        impl.moveKeyframe(SPPath.POSITION, 0, 2);
+        assertValidInterpolators(SPPath.POSITION, 1);
+        assertIsLinear(0);
+        impl.moveKeyframe(SPPath.POSITION, 2, 0);
+        assertValidInterpolators(SPPath.POSITION, 1);
+        assertIsLinear(0);
+
+        addPosition(2, 1);
+        impl.moveKeyframe(SPPath.POSITION, 0, 3);
+        assertValidInterpolators(SPPath.POSITION, 1);
+        assertIsLinear(0);
+        assertIsLinear(1);
+    }
+
     @Test
     public void testMoveKeyframe() {
         addPosition(1, 0);