From 25c7915cf770e42540fcd866cc1683ad6452064d Mon Sep 17 00:00:00 2001 From: Carlos Valente Date: Mon, 26 Aug 2024 10:49:17 +0200 Subject: [PATCH] refactor: account for overlapping in rundown --- .../rundown/event-block/EventBlock.utils.ts | 13 ++-- .../__tests__/rundownCache.test.ts | 77 +++++++++++++++---- .../services/rundown-service/rundownCache.ts | 52 +++++++++---- packages/utils/index.ts | 1 - .../utils/src/date-utils/checkOverlap.test.ts | 16 ---- packages/utils/src/date-utils/checkOverlap.ts | 19 ----- .../date-utils/getTimeFromPrevious.test.ts | 36 ++++++++- .../src/date-utils/getTimeFromPrevious.ts | 17 ++-- 8 files changed, 149 insertions(+), 82 deletions(-) delete mode 100644 packages/utils/src/date-utils/checkOverlap.test.ts delete mode 100644 packages/utils/src/date-utils/checkOverlap.ts diff --git a/apps/client/src/features/rundown/event-block/EventBlock.utils.ts b/apps/client/src/features/rundown/event-block/EventBlock.utils.ts index b45750ac5..e42898419 100644 --- a/apps/client/src/features/rundown/event-block/EventBlock.utils.ts +++ b/apps/client/src/features/rundown/event-block/EventBlock.utils.ts @@ -18,14 +18,13 @@ export function formatOverlap( const noPreviousElement = previousEnd === null || previousStart === null; if (noPreviousElement) return; - const overlap = previousEnd - timeStart; - if (overlap === 0) return; + const timeFromPrevious = previousEnd - timeStart; + if (timeFromPrevious === 0) return; const previousCrossMidnight = previousStart > previousEnd; const isNextDay = previousCrossMidnight - ? checkIsNextDay(previousEnd, timeStart) || previousEnd == 0 // exception for when previousEnd is precisely midnight - : checkIsNextDay(previousStart, timeStart); - + ? previousEnd === 0 || checkIsNextDay(previousEnd, timeStart, previousEnd - previousStart) // exception for when previousEnd is precisely midnight + : checkIsNextDay(previousStart, timeStart, previousEnd - previousStart); const correctedPreviousEnd = previousCrossMidnight ? previousEnd + dayInMs : previousEnd; if (isNextDay) { @@ -35,6 +34,6 @@ export function formatOverlap( return `Gap ${gapString} (next day)`; } - const overlapString = removeLeadingZero(millisToString(Math.abs(overlap))); - return `${overlap > 0 ? 'Overlap' : 'Gap'} ${overlapString}`; + const overlapString = removeLeadingZero(millisToString(Math.abs(timeFromPrevious))); + return `${timeFromPrevious > 0 ? 'Overlap' : 'Gap'} ${overlapString}`; } diff --git a/apps/server/src/services/rundown-service/__tests__/rundownCache.test.ts b/apps/server/src/services/rundown-service/__tests__/rundownCache.test.ts index b0cabe21a..aadab9a6b 100644 --- a/apps/server/src/services/rundown-service/__tests__/rundownCache.test.ts +++ b/apps/server/src/services/rundown-service/__tests__/rundownCache.test.ts @@ -10,7 +10,7 @@ import { TimeStrategy, TimerType, } from 'ontime-types'; -import { MILLIS_PER_HOUR, dayInMs, millisToString } from 'ontime-utils'; +import { MILLIS_PER_HOUR, MILLIS_PER_MINUTE, dayInMs } from 'ontime-utils'; import { calculateRuntimeDelays, getDelayAt, calculateRuntimeDelaysFrom } from '../delayUtils.js'; import { @@ -89,6 +89,65 @@ describe('generate()', () => { expect(initResult.totalDuration).toBe(700 - 100); }); + it('accounts for overlaps in rundown', () => { + const testRundown: OntimeRundown = [ + { type: SupportedEvent.Event, id: '1', timeStart: 9000, timeEnd: 10000, duration: 1000 } as OntimeEvent, + { type: SupportedEvent.Event, id: '2', timeStart: 9250, timeEnd: 9500, duration: 250 } as OntimeEvent, + { type: SupportedEvent.Event, id: '3', timeStart: 9500, timeEnd: 10500, duration: 1000 } as OntimeEvent, + ]; + + const initResult = generate(testRundown); + expect(initResult.totalDuration).toBe(10500 - 9000); // last end - first start + }); + + it('accounts for overlaps in rundown (with added gap)', () => { + const testRundown: OntimeRundown = [ + { type: SupportedEvent.Event, id: '1', timeStart: 9000, timeEnd: 10000, duration: 1000 } as OntimeEvent, + { type: SupportedEvent.Event, id: '2', timeStart: 9250, timeEnd: 9500, duration: 250 } as OntimeEvent, + { type: SupportedEvent.Event, id: '3', timeStart: 9500, timeEnd: 10500, duration: 1000 } as OntimeEvent, + { type: SupportedEvent.Event, id: '4', timeStart: 15000, timeEnd: 20000, duration: 5000 } as OntimeEvent, + ]; + + const initResult = generate(testRundown); + expect(initResult.totalDuration).toBe(20000 - 9000); // last end - first start + }); + + it('accounts for overlaps in rundown (with multiple days)', () => { + const testRundown: OntimeRundown = [ + { + type: SupportedEvent.Event, + id: '1', + timeStart: 9 * MILLIS_PER_HOUR, + timeEnd: 10 * MILLIS_PER_HOUR, + duration: MILLIS_PER_HOUR, + } as OntimeEvent, + { + type: SupportedEvent.Event, + id: '2', + timeStart: 9 * MILLIS_PER_HOUR + 15 * MILLIS_PER_MINUTE, + timeEnd: 9 * MILLIS_PER_HOUR + 45 * MILLIS_PER_MINUTE, + duration: 30 * MILLIS_PER_MINUTE, + } as OntimeEvent, + { + type: SupportedEvent.Event, + id: '3', + timeStart: 9 * MILLIS_PER_HOUR + 30 * MILLIS_PER_MINUTE, + timeEnd: 10 * MILLIS_PER_HOUR + 30 * MILLIS_PER_MINUTE, + duration: MILLIS_PER_HOUR, + } as OntimeEvent, + { + type: SupportedEvent.Event, + id: '4', + timeStart: 9 * MILLIS_PER_HOUR, + timeEnd: 10 * MILLIS_PER_HOUR, + duration: MILLIS_PER_HOUR, + } as OntimeEvent, + ]; + + const initResult = generate(testRundown); + expect(initResult.totalDuration).toBe(dayInMs + MILLIS_PER_HOUR); // day + last end - first start + }); + it('handles negative delays', () => { const testRundown: OntimeRundown = [ { type: SupportedEvent.Event, id: '1', timeStart: 100, timeEnd: 200, duration: 100 } as OntimeEvent, @@ -362,7 +421,7 @@ describe('generate()', () => { ]; const initResult = generate(testRundown, customProperties); expect(initResult.order.length).toBe(2); - expect(initResult.assignedCustomProperties).toMatchObject({ + expect(initResult.assignedCustomFields).toMatchObject({ lighting: ['1', '2'], sound: ['2'], }); @@ -498,20 +557,6 @@ describe('swap() mutation', () => { }); }); -/** - * - * - * - * - * - * - * - * - * - * - * - */ - describe('calculateRuntimeDelays', () => { it('calculates all delays in a given rundown', () => { const rundown: OntimeRundown = [ diff --git a/apps/server/src/services/rundown-service/rundownCache.ts b/apps/server/src/services/rundown-service/rundownCache.ts index c020d3e4e..3e78bfba4 100644 --- a/apps/server/src/services/rundown-service/rundownCache.ts +++ b/apps/server/src/services/rundown-service/rundownCache.ts @@ -11,7 +11,14 @@ import { OntimeRundownEntry, PlayableEvent, } from 'ontime-types'; -import { generateId, insertAtIndex, reorderArray, swapEventData, getTimeFromPrevious } from 'ontime-utils'; +import { + generateId, + insertAtIndex, + reorderArray, + swapEventData, + getTimeFromPrevious, + checkIsNextDay, +} from 'ontime-utils'; import { getDataProvider } from '../../classes/data-provider/DataProvider.js'; import { createPatch } from '../../utils/parser.js'; import { apply } from './delayUtils.js'; @@ -83,7 +90,6 @@ export function generate( totalDuration = 0; totalDelay = 0; - let previousEntry: PlayableEvent | null = null; let lastEntry: PlayableEvent | null = null; for (let i = 0; i < initialRundown.length; i++) { @@ -100,31 +106,48 @@ export function generate( // update rundown metadata, it only concerns playable events if (isPlayableEvent(currentEntry)) { + // fist start is always the first event if (firstStart === null) { firstStart = currentEntry.timeStart; } - // TODO: carry on last event - lastEnd = currentEntry.timeEnd; const timeFromPrevious: number = getTimeFromPrevious( currentEntry.timeStart, currentEntry.timeEnd, - previousEntry?.timeStart, - previousEntry?.timeEnd, - previousEntry?.duration, + lastEntry?.timeStart, + lastEntry?.timeEnd, + lastEntry?.duration, ); - totalDuration += timeFromPrevious + currentEntry.duration; + + if (timeFromPrevious === 0) { + // event starts on previous finish, we add its duration + totalDuration += currentEntry.duration; + } else if (timeFromPrevious > 0) { + // event has a gap, we add the gap and the duration + totalDuration += timeFromPrevious + currentEntry.duration; + } else if (timeFromPrevious < 0) { + // there is an overlap, we remove the overlap from the duration + // ensuring that the sum is not negative (ie: fully overlapped events) + // NOTE: we add the gap since it is a negative number + totalDuration += Math.max(currentEntry.duration + timeFromPrevious, 0); + } // remove eventual gaps from the accumulated delay // we only affect positive delays (time forwards) - if (totalDelay > 0 && previousEntry) { - const gap = Math.max(currentEntry.timeStart - previousEntry.timeEnd, 0); - totalDelay = Math.max(totalDelay - gap, 0); + if (totalDelay > 0 && timeFromPrevious > 0) { + totalDelay = Math.max(totalDelay - timeFromPrevious, 0); } // current event delay is the current accumulated delay currentEntry.delay = totalDelay; - // keep copy of event - previousEntry = currentEntry; + + // lastEntry is the event with the latest end time + if ( + lastEntry === null || + currentEntry.timeEnd > lastEntry.timeEnd || + checkIsNextDay(lastEntry.timeStart, currentEntry.timeStart, lastEntry.duration) + ) { + lastEntry = currentEntry; + } } } @@ -147,9 +170,10 @@ export function generate( rundown[currentEntry.id] = currentEntry; } + lastEnd = lastEntry?.timeEnd ?? null; isStale = false; customFieldChangelog.clear(); - return { rundown, order, links, totalDelay, totalDuration, assignedCustomProperties: assignedCustomFields }; + return { rundown, order, links, totalDelay, totalDuration, assignedCustomFields }; } /** Returns an ID guaranteed to be unique */ diff --git a/packages/utils/index.ts b/packages/utils/index.ts index 4800c38f0..a2ec92a35 100644 --- a/packages/utils/index.ts +++ b/packages/utils/index.ts @@ -75,7 +75,6 @@ export { validateEndAction, validateTimerType } from './src/validate-events/vali // feature business logic - rundown export { checkIsNow } from './src/date-utils/checkIsNow.js'; export { checkIsNextDay } from './src/date-utils/checkIsNextDay.js'; -export { checkOverlap } from './src/date-utils/checkOverlap.js'; export { getTimeFromPrevious } from './src/date-utils/getTimeFromPrevious.js'; // feature business logic - spreadsheet import diff --git a/packages/utils/src/date-utils/checkOverlap.test.ts b/packages/utils/src/date-utils/checkOverlap.test.ts deleted file mode 100644 index 504ce00d9..000000000 --- a/packages/utils/src/date-utils/checkOverlap.test.ts +++ /dev/null @@ -1,16 +0,0 @@ -import { checkOverlap } from './checkOverlap'; - -describe('checkOverlap', () => { - it('should return true if events fully overlap', () => { - expect(checkOverlap(1000, 2000, 1000, 2000)).toBe(true); - }); - - it('should return true if one of the events is inside the other', () => { - expect(checkOverlap(1000, 2000, 1000, 1000)).toBe(true); - expect(checkOverlap(1000, 2000, 1500, 1750)).toBe(true); - }); - - it("should return false if events don't overlap", () => { - expect(checkOverlap(1000, 2000, 2000, 3000)).toBe(false); - }); -}); diff --git a/packages/utils/src/date-utils/checkOverlap.ts b/packages/utils/src/date-utils/checkOverlap.ts deleted file mode 100644 index 976f4468c..000000000 --- a/packages/utils/src/date-utils/checkOverlap.ts +++ /dev/null @@ -1,19 +0,0 @@ -/** - * Check if two Ontime events overlap - * @link https://stackoverflow.com/questions/3269434/whats-the-most-efficient-way-to-test-if-two-ranges-overlap - * We use the deconstructed times to facilitate implementation in UI - */ -export function checkOverlap( - previousStart: number, - previousEnd: number, - currentStart: number, - currentEnd: number, -): boolean { - // deal with simple case where the event is later - if (currentStart >= previousEnd) { - return false; - } - - // at this point we know there may be an overlap - return Math.max(previousStart, currentStart) - Math.min(previousEnd, currentEnd) <= 0; -} diff --git a/packages/utils/src/date-utils/getTimeFromPrevious.test.ts b/packages/utils/src/date-utils/getTimeFromPrevious.test.ts index 30e75d524..944a862cb 100644 --- a/packages/utils/src/date-utils/getTimeFromPrevious.test.ts +++ b/packages/utils/src/date-utils/getTimeFromPrevious.test.ts @@ -1,3 +1,4 @@ +import { dayInMs, MILLIS_PER_HOUR } from './conversionUtils'; import { getTimeFromPrevious } from './getTimeFromPrevious'; describe('getTimeFromPrevious', () => { @@ -7,7 +8,40 @@ describe('getTimeFromPrevious', () => { const previousDuration = 2100000; // 35 minutes const currentStart = 75600000; // 21:00 const currentEnd = 81000000; // 22:30 - const expected = 75600000 - 71700000; // current staart - previousEnd + const expected = 75600000 - 71700000; // current start - previousEnd + + expect(getTimeFromPrevious(currentStart, currentEnd, previousStart, previousEnd, previousDuration)).toBe(expected); + }); + + it('accounts for partially overlapping events', () => { + const previousStart = 10; + const previousEnd = 12; + const previousDuration = 2; + const currentStart = 11; + const currentEnd = 12; + const expected = -(previousEnd - currentStart); + + expect(getTimeFromPrevious(currentStart, currentEnd, previousStart, previousEnd, previousDuration)).toBe(expected); + }); + + it('accounts for events that are fully contained', () => { + const previousStart = 8; + const previousEnd = 16; + const previousDuration = 8; + const currentStart = 10; + const currentEnd = 15; + const expected = -(previousEnd - currentStart); + + expect(getTimeFromPrevious(currentStart, currentEnd, previousStart, previousEnd, previousDuration)).toBe(expected); + }); + + it('fully overlapping events are the next day', () => { + const previousStart = 10 * MILLIS_PER_HOUR; + const previousEnd = 12 * MILLIS_PER_HOUR; + const previousDuration = previousEnd - previousStart; + const currentStart = 10 * MILLIS_PER_HOUR; + const currentEnd = 12 * MILLIS_PER_HOUR; + const expected = dayInMs - previousDuration; expect(getTimeFromPrevious(currentStart, currentEnd, previousStart, previousEnd, previousDuration)).toBe(expected); }); diff --git a/packages/utils/src/date-utils/getTimeFromPrevious.ts b/packages/utils/src/date-utils/getTimeFromPrevious.ts index 39b877615..e0dad100d 100644 --- a/packages/utils/src/date-utils/getTimeFromPrevious.ts +++ b/packages/utils/src/date-utils/getTimeFromPrevious.ts @@ -1,6 +1,5 @@ -import { checkIsNextDay } from './checkIsNextDay'; -import { checkOverlap } from './checkOverlap'; -import { dayInMs } from './conversionUtils'; +import { checkIsNextDay } from './checkIsNextDay.js'; +import { dayInMs } from './conversionUtils.js'; /** * Utility returns the time elapsed (gap or overlap) from the previous @@ -25,20 +24,22 @@ export function getTimeFromPrevious( // event is the day after if (checkIsNextDay(previousStart, currentStart, previousDuration)) { - // duration is difference between normalised start and previous end + // time from previous is difference between normalised start and previous end return currentStart + dayInMs - previousEnd; } // event has a gap from previous if (currentStart > previousEnd) { + // time from previous is difference between start and previous end return currentStart - previousEnd; } // event overlaps with previous - if (checkOverlap(previousStart, previousEnd, currentStart, currentEnd)) { - // duration is the amount of time the current event has over the previous - // this value must be capped at 0 - return Math.max(currentEnd - previousEnd, 0); + // TODO: account for midnight roll + const overlap = previousEnd - currentStart; + if (overlap > 0) { + // time is a negative number indicating the amount of overlap + return -overlap; } // we need to make sure we return a number, but there are no business cases for this