diff --git a/src/app/features/galaxy-system/routing-client.spec.ts b/src/app/features/galaxy-system/routing-client.spec.ts index 86fd2b3..03c2b0e 100644 --- a/src/app/features/galaxy-system/routing-client.spec.ts +++ b/src/app/features/galaxy-system/routing-client.spec.ts @@ -148,12 +148,38 @@ describe('RoutingClient with a worker', () => { const { client, worker } = clientWithFake(); const once = client.route(100, 104, 1.5, 8); const again = client.route(100, 104, 1.5, 8); + const widerRange = client.route(100, 104, 2.5, 8); expect(worker.requests).toHaveLength(1); worker.answer({ kind: 'route', requestId: worker.requests[0].requestId, route: null, neededRangePc: 4 }); expect(await again).toEqual(await once); - expect(worker.requests).toHaveLength(1); + await flush(); + // The same two stars at another range is another question. + expect(worker.requests.map((request) => request.rangePc)).toEqual([1.5, 2.5]); + worker.answer({ kind: 'route', requestId: worker.requests[1].requestId, route: null, neededRangePc: null }); + await expect(widerRange).resolves.toEqual({ route: null, neededRangePc: null }); + client.dispose(); + }); + + // Turning the layer off and on again while the worker is busy asks for the same graph twice. Were + // the second to replace the first, the first's rejection would wipe the scene's record of the second. + it('shares a graph already on its way for the same range and the same list of drawn stars', async () => { + const { client, worker } = clientWithFake(); + const drawn = Uint32Array.of(0, 1, 2); + const building = client.links(3, drawn); + const waiting = client.links(5, drawn); + const again = client.links(5, drawn); + const sameAsBuilding = client.links(3, drawn); + + worker.answer({ kind: 'links', requestId: worker.requests[0].requestId, segments: new Float32Array(6) }); + await expect(building).resolves.toHaveLength(6); + await expect(sameAsBuilding).resolves.toHaveLength(6); + await flush(); + worker.answer({ kind: 'links', requestId: worker.requests[1].requestId, segments: new Float32Array(12) }); + await expect(waiting).resolves.toHaveLength(12); + await expect(again).resolves.toHaveLength(12); + expect(worker.requests.map((request) => request.kind === 'links' && request.rangePc)).toEqual([3, 5]); client.dispose(); }); diff --git a/src/app/features/galaxy-system/routing-client.ts b/src/app/features/galaxy-system/routing-client.ts index 9ffb096..f165898 100644 --- a/src/app/features/galaxy-system/routing-client.ts +++ b/src/app/features/galaxy-system/routing-client.ts @@ -18,12 +18,6 @@ export class SupersededRequest extends Error { /** A request made and not yet answered: what was asked, and the promise whoever asked is holding. */ interface Outstanding { readonly request: RoutingRequest; - /** - * A route question without its id, so the same route asked for twice can be recognised. Empty for - * a graph: the scene never asks for the same graph twice, and spelling out 70 000 drawn stars to - * compare costs more than the comparison could save. - */ - readonly question: string; readonly promise: Promise; readonly resolve: (response: RoutingResponse) => void; readonly reject: (error: Error) => void; @@ -36,9 +30,19 @@ function outstanding(request: RoutingRequest): Outstanding { resolve = onResolve; reject = onReject; }); - // eslint-disable-next-line @typescript-eslint/no-unused-vars - const { requestId, ...question } = request; - return { request, question: request.kind === 'route' ? JSON.stringify(question) : '', promise, resolve, reject }; + return { request, promise, resolve, reject }; +} + +/** + * Whether two requests ask the same question. A graph is the same when it is for the same range and + * the very same list of drawn stars: the star field replaces that list whenever the set changes, so + * one array is one set, and comparing 70 000 indices would cost more than sharing could save. + */ +function asksTheSame(a: RoutingRequest, b: RoutingRequest): boolean { + if (a.kind === 'links' || b.kind === 'links') { + return a.kind === 'links' && b.kind === 'links' && a.rangePc === b.rangePc && a.drawn === b.drawn; + } + return a.fromId === b.fromId && a.toId === b.toId && a.rangePc === b.rangePc && a.ceilingPc === b.ceilingPc; } /** The routing worker, where this environment has one. */ @@ -50,12 +54,13 @@ function startRoutingWorker(): Worker | undefined { * Asks the route questions of a worker holding its own copy of the catalogue, and hands back * promises. * - * The worker answers one request at a time and cannot drop one it has started: a jump-link graph - * at 8 pc is seconds of work. So requests are held here and sent one by one, and while one is out, - * only the latest of each kind waits behind it — a newer graph replaces an older one before it is - * ever built, and the older promise is rejected with {@link SupersededRequest}. Routes go ahead of - * graphs, being quick and asked for by a click. The same route asked for again while it is still - * outstanding shares the answer rather than being worked out twice. + * The worker answers one request at a time and cannot drop one it has started: a route with no path + * can be seconds of work, and a graph of the drawn stars at 8 pc a few hundred milliseconds. So + * requests are held here and sent one by one, and while one is out, only the latest of each kind + * waits behind it — a newer graph replaces an older one before it is ever built, and the older + * promise is rejected with {@link SupersededRequest}. Routes go ahead of graphs, being quick to ask + * for and asked for by a click. The same question asked again while it is still outstanding shares + * the answer rather than being worked out twice; see `asksTheSame`. * * Where there is no worker — the unit tests' DOM has none, and a worker can fail to load or crash — * the same answers are worked out in place, from the index the scene already holds. @@ -115,11 +120,13 @@ export class RoutingClient { if (!this.worker) { return new Promise((resolve) => resolve(answerRouting(this.localIndex, request))); } - const asked = outstanding(request); - const same = asked.question ? [this.inFlight, this.waiting[request.kind]].find((other) => other?.question === asked.question) : undefined; + // Shared rather than replaced: an identical request superseding the one it repeats would reject it, + // and whoever holds that promise would take the rejection for its own question. + const same = [this.inFlight, this.waiting[request.kind]].find((other) => other !== undefined && asksTheSame(other.request, request)); if (same) { return same.promise; } + const asked = outstanding(request); this.waiting[request.kind]?.reject(new SupersededRequest()); this.waiting[request.kind] = asked; this.sendNext();