Remove catch-up behavior of LLTimers:every()

This was bug-prone and hard to reason about. Switch to behavior more
closely matching the existing behavior of `llSetTimerEvent()`'s internal
scheduling logic.

Fixes #37

Author: HaroldCindy

VM/src/llltimers.cpp

@@ -19,8 +19,8 @@
 enum TimerDataIndex {
     TIMER_HANDLER = 1,
     TIMER_INTERVAL = 2,
-    TIMER_NEXT_RUN = 3,              // Actual time to fire (may be clamped for catch-up)
-    TIMER_LOGICAL_SCHEDULE = 4,      // Logical schedule time (never clamped, always += interval)
+    TIMER_NEXT_RUN = 3,              // Time at which this timer is next eligible to fire
+    TIMER_LOGICAL_SCHEDULE = 4,      // Historical slot: kept for serialization stability, mirrors TIMER_NEXT_RUN
     TIMER_LEN = TIMER_LOGICAL_SCHEDULE,
 };
 
@@ -554,20 +554,15 @@ DEFINE_YIELDABLE(lltimers_tick, 0)
                 continue;
             }
 
-            // Get the nextRun time from the timer
+            // Get the nextRun time from the timer. We read this BEFORE updating
+            // it so we can pass the pre-update value to the handler as its
+            // "scheduled time" (the earliest instant this tick was allowed to
+            // fire), letting handlers compute `now - scheduled_time` to see
+            // how much extra delay there was beyond the minimum interval.
             lua_rawgeti(L, CURRENT_TIMER, TIMER_NEXT_RUN);
             double next_run = lua_tonumber(L, -1);
             lua_pop(L, 1);
 
-            // Get the logical schedule time (what we'll pass to the handler)
-            // We read this BEFORE updating it so the handler gets the current value
-            lua_rawgeti(L, CURRENT_TIMER, TIMER_LOGICAL_SCHEDULE);
-            double logical_schedule = lua_tonumber(L, -1);
-            lua_pop(L, 1);
-
-            // Save the original value - this is what the handler should receive
-            double handler_scheduled_time = logical_schedule;
-
             // Verify nextRun is a reasonable number
             LUAU_ASSERT(next_run >= 0.0);
 
@@ -611,75 +606,43 @@ DEFINE_YIELDABLE(lltimers_tick, 0)
             }
             else
             {
-                // Schedule its next run using absolute scheduling with clamped catch-up
-                // (next = previous_scheduled_time + interval)
-                // This prevents drift and ensures the timer maintains its rhythm.
-                // However, if the timer is very late (> 2 seconds), skip ahead to prevent
-                // excessive catch-up iterations that could bog down the system.
-                //
-                // We maintain two schedules:
-                // - TIMER_NEXT_RUN: May be clamped to prevent catch-up storms
-                // - TIMER_LOGICAL_SCHEDULE: Never clamped, always += interval
-                //   This lets handlers know their true delay from the logical schedule.
+                // Schedule the next run. The default is cadence-preserving
+                // absolute scheduling (previous_next_run + interval), so a
+                // healthy 6s timer stays near 6s/12s/18s/... without drift.
+                // But if that target is already in the past, meaning we
+                // would need to fire again immediately to "catch up" on ticks
+                // missed while the script was descheduled.
                 //
-                // Note that we do this BEFORE the timer is ever run.
-                // This ensures that handler runtime has no effect on
-                // when the handler will be invoked next.
-                bool did_clamp = false;
-
-                // Correctly handles scheduling an event for _at least immediately after_
-                // the current time, even if addition of a tiny interval underflows.
-                // For interval=0: use start_time to prevent re-trigger without clock advance
-                // For other intervals: use next_run to maintain rhythm
-                double next_scheduled = std::max(
-                    std::nextafter(interval == 0.0 ? start_time : next_run, INFINITY),
-                    (interval != 0.0) ? next_run + interval : 0.0
-                );
-                double new_next_run = next_scheduled;
-
-                // Catchup logic with overflow protection
-                constexpr double MAX_CATCHUP_TIME = 2.0;
-                if (interval > 0 && start_time - next_scheduled > MAX_CATCHUP_TIME)
+                // Note: we compute this BEFORE invoking the handler so that
+                // handler runtime has no effect on when it is next invoked.
+                double new_next_run;
+                if (interval == 0.0)
                 {
-                    // Skip ahead to next interval after current time
-                    // This prevents spiral of death from excessive catch-up
-                    double time_behind = start_time - next_run;
-                    double intervals_to_skip = std::ceil(time_behind / interval);
-
-                    if (!std::isfinite(intervals_to_skip))
-                    {
-                        // Division overflowed to infinity - interval is extremely small, treat as ASAP
-                        new_next_run = std::nextafter(start_time, INFINITY);
-                    }
-                    else
+                    // It should run again ASAP (but not immediately)!
+                    new_next_run = std::nextafter(start_time, INFINITY);
+                }
+                else
+                {
+                    new_next_run = next_run + interval;
+                    if (new_next_run <= start_time)
                     {
-                        new_next_run = next_run + (intervals_to_skip * interval);
+                        // Would need to catch up, don't. Reset cadence instead.
+                        new_next_run = start_time + interval;
+                        if (new_next_run <= start_time)
+                        {
+                            // Can happen with very small magnitude intervals, just
+                            // schedule for the next representable time after start_time.
+                            new_next_run = std::nextafter(start_time, INFINITY);
+                        }
                     }
-                    // Ensure next tick is at least one full interval from now
-                    // This prevents "fast ticks" when waking up close to a catchup boundary
-                    new_next_run = std::max(new_next_run, start_time + interval);
-                    did_clamp = true;
                 }
 
-                // Update actual next run time (may be clamped)
                 lua_pushnumber(L, new_next_run);
                 lua_rawseti(L, CURRENT_TIMER, TIMER_NEXT_RUN);
 
-                // Update logical schedule
-                // When clamping, sync to new_next_run (reset to new reality)
-                // When not clamping, increment normally (logical_schedule + interval)
-                if (did_clamp)
-                {
-                    // Sync logical schedule when clamping - we're giving up on catch-up
-                    // so reset the logical schedule to match the new reality.
-                    // This ensures handlers see the initial delay (current fire), then return to normal.
-                    lua_pushnumber(L, new_next_run);
-                }
-                else
-                {
-                    // For interval=0, use next_scheduled since logical_schedule + 0 never changes
-                    lua_pushnumber(L, interval == 0.0 ? next_scheduled : logical_schedule + interval);
-                }
+                // TIMER_LOGICAL_SCHEDULE is a historical slot, we keep it here
+                // so we don't muck up existing states.
+                lua_pushnumber(L, new_next_run);
                 lua_rawseti(L, CURRENT_TIMER, TIMER_LOGICAL_SCHEDULE);
             }
 
@@ -694,11 +657,11 @@ DEFINE_YIELDABLE(lltimers_tick, 0)
             // No pcall(), errors bubble up to the global error handler!
             lua_pushvalue(L, HANDLER_FUNC);
             // Include when it was scheduled to run as first arg, allowing callees to do a diff between
-            // scheduled and actual time.
-            // We pass the saved handler_scheduled_time (the original logical schedule) so handlers
-            // can detect delays. When clamping occurs, the handler still receives the ORIGINAL
-            // scheduled time (when it was supposed to run), not the synced time.
-            lua_pushnumber(L, handler_scheduled_time);
+            // scheduled and actual time. We pass the pre-update next_run value, i.e. the earliest
+            // instant this tick was allowed to fire (previous_fire + interval for repeating timers,
+            // or create_time + interval for the first tick / a one-shot). `now - scheduled_time`
+            // gives the extra delay beyond the minimum interval.
+            lua_pushnumber(L, next_run);
             // Include the interval as second arg, enabling handlers to calculate missed intervals.
             // For once() timers, this will be nil. For on() timers, it's the interval.
             if (is_one_shot) {

tests/conformance/lltimers.lua

@@ -168,10 +168,10 @@ end
 LLTimers:_tick()
 assert(zero_continuous_count == 5, "Zero-interval timer should fire 5 times")
 
--- Now advance past what would be the 2-second clamping threshold for interval>0 timers
--- For interval=0: uses nextafter(next_run), no catchup needed
+-- Now jump far ahead to exercise the large-delay path
+-- For interval=0: uses nextafter(start_time), no catchup path
 -- This used to trigger division by zero: ceil(time_behind / 0)
-setclock(53.0)  -- More than 2 seconds later
+setclock(53.0)  -- Far ahead
 LLTimers:_tick()
 assert(zero_continuous_count == 6, "Zero-interval timer should continue after large time jump")
 
@@ -192,7 +192,7 @@ for i, expected_time in expected_times do
     )
 end
 
--- Test very small non-zero intervals use nextafter (no clamping)
+-- Test very small non-zero intervals that underflow against the clock magnitude
 setclock(60.0)
 local tiny_count = 0
 local tiny_scheduled_times = {}
@@ -203,30 +203,28 @@ end)
 
 -- Fire several times with small time advances
 -- Interval 1e-308 is so small that next_run + interval underflows to next_run
--- So nextafter(next_run) is used instead, ensuring forward progress
+-- So the reset branch runs every tick, with next_run driven by the clock
 for i = 1, 5 do
     incrementclock(0.01)
     LLTimers:_tick()
 end
 assert(tiny_count == 5, "Tiny interval timer should fire multiple times")
 
--- Now test catchup overflow scenario
--- Advance >2 seconds, which would cause ceil(time_behind / 1e-308) to overflow
--- The implementation should detect overflow and use nextafter(start_time) instead
-setclock(63.0)  -- More than 2 seconds later
+-- Large time jump: same reset path as a normal late tick.
+setclock(63.0)
 LLTimers:_tick()
-assert(tiny_count == 6, "Tiny interval timer should handle catchup overflow")
+assert(tiny_count == 6, "Tiny interval timer should fire after large time jump")
 
--- Verify it continues working after overflow
+-- Verify it continues working after the jump
 incrementclock(0.01)
 LLTimers:_tick()
-assert(tiny_count == 7, "Tiny interval timer should continue after overflow")
+assert(tiny_count == 7, "Tiny interval timer should continue after jump")
 
 LLTimers:off(tiny_handler)
 
 -- Verify scheduled times match expected values
--- Tiny intervals stay at 60.0 due to underflow (60.0 + 1e-308 = 60.0), then catch-up syncs to 63.0
-local tiny_expected_times = {60.0, 60.0, 60.0, 60.0, 60.0, 60.0, 63.0}
+-- Tiny interval underflows per-tick, scheduled_time steps with the clock via the reset branch
+local tiny_expected_times = {60.0, 60.011, 60.022, 60.033, 60.044, 60.055, 63.0}
 for i, expected_time in tiny_expected_times do
     local actual_time = tiny_scheduled_times[i]
     assert(
@@ -648,7 +646,7 @@ assert(math.abs(repeat_scheduled_times[1] - 35.5) < 0.001)
 assert(math.abs(repeat_scheduled_times[2] - 36.0) < 0.001)
 assert(math.abs(repeat_scheduled_times[3] - 36.5) < 0.001)
 
--- Test clamped catch-up: timers >2s late skip ahead instead of rapid-firing
+-- Test no-catchup behavior: a very-late timer fires once, then resets cadence
 setclock(40.0)
 local catchup_fires = 0
 local catchup_scheduled_times = {}
@@ -657,7 +655,7 @@ local catchup_handler = LLTimers:every(0.1, function(scheduled_time)
     table.insert(catchup_scheduled_times, scheduled_time)
 end)
 
--- Make timer VERY late (4.9 seconds, exceeds 2-second threshold)
+-- Make timer VERY late (4.9 seconds)
 setclock(45.0)
 LLEvents:_handleEvent('timer')
 
@@ -667,22 +665,19 @@ assert(catchup_fires == 1, "Timer should fire once per handleEvent call")
 -- scheduled_time parameter shows when it WAS scheduled (40.1), not when it got rescheduled to
 assert(math.abs(catchup_scheduled_times[1] - 40.1) < 0.001, "First fire shows original schedule time")
 
--- Fire again to verify logical schedule syncs when clamping
+-- Fire again to verify the cadence was reset (not resumed from the old schedule)
 setclock(45.1)
 LLEvents:_handleEvent('timer')
 assert(catchup_fires == 2, "Should fire again on next handleEvent")
--- When we clamp, we sync the logical schedule to the new reality
--- This means handlers see the initial delay (first fire), then return to normal
-assert(catchup_scheduled_times[2] > 44.9, "Second fire shows synced schedule (~45.0)")
-assert(catchup_scheduled_times[2] < 45.2, "Second fire shows synced schedule (~45.0)")
--- Handler sees normal delay now: getclock() - scheduled_time = 45.1 - 45.0 = ~0.1s
+-- Handler now sees the reset schedule (~45.1), not the ~40.2 it would see under old catchup
+assert(catchup_scheduled_times[2] > 44.9, "Second fire shows reset schedule (~45.1)")
+assert(catchup_scheduled_times[2] < 45.2, "Second fire shows reset schedule (~45.1)")
+-- Handler sees normal delay now: getclock() - scheduled_time = 45.1 - 45.1 = ~0s
 
 -- Clean up
 LLTimers:off(catchup_handler)
 
--- Test minimum interval guarantee after catchup
--- After waking from a long sleep near a catchup boundary, the SECOND tick should
--- never fire faster than the specified interval. Regression test for "overcorrection" bug.
+-- Test the "never sooner than interval" guarantee after a long delay.
 setclock(0.0)
 local min_interval_fires = 0
 local min_interval_times = {}
@@ -692,7 +687,7 @@ local min_interval_handler = LLTimers:every(1.0, function(scheduled_time)
 end)
 
 -- Timer created at T=0, first scheduled for T=1.0
--- Simulate script disable/re-enable: jump forward 5.5s (past 2s catchup threshold)
+-- Simulate script disable/re-enable: jump forward 5.5s (well past next_run)
 setclock(5.5)
 LLEvents:_handleEvent('timer')
 assert(min_interval_fires == 1, "First tick should fire immediately after wake")