cmu-cs-academy / desktop-cmu-graphics

BSD 3-Clause "New" or "Revised" License
15 stars 9 forks source link

Fix how often onStep is called. #58

Closed rriley closed 7 months ago

rriley commented 7 months ago

This change fixes how often onStep is called by using a tighter tolerance for how often to call it based on the milli-seconds passed since the last tick. This is important because any programs that count ticks to estimate time elapsed were previously very inaccurate. For a stepsPerSecond of 30, the old code would drift by 14.5 seconds every minute for a simple stopwatch, which is excessive and noticeable. This modified version only drifts 0.3 seconds.

If a student needs more accuracy, they should probably hook the onMainLoopEvent to get the exact milli-seconds passed each loop and do the math themselves.

rriley commented 7 months ago

Here is a simple program to test drift:

from cmu_graphics import *
import datetime

# Patched drift: 0.3 seconds in 60 seconds
# Unpatched drift: 14.5 seconds in 60 seconds

def onAppStart(app):
    app.stepsPerSecond = 30
    app.startTime = None
    app.count = 0

def redrawAll(app):
    now = datetime.datetime.now()
    if app.startTime != None:
        delta = now - app.startTime
        drift = delta.total_seconds() - (app.count / app.stepsPerSecond)
        drawLabel(
            f"Internal timer: {app.count/app.stepsPerSecond:.2f} seconds",
            app.width // 2,
            app.height // 3,
            size=app.width // 20,
        )
        drawLabel(
            f"Real time: {delta.total_seconds():.2f} seconds",
            app.width // 2,
            app.height // 2,
            size=app.width // 20,
        )
        drawLabel(
            f"Drift: {drift:.2f} seconds",
            app.width // 2,
            2 * app.height // 3,
            size=app.width // 20,
        )

def onStep(app):
    # This seems like something that should be done in onAppStart, but it is
    # better to do here so that we make sure both timers start at as close
    # to the same time as possible.
    if app.startTime == None:
        app.count = 0
        app.startTime = datetime.datetime.now()
    else:
        app.count += 1

# This is how you run the program
runApp(width=800, height=800)
dkosbie commented 7 months ago

Very cool! :-) Thanks!!!

On Sun, Feb 11, 2024 at 9:02 AM Ryan Riley @.***> wrote:

This change fixes how often onStep is called by using a tighter tolerance for how often to call it based on the milli-seconds passed since the last tick. This is important because any programs that count ticks to estimate time elapsed were previously very inaccurate. For a stepsPerSecond of 30, the old code would drift by 14.5 seconds every minute for a simple stopwatch, which is excessive and noticeable. This modified version only drifts 0.3 seconds.

If a student needs more accuracy, they should probably hook the onMainLoopEvent to get the exact milli-seconds passed each loop and do the math themselves.

You can view, comment on, or merge this pull request online at:

https://github.com/cmu-cs-academy/desktop-cmu-graphics/pull/58 Commit Summary

File Changes

(1 file https://github.com/cmu-cs-academy/desktop-cmu-graphics/pull/58/files)

Patch Links:

— Reply to this email directly, view it on GitHub https://github.com/cmu-cs-academy/desktop-cmu-graphics/pull/58, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA3PPRBXEWR5SUPTM2Y36ITYTDFJBAVCNFSM6AAAAABDDR7ABOVHI2DSMVQWIX3LMV43ASLTON2WKOZSGEZDQOJZHE4TQMQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>

schmave commented 7 months ago

Looks like a big improvement. Thanks for looking into this so carefully!