Skip to content

Fix animation frame duration issue - #86

Open
xendke wants to merge 2 commits into
NobleRobot:mainfrom
xendke:patch-4
Open

Fix animation frame duration issue#86
xendke wants to merge 2 commits into
NobleRobot:mainfrom
xendke:patch-4

Conversation

@xendke

@xendke xendke commented Jul 7, 2024

Copy link
Copy Markdown
Contributor

I've noticed that in my walking animation (a looping 2 frame animation) there's one frame being drawn more than the other. This PR fixes that.

It seems that there were a couple of instances of self.frameDuration that should have really been self.current.frameDuration

Before

before

After

after

assada added a commit to assada/NobleEngine that referenced this pull request Feb 24, 2026
ericlewis added a commit to ericlewis/NobleEngine that referenced this pull request Aug 11, 2026
…Noble.Animation. (upstream PR NobleRobot#86)

Fixes uneven frame timing for animation states whose frameDuration
differs from the first-added state: previousFrameDurationCount was
being reset from the stale animation-level self.frameDuration instead
of self.current.frameDuration at the loop/next-state sites. Also
redirects the now-unused animation-level frameDuration assignment in
addState. Applies upstream PR NobleRobot#86.

Co-authored-by: Juan Xavier Gomez <xendke@users.noreply.github.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ericlewis added a commit to ericlewis/NobleEngine that referenced this pull request Aug 11, 2026
Commit 6be1f0e (upstream PR NobleRobot#86) reset previousFrameDurationCount to
the current state's frameDuration, but the advance condition
(frameDurationCount - previousFrameDurationCount) >= frameDuration,
together with frameDurationCount resetting to 1, means the consistent
reset value is 1 (the value both counters are initialized with). Using
the state's frameDuration made the first frame after a loop or state
change display for (2 * frameDuration - 1) ticks instead of
frameDuration - a visible stutter on every loop whenever frameDuration
is greater than 1. In the state-transition branch it also read the
outgoing state's frameDuration, since setState() runs on the next line.

Reset both counters to 1 in both branches. This also makes the
addState() line "self.current.frameDuration = frameDuration" a
redundant assignment (the state table constructor already sets it),
so remove it.

Refs upstream PR NobleRobot#86.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ericlewis added a commit to ericlewis/NobleEngine that referenced this pull request Aug 11, 2026
…Noble.Animation. (upstream PR NobleRobot#86)

Fixes uneven frame timing for animation states whose frameDuration
differs from the first-added state: previousFrameDurationCount was
being reset from the stale animation-level self.frameDuration instead
of self.current.frameDuration at the loop/next-state sites. Also
redirects the now-unused animation-level frameDuration assignment in
addState. Applies upstream PR NobleRobot#86.

Co-authored-by: Juan Xavier Gomez <xendke@users.noreply.github.com>
ericlewis added a commit to ericlewis/NobleEngine that referenced this pull request Aug 11, 2026
Commit 6be1f0e (upstream PR NobleRobot#86) reset previousFrameDurationCount to
the current state's frameDuration, but the advance condition
(frameDurationCount - previousFrameDurationCount) >= frameDuration,
together with frameDurationCount resetting to 1, means the consistent
reset value is 1 (the value both counters are initialized with). Using
the state's frameDuration made the first frame after a loop or state
change display for (2 * frameDuration - 1) ticks instead of
frameDuration - a visible stutter on every loop whenever frameDuration
is greater than 1. In the state-transition branch it also read the
outgoing state's frameDuration, since setState() runs on the next line.

Reset both counters to 1 in both branches. This also makes the
addState() line "self.current.frameDuration = frameDuration" a
redundant assignment (the state table constructor already sets it),
so remove it.

Refs upstream PR NobleRobot#86.
@ericlewis

Copy link
Copy Markdown

Thanks for this fix — switching the reset sites to read self.current.frameDuration instead of the stale animation-level self.frameDuration is right, but I think the reset value needs one more change.

The advance check is (frameDurationCount - previousFrameDurationCount) >= self.current.frameDuration, and both counters are initialized to 1. At the loop/next-state sites frameDurationCount is reset to 1, so resetting previousFrameDurationCount to self.current.frameDuration makes the delta start negative: the first frame after a loop or state change displays for 2 × frameDuration − 1 ticks instead of frameDuration — a visible stutter on every loop whenever frameDuration > 1 (e.g. 5 ticks instead of 3 for a frameDuration of 3). In the next-state branch it also reads the outgoing state's frameDuration, since setState() only runs on the following line.

Since frameDurationCount goes back to 1, the consistent reset for previousFrameDurationCount is also 1 (the value both counters are initialized with):

self.frameDurationCount = 1
self.previousFrameDurationCount = 1

With that, the self.current.frameDuration = frameDuration assignment this PR adds in addState() is no longer needed — the state table constructor already sets it.

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.

2 participants