[match] put players with room roles in same private instance when unpublished

apps/match/src/match.app.ts

@@ -30,6 +30,7 @@ import {
 	getRoomInstancesByRoom,
 	getRoomInstanceSummariesByRoom,
 	getRoomInvite,
+	getSharedPrivateInstance,
 	getStoredRoomInstance,
 	hasRoomInviteTo,
 	InviteMode,
@@ -1486,10 +1487,16 @@ async function resolveRoomInstance(
 	// it never admitted — a friend follows the owner in, and `IsPrivate: false` tells every
 	// client the session is open. The owner walking through their own room's subroom door
 	// posts `JoinMode` 0, so this has to be decided here rather than trusted from the body.
-	// Private means a fresh instance each time, as `JoinMode` 2 always has: the room's
+	// Room routes only, like the gate — an event or a clubhouse held in a private room is a
-	// people reach each other by invite. Room routes only, like the gate — an event or a
+	// session its own guest list shares.
-	// clubhouse held in a private room is a session its own guest list shares.
+	//
-	const privateInstance = isPrivate || (gateOnAccessibility && !isPublished(room))
+	// It is also ONE session, not a fresh one per matchmake: an unpublished room has a
+	// single private instance (per subroom and build), and everyone the gate admits is
+	// placed in it — that is what "come look at my room" means to a co-owner. Spawning a
+	// fresh instance each time, as `JoinMode` 2 does on a published room, put the creator
+	// and each person they let in into separate empty copies of the room.
+	const unpublished = gateOnAccessibility && !isPublished(room)
+	const privateInstance = isPrivate || unpublished
 
 	// The build this player is on, from their token. A 2023 client can't load a scene
 	// saved at a newer persistence version, so it is refused the room outright (see
@@ -1510,21 +1517,32 @@ async function resolveRoomInstance(
 	// your current instance (e.g. the only public instance of a room you're already in)
 	// returns the same id and hangs the client mid-join. Exclude it from the join
 	// search, which pushes them to another live instance if one exists or forces a
-	// fresh one below. (Only the public path reuses instances, so only it needs the
+	// fresh one below. (Only the paths that reuse instances — public, and the shared
-	// read; a private matchmake always gets a fresh instance.)
+	// session of an unpublished room — need the read; a private matchmake into a
-	const currentInstanceId = privateInstance
+	// published room always gets a fresh instance.)
-		? undefined
+	const reusesInstance = unpublished || !privateInstance
-		: (await getPresence<RoomInstance>(c.env.DB, ownerId))?.roomInstance?.roomInstanceId
+	const currentInstanceId = reusesInstance
+		? (await getPresence<RoomInstance>(c.env.DB, ownerId))?.roomInstance?.roomInstanceId
+		: undefined
 	// The same build, with GAME_VERSION standing in for a token that names none. It scopes
 	// the search below and is stamped on the instance when one is created, which is what
 	// keeps a session to a single client version.
 	const gameVersion = tokenVersion ?? GAME_VERSION
-	// Reuse an existing joinable public instance *of the same subroom and the same
+	// Reuse an existing joinable instance *of the same subroom and the same build* —
-	// build* — subrooms are separate places, so joining one must never land you in
+	// subrooms are separate places, so joining one must never land you in another, and
-	// another, and neither must a session running a different version of the room.
+	// neither must a session running a different version of the room. A published room
-	// Private matchmakes always get a fresh instance. Create one when there's nothing
+	// reuses a public instance; an unpublished one reuses its single private session.
-	// to join.
+	// A private matchmake into a published room always gets a fresh instance. Create
-	let instance = privateInstance
+	// one when there's nothing to join.
+	let instance = unpublished
+		? await getSharedPrivateInstance(
+				c.env.DB,
+				f.roomId,
+				gameVersion,
+				f.subRoomId,
+				currentInstanceId
+			)
+		: privateInstance
 			? null
 			: await getJoinableInstance(c.env.DB, f.roomId, gameVersion, f.subRoomId, currentInstanceId)
 	if (!instance) {
@@ -1566,7 +1584,9 @@ async function resolveRoomInstance(
  * {@link canEnterRoom}): a room that isn't published answers `RoomIsPrivate` (25) to anyone
  * but its creator, its role holders and its invitees, in either join mode — a private
  * instance of an unpublished room is still a way into it. Those it does admit always get
- * a PRIVATE instance of it, whatever `JoinMode` they posted.
+ * a PRIVATE instance of it, whatever `JoinMode` they posted — and the SAME one: an
+ * unpublished room has a single private instance per subroom and build, shared by
+ * everyone it admits (see `getSharedPrivateInstance`).
  *
  * `subRoomId` is optional: absent, `resolveRoomInstance` falls back to the room's first
  * subroom (its default entrance).

apps/match/src/test/integration/api.test.ts

@@ -1183,9 +1183,10 @@ describe('public endpoints', () => {
 				['8809', '20230414.01'],
 				['8810', '20221117'],
 			]) {
-				expect((await matchmake('/matchmake/room/2', player, version)).roomInstance).toMatchObject(
+				expect((await matchmake('/matchmake/room/2', player, version)).roomInstance).toMatchObject({
-					{ roomId: 2, name: '^RecCenter' }
+					roomId: 2,
-				)
+					name: '^RecCenter',
+				})
 			}
 			expect((await matchmake('/matchmake/room/2', '8811', '')).roomInstance).toMatchObject({
 				roomId: 2,
@@ -1279,9 +1280,16 @@ describe('auth-gated endpoints', () => {
 		expect(b.roomInstance.photonRoomId).toBe(a.roomInstance.photonRoomId)
 		expect(b.roomInstance.roomInstanceId).toBe(a.roomInstance.roomInstanceId)
 
-		// A private matchmake (JoinMode 2) gets its own distinct instance.
+		// A private matchmake (JoinMode 2) gets its own distinct instance — and so does the
+		// next one: a PUBLISHED room may have any number of private instances. Only an
+		// unpublished room collapses them into one shared session (see the accessibility
+		// tests below).
 		const priv = await matchmake('902', '2')
 		expect(priv.roomInstance.photonRoomId).not.toBe(a.roomInstance.photonRoomId)
+		const priv2 = await matchmake('903', '2')
+		expect(priv2.roomInstance.photonRoomId).not.toBe(a.roomInstance.photonRoomId)
+		expect(priv2.roomInstance.photonRoomId).not.toBe(priv.roomInstance.photonRoomId)
+		expect(priv2.roomInstance.roomInstanceId).not.toBe(priv.roomInstance.roomInstanceId)
 	})
 
 	test('POST /matchmake/room/:roomId only pools players on the same client build', async () => {
@@ -3049,7 +3057,11 @@ describe('auth-gated endpoints', () => {
 		expect((await v2('/matchmake/v2/room/2', '9880', 2)).RoomInstance?.RoomId).toBe(2)
 	})
 
-	test('an unpublished room’s instances are always PRIVATE, whatever JoinMode asked for', async () => {
+	test('an unpublished room has ONE private instance, shared by everyone it admits', async () => {
+		// Start clean: an earlier case may have left room 6 sessions and presence behind,
+		// and which instance a matchmake lands in is the whole point here.
+		await env.DB.prepare('DELETE FROM room_instance WHERE room_id = 6').run()
+		await env.DB.prepare('DELETE FROM presence WHERE account_id IN (42, 44, 9880, 9885)').run()
 		type V2 = {
 			ErrorCode: number
 			RoomInstance: { RoomInstanceId: number; RoomId: number; IsPrivate: boolean } | null
@@ -3076,7 +3088,10 @@ describe('auth-gated endpoints', () => {
 		expect(own.ErrorCode).toBe(0)
 		expect(own.RoomInstance).toMatchObject({ RoomId: 6, IsPrivate: true })
 		expect(await stored(own.RoomInstance?.RoomInstanceId)).toBe(1)
-		// The 2023 spelling, and JoinMode 1, are the same.
+		// The 2023 spelling, and JoinMode 1, are the same. The creator is already standing
+		// in the session above, and a matchmake never hands back the instance the caller is
+		// in (the client hangs on an unchanged id), so this one is a FRESH private session —
+		// and the first now sits empty, within the sweep's grace window.
 		const v1 = (await (
 			await exports.default.fetch(`${ORIGIN}/matchmake/room/6`, {
 				method: 'POST',
@@ -3086,18 +3101,56 @@ describe('auth-gated endpoints', () => {
 		).json()) as { roomInstance: { roomInstanceId: number; isPrivate: boolean } | null }
 		expect(v1.roomInstance?.isPrivate).toBe(true)
 		expect(await stored(v1.roomInstance?.roomInstanceId)).toBe(1)
+		expect(v1.roomInstance?.roomInstanceId).not.toBe(own.RoomInstance?.RoomInstanceId)
 
-		// Nothing public to reuse: a role holder asking for a public session gets a private
+		// A role holder joins the creator's session: an unpublished room has ONE private
-		// one of their own rather than joining the creator's.
+		// instance its people share, so a Host asking for a public session is placed WITH
+		// the creator — in the instance they're standing in, not the empty one they left,
+		// and never in a private instance of their own. Nothing public exists to reuse.
 		const host = await v2('/matchmake/v2/room/6', '44', 0)
 		expect(host.RoomInstance?.IsPrivate).toBe(true)
-		expect(host.RoomInstance?.RoomInstanceId).not.toBe(v1.roomInstance?.roomInstanceId)
+		expect(host.RoomInstance?.RoomInstanceId).toBe(v1.roomInstance?.roomInstanceId)
 		expect(
 			await env.DB.prepare(
 				'SELECT COUNT(*) AS n FROM room_instance WHERE room_id = 6 AND is_private = 0'
 			).first<{ n: number }>()
 		).toEqual({ n: 0 })
 
+		// JoinMode 2 does not fork an unpublished room either: an invitee asking for a
+		// private session is put in the same one, in both spellings of the route.
+		await env.DB.prepare(
+			'INSERT INTO room_invite (from_player_id, to_player_id, room_id, created_at) VALUES (42, 9880, 6, ?1)'
+		)
+			.bind(Math.floor(Date.now() / 1000))
+			.run()
+		try {
+			const invitee = await v2('/matchmake/v2/room/6/6', '9880', 2)
+			expect(invitee.RoomInstance?.RoomInstanceId).toBe(v1.roomInstance?.roomInstanceId)
+			// Standing in it now, so the 2023 spelling moves them to the only other
+			// session — the one the creator abandoned — rather than back into the same id.
+			const again = (await (
+				await exports.default.fetch(`${ORIGIN}/matchmake/room/6`, {
+					method: 'POST',
+					headers: {
+						...(await bearer('9880')),
+						'Content-Type': 'application/x-www-form-urlencoded',
+					},
+					body: 'JoinMode=2',
+				})
+			).json()) as { roomInstance: { roomInstanceId: number; isPrivate: boolean } | null }
+			expect(again.roomInstance?.isPrivate).toBe(true)
+			expect(again.roomInstance?.roomInstanceId).toBe(own.RoomInstance?.RoomInstanceId)
+			// Still exactly the two sessions: nothing was minted for anyone it admitted.
+			expect(
+				await env.DB.prepare('SELECT COUNT(*) AS n FROM room_instance WHERE room_id = 6').first<{
+					n: number
+				}>()
+			).toEqual({ n: 2 })
+		} finally {
+			await env.DB.prepare('DELETE FROM room_invite WHERE to_player_id = 9880').run()
+			await env.DB.prepare('DELETE FROM presence WHERE account_id = 9880').run()
+		}
+
 		// A FRIEND of the creator is not one of the room's people: neither follow gets them
 		// in — the 2023 route by the room's gate, the v2 route because the session is private.
 		await env.DB.prepare(
@@ -3125,10 +3178,12 @@ describe('auth-gated endpoints', () => {
 			).toBeNull()
 
 			// One of the room's own people who is also a friend may follow: the room admits
-			// them itself. Here the creator follows the Host into the Host's session.
+			// them itself. Here the creator steps out to a public room, then follows the Host
+			// back into the shared session.
 			await env.DB.prepare(
 				'INSERT INTO relationship (requester_id, target_id, relationship_type) VALUES (42, 44, 3)'
 			).run()
+			expect((await v2('/matchmake/v2/room/2', '42', 0)).ErrorCode).toBe(0)
 			const ownFollow = (await (
 				await exports.default.fetch(`${ORIGIN}/matchmake/player/44`, {
 					method: 'POST',

packages/domain/src/room-instance-db.ts

@@ -432,6 +432,58 @@ export async function getJoinableInstance(
 	return row ? toDto(parse(row.data)) : null
 }
 
+/**
+ * The ONE private session of an unpublished room — the instance of `subRoomId` running
+ * `gameVersion` that the room's people share — or null when nobody has opened one. The
+ * complement of {@link getJoinableInstance}: that search only ever reuses PUBLIC instances,
+ * and an unpublished room has none (every instance of it is private, see the match
+ * worker's `resolveRoomInstance`), so without this each of the room's people who
+ * matchmaked in got a private instance of their own and never found each other — a
+ * co-owner answering the creator's "come look at this" landed in an empty copy of the
+ * room beside them.
+ *
+ * Scoped like the public search: by subroom (separate places) and by build (a session
+ * belongs to one client version; another build's session reads as nothing to join and the
+ * caller opens one beside it). Full instances are skipped — there is nothing to put a
+ * player into — and `excludeInstanceId` drops the one the caller is already standing in,
+ * because the client keys its room transition off a CHANGING instance id and hangs when
+ * handed the same one back.
+ *
+ * That exclusion is also why the search prefers the instance with the most live players
+ * in it over the oldest: a creator re-entering the subroom they're in is moved to a fresh
+ * instance, and the one they left sits empty for the sweep's grace window. A co-owner
+ * arriving in the meantime must land with the creator, not in the abandoned shell.
+ */
+export async function getSharedPrivateInstance(
+	db: D1Database,
+	roomId: number,
+	gameVersion: string,
+	subRoomId: number,
+	excludeInstanceId?: number,
+	now = Math.floor(Date.now() / 1000)
+): Promise<RoomInstanceDto | null> {
+	const binds: Array<number | string> = [roomId, gameVersion, subRoomId, now]
+	let exclude = ''
+	if (excludeInstanceId !== undefined) {
+		binds.push(excludeInstanceId)
+		exclude = `AND id != ?${binds.length}`
+	}
+	const row = await db
+		.prepare(
+			`SELECT data FROM room_instance
+			 WHERE room_id = ?1 AND game_version = ?2 AND sub_room_id = ?3
+			   AND is_private = 1 AND is_full = 0 ${exclude}
+			 ORDER BY (
+			   SELECT COUNT(*) FROM presence
+			    WHERE presence.room_instance_id = room_instance.id AND presence.expires_at > ?4
+			 ) DESC, id
+			 LIMIT 1`
+		)
+		.bind(...binds)
+		.first<{ data: string }>()
+	return row ? toDto(parse(row.data)) : null
+}
+
 /**
  * All instances of a given room — every build's, unless `gameVersion` scopes it to the
  * sessions running one. An instance belongs to a single client build (see