From b9ab1c6fd7fa22ac63450aa075bc99f3565b9efa Mon Sep 17 00:00:00 2001 From: Alex Christoffer Rasmussen Date: Thu, 5 Jun 2025 06:13:49 +0200 Subject: [PATCH] Refactor: better rounding (#1594) --- apps/client/src/common/utils/time.ts | 2 +- .../src/features/viewers/common/viewUtils.ts | 4 +- .../runtime-service/RuntimeService.ts | 41 ++++++---- .../runtime-service/rundownService.utils.ts | 19 ++--- e2e/tests/features/212-time-until.spec.ts | 4 +- packages/utils/index.ts | 2 - .../src/date-utils/conversionUtils.test.ts | 78 +------------------ .../utils/src/date-utils/conversionUtils.ts | 47 ++++------- .../src/date-utils/timeFormatting.test.ts | 72 +++++++++-------- .../utils/src/date-utils/timeFormatting.ts | 5 +- 10 files changed, 93 insertions(+), 181 deletions(-) diff --git a/apps/client/src/common/utils/time.ts b/apps/client/src/common/utils/time.ts index 0778fe1f6..760be0f54 100644 --- a/apps/client/src/common/utils/time.ts +++ b/apps/client/src/common/utils/time.ts @@ -119,7 +119,7 @@ export function formatDuration(duration: number, hideSeconds = true): string { } if (!hideSeconds) { - const seconds = Math.floor((duration % MILLIS_PER_MINUTE) / MILLIS_PER_SECOND); + const seconds = Math.ceil((duration % MILLIS_PER_MINUTE) / MILLIS_PER_SECOND); if (seconds > 0) { result += `${seconds}s`; } diff --git a/apps/client/src/features/viewers/common/viewUtils.ts b/apps/client/src/features/viewers/common/viewUtils.ts index cb4093a55..27379f357 100644 --- a/apps/client/src/features/viewers/common/viewUtils.ts +++ b/apps/client/src/features/viewers/common/viewUtils.ts @@ -81,7 +81,7 @@ export function getFormattedTimer( localisedMinutes: string, options: FormattingOptions, ): string { - if (timer == null) { + if (timer == null || timerType === TimerType.None) { return options.removeSeconds ? timerPlaceholderMin : timerPlaceholder; } @@ -98,7 +98,7 @@ export function getFormattedTimer( } } - let display = millisToString(timeToParse); + let display = millisToString(timeToParse, { direction: timerType }); if (options.removeLeadingZero) { display = removeLeadingZero(display); } diff --git a/apps/server/src/services/runtime-service/RuntimeService.ts b/apps/server/src/services/runtime-service/RuntimeService.ts index 81f5e66a7..051aae0c6 100644 --- a/apps/server/src/services/runtime-service/RuntimeService.ts +++ b/apps/server/src/services/runtime-service/RuntimeService.ts @@ -717,6 +717,11 @@ function broadcastResult(_target: any, _propertyKey: string, descriptor: Propert // combine all big changes const hasImmediateChanges = hasNewLoaded || justStarted || hasChangedPlayback || offsetModeChanged; + // we would like the wall clock to tick on a regular rate + const normalClockUpdate = + getShouldClockUpdate(RuntimeService.previousClockUpdate, state.clock) || + getForceUpdate(RuntimeService.previousClockUpdate, state.clock); + /** * Timer should be updated if * - big changes @@ -733,6 +738,7 @@ function broadcastResult(_target: any, _propertyKey: string, descriptor: Propert /** * Runtime should be updated if + * - clock tick * - big changes * - the timer is updating so runtime also updates to keep them in sync ??? * - notification rate has been exceeded @@ -740,7 +746,10 @@ function broadcastResult(_target: any, _propertyKey: string, descriptor: Propert * Then check if there is actually a change in the data */ const shouldRuntimeUpdate = - (hasImmediateChanges || shouldUpdateTimer || getForceUpdate(RuntimeService.previousRuntimeUpdate, state.clock)) && + (normalClockUpdate || + hasImmediateChanges || + shouldUpdateTimer || + getForceUpdate(RuntimeService.previousRuntimeUpdate, state.clock)) && !deepEqual(RuntimeService.previousState?.runtime, state.runtime); /** @@ -751,13 +760,9 @@ function broadcastResult(_target: any, _propertyKey: string, descriptor: Propert /** * Many other values are calculated based on the clock * so if any of them are updated we also need to send the clock - * in case nothing else is updating the clock will bw updated at the notification rate + * in case nothing else is updating the clock will be updated at the notification rate */ - const shouldUpdateClock = - shouldUpdateTimer || - shouldRuntimeUpdate || - shouldBlockUpdate || - getForceUpdate(RuntimeService.previousClockUpdate, state.clock); + const shouldUpdateClock = shouldRuntimeUpdate || shouldBlockUpdate || normalClockUpdate; //Now we set all the updates on the eventstore and update the previous value if (hasChangedPlayback) { @@ -828,15 +833,19 @@ function broadcastResult(_target: any, _propertyKey: string, descriptor: Propert // Helper function to save the restore state function saveRestoreState(state: runtimeState.RuntimeState) { - restoreService.save({ - playback: state.timer.playback, - selectedEventId: state.eventNow?.id ?? null, - startedAt: state.timer.startedAt, - addedTime: state.timer.addedTime, - pausedAt: state._timer.pausedAt, - firstStart: state.runtime.actualStart, - blockStartAt: state.currentBlock.startedAt, - }); + restoreService + .save({ + playback: state.timer.playback, + selectedEventId: state.eventNow?.id ?? null, + startedAt: state.timer.startedAt, + addedTime: state.timer.addedTime, + pausedAt: state._timer.pausedAt, + firstStart: state.runtime.actualStart, + blockStartAt: state.currentBlock.startedAt, + }) + .catch((_e) => { + //we don't do anything with the error here + }); } batch.send(); diff --git a/apps/server/src/services/runtime-service/rundownService.utils.ts b/apps/server/src/services/runtime-service/rundownService.utils.ts index 0ecc2ab9a..a551d64f2 100644 --- a/apps/server/src/services/runtime-service/rundownService.utils.ts +++ b/apps/server/src/services/runtime-service/rundownService.utils.ts @@ -1,20 +1,16 @@ import { millisToSeconds } from 'ontime-utils'; -import { MaybeNumber } from 'ontime-types'; +import { MaybeNumber, TimerType } from 'ontime-types'; import { timerConfig } from '../../setup/config.js'; /** * Checks whether we should update the clock value - * - clock has slid * - we have rolled into a new seconds unit + * this is different from the timer update as it looks at the clock as counting up */ export function getShouldClockUpdate(previousUpdate: number, now: number): boolean { - const shouldForceUpdate = getForceUpdate(previousUpdate, now); - if (shouldForceUpdate) { - return true; - } - const isClockSecondAhead = millisToSeconds(now) !== millisToSeconds(previousUpdate + timerConfig.triggerAhead); - return isClockSecondAhead; + const newSeconds = millisToSeconds(now, TimerType.CountUp) !== millisToSeconds(previousUpdate, TimerType.CountUp); + return newSeconds; } /** @@ -22,10 +18,6 @@ export function getShouldClockUpdate(previousUpdate: number, now: number): boole * - we have rolled into a new seconds unit */ export function getShouldTimerUpdate(previousValue: MaybeNumber, currentValue: MaybeNumber): boolean { - if (currentValue === null) { - return false; - } - // we avoid trigger ahead since it can cause duplicate triggers const shouldUpdateTimer = millisToSeconds(currentValue) !== millisToSeconds(previousValue); return shouldUpdateTimer; } @@ -39,6 +31,5 @@ export function getShouldTimerUpdate(previousValue: MaybeNumber, currentValue: M export function getForceUpdate(previousUpdate: number, now: number): boolean { const isClockBehind = now < previousUpdate; const hasExceededRate = now - previousUpdate >= timerConfig.notificationRate; - const newSeconds = millisToSeconds(previousUpdate) !== millisToSeconds(now); - return isClockBehind || hasExceededRate || newSeconds; + return isClockBehind || hasExceededRate; } diff --git a/e2e/tests/features/212-time-until.spec.ts b/e2e/tests/features/212-time-until.spec.ts index 81b5beb40..146be2f16 100644 --- a/e2e/tests/features/212-time-until.spec.ts +++ b/e2e/tests/features/212-time-until.spec.ts @@ -32,7 +32,7 @@ test('time until absolute', async ({ page }) => { await page.getByTestId('entry-1').getByTestId('time-input-duration').fill('30s'); await page.getByTestId('entry-1').getByTestId('time-input-duration').press('Enter'); - await expect(page.getByTestId('entry-2').locator('#event-block')).toContainText('29s'); + await expect(page.getByTestId('entry-2').locator('#event-block')).toContainText('30s'); await expect(page.getByTestId('entry-3').locator('#event-block')).toContainText('10m'); await expect(page.getByTestId('entry-4').locator('#event-block')).toContainText('20m'); }); @@ -69,7 +69,7 @@ test('time until relative', async ({ page }) => { await page.getByTestId('entry-1').getByTestId('time-input-duration').fill('30s'); await page.getByTestId('entry-1').getByTestId('time-input-duration').press('Enter'); - await expect(page.getByTestId('entry-2').locator('#event-block')).toContainText('29s'); + await expect(page.getByTestId('entry-2').locator('#event-block')).toContainText('30s'); await expect(page.getByTestId('entry-3').locator('#event-block')).toContainText('10m'); await expect(page.getByTestId('entry-4').locator('#event-block')).toContainText('20m'); }); diff --git a/packages/utils/index.ts b/packages/utils/index.ts index 452044089..a9d6bb3f4 100644 --- a/packages/utils/index.ts +++ b/packages/utils/index.ts @@ -36,8 +36,6 @@ export { MILLIS_PER_HOUR, MILLIS_PER_MINUTE, MILLIS_PER_SECOND, - millisToHours, - millisToMinutes, millisToSeconds, secondsInMillis, } from './src/date-utils/conversionUtils.js'; diff --git a/packages/utils/src/date-utils/conversionUtils.test.ts b/packages/utils/src/date-utils/conversionUtils.test.ts index 5cf99c11d..4ee5ab2d3 100644 --- a/packages/utils/src/date-utils/conversionUtils.test.ts +++ b/packages/utils/src/date-utils/conversionUtils.test.ts @@ -1,4 +1,4 @@ -import { millisToHours, millisToMinutes, millisToSeconds, secondsInMillis } from './conversionUtils'; +import { millisToSeconds, secondsInMillis } from './conversionUtils'; describe('millisToSecond()', () => { test('null values', () => { @@ -37,82 +37,6 @@ describe('millisToSecond()', () => { }); }); -describe('millisToMinutes()', () => { - test('null values', () => { - const t = { val: null, result: 0 }; - expect(millisToMinutes(t.val)).toBe(t.result); - }); - - test('valid millis', () => { - const t = { val: 3600000, result: 60 }; - expect(millisToMinutes(t.val)).toBe(t.result); - }); - - test('negative millis', () => { - const t = { val: -3600000, result: -60 }; - expect(millisToMinutes(t.val)).toBe(t.result); - }); - - test('0', () => { - const t = { val: 0, result: 0 }; - expect(millisToMinutes(t.val)).toBe(t.result); - }); - - test('-0', () => { - const t = { val: -0, result: 0 }; - expect(millisToMinutes(t.val)).toBe(t.result); - }); - - test('86401000 (24 hours and 1 second)', () => { - const t = { val: 86401000, result: 1440 }; - expect(millisToMinutes(t.val)).toBe(t.result); - }); - - test('-86401000 (-24 hours and 1 second)', () => { - // negative numbers are rounded up - const t = { val: -86401000, result: -1441 }; - expect(millisToMinutes(t.val)).toBe(t.result); - }); -}); - -describe('millisToHours()', () => { - test('null values', () => { - const t = { val: null, result: 0 }; - expect(millisToHours(t.val)).toBe(t.result); - }); - - test('valid millis', () => { - const t = { val: 3600000, result: 1 }; - expect(millisToHours(t.val)).toBe(t.result); - }); - - test('negative millis', () => { - const t = { val: -3600000, result: -1 }; - expect(millisToHours(t.val)).toBe(t.result); - }); - - test('0', () => { - const t = { val: 0, result: 0 }; - expect(millisToHours(t.val)).toBe(t.result); - }); - - test('-0', () => { - const t = { val: -0, result: 0 }; - expect(millisToHours(t.val)).toBe(t.result); - }); - - test('86401000 (24 hours and 1 second)', () => { - const t = { val: 86401000, result: 24 }; - expect(millisToHours(t.val)).toBe(t.result); - }); - - test('-86401000 (-24 hours and 1 second)', () => { - // negative numbers are rounded up - const t = { val: -86401000, result: -25 }; - expect(millisToHours(t.val)).toBe(t.result); - }); -}); - describe('secondsInMillis()', () => { it('return 0 if value is null', () => { expect(secondsInMillis(null)).toBe(0); diff --git a/packages/utils/src/date-utils/conversionUtils.ts b/packages/utils/src/date-utils/conversionUtils.ts index d245fe1b5..d1fd6808f 100644 --- a/packages/utils/src/date-utils/conversionUtils.ts +++ b/packages/utils/src/date-utils/conversionUtils.ts @@ -1,4 +1,5 @@ import type { MaybeNumber } from 'ontime-types'; +import { TimerType } from 'ontime-types'; export const MILLIS_PER_SECOND = 1000; export const MILLIS_PER_MINUTE = 1000 * 60; @@ -7,44 +8,28 @@ export const MILLIS_PER_HOUR = 1000 * 60 * 60; export const dayInMs = 86400000; export const maxDuration = dayInMs - MILLIS_PER_SECOND; -/** - * Utility converts milliseconds to a specific unit - * @param millis - * @param conversion - * @returns - */ -function convertMillis(millis: MaybeNumber, conversion: number): number { - if (!millis) { - return 0; - } - return Math.floor(millis / conversion); -} - /** * Converts value in milliseconds to seconds * @param millis * @returns */ -export function millisToSeconds(millis: MaybeNumber): number { - return convertMillis(millis, MILLIS_PER_SECOND); -} +export function millisToSeconds( + millis: MaybeNumber, + direction: TimerType.CountDown | TimerType.CountUp = TimerType.CountDown, +) { + if (millis === null) return 0; -/** - * Converts value in milliseconds to minutes - * @param millis - * @returns - */ -export function millisToMinutes(millis: MaybeNumber): number { - return convertMillis(millis, MILLIS_PER_MINUTE); -} + let seconds = 0; + if (direction === TimerType.CountDown) { + seconds = Math.ceil(millis / MILLIS_PER_SECOND); + } -/** - * Converts value in milliseconds to hours - * @param millis - * @returns - */ -export function millisToHours(millis: MaybeNumber): number { - return convertMillis(millis, MILLIS_PER_HOUR); + if (direction === TimerType.CountUp) { + seconds = Math.floor(millis / MILLIS_PER_SECOND); + } + + // this is there to avoid result giving -0 + return seconds === 0 ? 0 : seconds; } /** diff --git a/packages/utils/src/date-utils/timeFormatting.test.ts b/packages/utils/src/date-utils/timeFormatting.test.ts index f0b114dec..fac223859 100644 --- a/packages/utils/src/date-utils/timeFormatting.test.ts +++ b/packages/utils/src/date-utils/timeFormatting.test.ts @@ -1,3 +1,5 @@ +import { TimerType } from 'ontime-types'; + import { dayInMs, MILLIS_PER_HOUR } from './conversionUtils'; import { formatFromMillis, millisToString, removeLeadingZero } from './timeFormatting'; @@ -13,32 +15,33 @@ describe('millisToString()', () => { test('negative times are rounded up', () => { const testScenarios = [ - { millis: 300, expected: '00:00:00' }, - { millis: -300, expected: '-00:00:01' }, - { millis: 1000, expected: '00:00:01' }, - { millis: -1000, expected: '-00:00:01' }, - { millis: 1500, expected: '00:00:01' }, - { millis: -1500, expected: '-00:00:02' }, - { millis: 60000 - 1, expected: '00:00:59' }, - { millis: -(60000 - 1), expected: '-00:01:00' }, - { millis: 60000, expected: '00:01:00' }, - { millis: -60000, expected: '-00:01:00' }, - { millis: 600000, expected: '00:10:00' }, - { millis: -600000, expected: '-00:10:00' }, - { millis: 3600000, expected: '01:00:00' }, - { millis: -3600000, expected: '-01:00:00' }, - { millis: 36000000, expected: '10:00:00' }, - { millis: -36000000, expected: '-10:00:00' }, - { millis: 86399000, expected: '23:59:59' }, - { millis: -86399000, expected: '-23:59:59' }, - { millis: 86400000, expected: '24:00:00' }, - { millis: -86400000, expected: '-24:00:00' }, - { millis: 86401000, expected: '24:00:01' }, - { millis: -86401000, expected: '-24:00:01' }, + { millis: 300, expected_down: '00:00:01', expected_up: '00:00:00' }, + { millis: -300, expected_down: '-00:00:00', expected_up: '-00:00:01' }, + { millis: 1000, expected_down: '00:00:01', expected_up: '00:00:01' }, + { millis: -1000, expected_down: '-00:00:01', expected_up: '-00:00:01' }, + { millis: 1500, expected_down: '00:00:02', expected_up: '00:00:01' }, + { millis: -1500, expected_down: '-00:00:01', expected_up: '-00:00:02' }, + { millis: 60000 - 1, expected_down: '00:01:00', expected_up: '00:00:59' }, + { millis: -(60000 - 1), expected_down: '-00:00:59', expected_up: '-00:01:00' }, + { millis: 60000, expected_down: '00:01:00', expected_up: '00:01:00' }, + { millis: -60000, expected_down: '-00:01:00', expected_up: '-00:01:00' }, + { millis: 600000, expected_down: '00:10:00', expected_up: '00:10:00' }, + { millis: -600000, expected_down: '-00:10:00', expected_up: '-00:10:00' }, + { millis: 3600000, expected_down: '01:00:00', expected_up: '01:00:00' }, + { millis: -3600000, expected_down: '-01:00:00', expected_up: '-01:00:00' }, + { millis: 36000000, expected_down: '10:00:00', expected_up: '10:00:00' }, + { millis: -36000000, expected_down: '-10:00:00', expected_up: '-10:00:00' }, + { millis: 86399000, expected_down: '23:59:59', expected_up: '23:59:59' }, + { millis: -86399000, expected_down: '-23:59:59', expected_up: '-23:59:59' }, + { millis: 86400000, expected_down: '24:00:00', expected_up: '24:00:00' }, + { millis: -86400000, expected_down: '-24:00:00', expected_up: '-24:00:00' }, + { millis: 86401000, expected_down: '24:00:01', expected_up: '24:00:01' }, + { millis: -86401000, expected_down: '-24:00:01', expected_up: '-24:00:01' }, ]; testScenarios.forEach((scenario) => { - expect(millisToString(scenario.millis)).toBe(scenario.expected); + expect(millisToString(scenario.millis, { direction: TimerType.CountDown })).toBe(scenario.expected_down); + expect(millisToString(scenario.millis, { direction: TimerType.CountUp })).toBe(scenario.expected_up); }); }); @@ -48,20 +51,21 @@ describe('millisToString()', () => { test('random properties', () => { const testScenarios = [ - { millis: 300, expected: '00:00:00' }, - { millis: 1000, expected: '00:00:01' }, - { millis: 1500, expected: '00:00:01' }, - { millis: 60000, expected: '00:01:00' }, - { millis: 600000, expected: '00:10:00' }, - { millis: 3600000, expected: '01:00:00' }, - { millis: 36000000, expected: '10:00:00' }, - { millis: 86399000, expected: '23:59:59' }, - { millis: 86400000, expected: '24:00:00' }, - { millis: 86401000, expected: '24:00:01' }, + { millis: 300, expected_down: '00:00:01', expected_up: '00:00:00' }, + { millis: 1000, expected_down: '00:00:01', expected_up: '00:00:01' }, + { millis: 1500, expected_down: '00:00:02', expected_up: '00:00:01' }, + { millis: 60000, expected_down: '00:01:00', expected_up: '00:01:00' }, + { millis: 600000, expected_down: '00:10:00', expected_up: '00:10:00' }, + { millis: 3600000, expected_down: '01:00:00', expected_up: '01:00:00' }, + { millis: 36000000, expected_down: '10:00:00', expected_up: '10:00:00' }, + { millis: 86399000, expected_down: '23:59:59', expected_up: '23:59:59' }, + { millis: 86400000, expected_down: '24:00:00', expected_up: '24:00:00' }, + { millis: 86401000, expected_down: '24:00:01', expected_up: '24:00:01' }, ]; testScenarios.forEach((scenario) => { - expect(millisToString(scenario.millis)).toBe(scenario.expected); + expect(millisToString(scenario.millis, { direction: TimerType.CountDown })).toBe(scenario.expected_down); + expect(millisToString(scenario.millis, { direction: TimerType.CountUp })).toBe(scenario.expected_up); }); }); }); diff --git a/packages/utils/src/date-utils/timeFormatting.ts b/packages/utils/src/date-utils/timeFormatting.ts index 01b3b3254..400cec36d 100644 --- a/packages/utils/src/date-utils/timeFormatting.ts +++ b/packages/utils/src/date-utils/timeFormatting.ts @@ -1,4 +1,4 @@ -import type { MaybeNumber } from 'ontime-types'; +import type { MaybeNumber, TimerType } from 'ontime-types'; import { millisToSeconds, secondsToHours, secondsToMinutes } from './conversionUtils.js'; @@ -8,6 +8,7 @@ export function pad(val: number): string { type FormatOptions = { fallback?: string; + direction?: TimerType.CountDown | TimerType.CountUp; }; /** @@ -23,7 +24,7 @@ export function millisToString(millis?: MaybeNumber, options?: FormatOptions): s const isNegative = millis < 0; - const totalSeconds = Math.abs(millisToSeconds(millis)); + const totalSeconds = Math.abs(millisToSeconds(millis, options?.direction)); const seconds = totalSeconds % 60; const minutes = secondsToMinutes(totalSeconds) % 60; const hours = secondsToHours(totalSeconds);