Repeat action causes small stutter

[mattiashagstrand]

When repeating an action (e.g. rotation) each time the action repeats their is a small stutter.
This is caused by the action reset because an action rarely ends at a completion ratio of exactly one.

Changing

internal func reset() {
    startTime = nil
    lastUpdateTime = nil
}

to

internal func reset() {
    startTime = lastUpdateTime
}

in Action seems to fix the problem in the case when an action is repeated but could potentially cause problems in other cases. Maybe Action should have a restartmethod? In that case the responsibility for calling resetor restart probably needs to be moved out of ActionWrapper.

[JohnSundell]

Great find @mattiashagstrand! 👍 How about if we always make sure that the last update that an action gets has a completion ratio of exactly 1?

[mattiashagstrand]

Yes, if the completion rate always becomes exactly one it should solve the problem.
To achieve that we need to make sure the animation time is dividable by 1/[screen refresh rate] right?

[JohnSundell]

That's not really enough, since the framerate can vary in practice. What I think we need to do is to always run a final update with completionRatio = 1.0 even if the action is technically supposed to be done. That is, give it a final update before finishing it in case the completion ratio isn't exactly 1. What do you think @mattiashagstrand?

[mattiashagstrand]

The problem isn't that we don't do an update with completionRatio = 1.0, but that we truncate completionRatio to 1.0 in the last update.

Here is a simplified example:

Say that we want to move an actor 100points in 1second and repeat that move, and assume that one update happens every 0.3seconds.

In the first 3 updates completionRatio will be 0.3, 0.6and 0.9 which is ok. However, in the last update before the action repeats completionRatio will be 1.2 and will be truncated to 1.0. Then the action repeats and in the next update completionRatio will be 0.3. This means that the actor has moved in the steps 30, 60, 90, 100, 130.

If we had used an action to move 200points in 2 seconds the steps would have been 30, 60, 90, 120, 150.

I tested a bit more and I don't think my suggested solution works.
We actually have two problems:

1. The animation stutters a little because of the difference in step length.
2. The action moves the actor slower than you would expect.

Was this understandable @JohnSundell?

[mattiashagstrand]

I created a test that demonstrates the problem:

func testRepeatingActionSmoothMovement() {
    let moveVector = Vector(dx: 100, dy: 0)
    let moveAction = MoveAction<Actor>(vector: moveVector, duration: 1)

    let actor = Actor()
    actor.repeat(moveAction)
    game.scene.add(actor)
    game.update()

    game.timeTraveler.travel(by: 0.3)
    game.update()
    XCTAssertEqual(actor.position.x, 30.0, accuracy: 0.001)

    game.timeTraveler.travel(by: 0.6)
    game.update()
    XCTAssertEqual(actor.position.x, 90.0, accuracy: 0.001)

    game.timeTraveler.travel(by: 0.3)
    game.update()
    game.update()
    game.timeTraveler.travel(by: 0.3)
    game.update()
    XCTAssertEqual(actor.position.x, 150.0, accuracy: 0.001)
}

I thought it was a bit strange that the extra game.update() was needed so I also printed out completionRatio as the test ran:

completionRatio: 0.0
completionRatio: 0.300000011920929
completionRatio: 0.900000035762787
completionRatio: 1.0
completionRatio: 0.0
completionRatio: 0.300000011920929

Looks like the repeat takes an extra update to start repeating. That can probably also cause stuttering.

[mattiashagstrand]

I have now confirmed that my fix doesn't fix anything... 😣

I tried removing the truncation of completionRatio so that it becomes 1.2 in the last updated in the first repetition. That makes the test pass but probably not what we want.

What is needed is for the extra 0.2 to carry over to the completionRatio for the first update in the next iteration. Not sure how to di that though without breaking the nice separation of concerns in the current design.

Will stop spamming now and let you think about this @JohnSundell 😄