Feature/sv demo cinema - mem leak fix + playdemo wait for clients

code/qcommon/vm.c

@@ -1940,7 +1940,7 @@ intptr_t QDECL VM_Call( vm_t *vm, int nargs, int callnum, ... )
 	int i;
 
 	if ( !vm ) {
-		Com_Error( ERR_FATAL, "VM_Call with NULL vm" );
+		Com_Error( ERR_FATAL, "VM_Call with NULL vm (callnum %d)", callnum );
 	}
 
 #ifdef DEBUG

code/server/server.h

@@ -101,6 +101,7 @@ typedef struct {
 	int				demoClientNum;				// recorded player's clientNum from demo header
 	int				demoChecksumFeed;			// checksumFeed from demo header
 	qboolean		demoEnded;					// EOF reached, holding on last frame
+	qboolean		demoWaitingForViewer;		// hold on first frame until a client is CS_ACTIVE
 	int				demoMessageSequence;		// sequence counter from the demo stream
 	playerState_t	demoPS;						// current decoded playerState
 	entityState_t	demoPrevPS_base;			// previous playerState for delta decoding

code/server/sv_demo_play.cpp

@@ -539,9 +539,11 @@ void SV_SpawnDemoServer( const char *demoName )
 	CL_ShutdownAll();
 #endif
 
-	// Do NOT clear the hunk — we have no BSP, so no new hunk allocation
-	// is needed.  Just clear the per-level server_t and reset snapshot
-	// storage.
+	// We still hunk-allocate snapshot entity storage below on every call
+	// (same as SV_SpawnServer), so the hunk must be cleared first or it
+	// fills up and Hunk_Alloc starts failing after a handful of demos.
+	Hunk_Clear();
+	CM_ClearMap();
 
 	// Init/resize client slots if needed.
 	if ( !Cvar_VariableIntegerValue( "sv_running" ) ) {
@@ -638,20 +640,36 @@ void SV_SpawnDemoServer( const char *demoName )
 	// Publish the mapname so clients see it in the serverinfo.
 	const char *mapname = Info_ValueForKey( sv.configstrings[CS_SERVERINFO], "mapname" );
 	if ( mapname && *mapname ) {
+		// Cinema skips CM_LoadMap (no collision data needed for snapshot replay), so
+		// unlike a normal map spawn this never gets a missing-BSP check for free.
+		// Check for existence only, before committing to playback, so a missing map
+		// aborts cleanly here instead of every connecting client discovering it and
+		// disconnecting individually.
+		fileHandle_t mapFile;
+		if ( FS_FOpenFileRead( va( "maps/%s.bsp", mapname ), &mapFile, qtrue ) < 0 ) {
+			Com_Printf( S_COLOR_RED "SV_SpawnDemoServer: map '%s' referenced by this demo "
+				"was not found (maps/%s.bsp) — clients would be unable to load it and "
+				"disconnect\n", mapname, mapname );
+			SV_Shutdown( "demo cinema init failed" );
+			return;
+		}
+		FS_FCloseFile( mapFile );
 		Cvar_Set( "mapname", mapname );
 	}
 	sv_mapname = Cvar_Get( "mapname", "nomap", CVAR_SERVERINFO | CVAR_ROM );
 
 	// Mark demo playback active.
-	sv.demoPlayback = qtrue;
-	sv.demoEnded    = qfalse;
-	sv.demoNumEnts  = 0;
-	sv.demoPS       = {};
-	sv.demoPrevPS   = {};
+	sv.demoPlayback         = qtrue;
+	sv.demoEnded            = qfalse;
+	sv.demoWaitingForViewer = qtrue;
+	sv.demoNumEnts          = 0;
+	sv.demoPS               = {};
+	sv.demoPrevPS           = {};
 
 	sv.state = SS_GAME;
 
 	Cvar_Set( "sv_running", "1" );
+	Cvar_Set( "sv_playingDemo", "1" );
 
 	// Read the very first snapshot so sv.time is valid before any client connects.
 	SV_DemoAdvance();

code/server/sv_game.cpp

@@ -1139,7 +1139,7 @@ See if the current console command is claimed by the game
 ====================
 */
 qboolean SV_GameCommand( void ) {
-	if ( sv.state != SS_GAME ) {
+	if ( sv.state != SS_GAME || !gvm ) {
 		return qfalse;
 	}
 

code/server/sv_init.cpp

@@ -674,6 +674,9 @@ void SV_SpawnServer( const char *mapname, qboolean killBots ) {
 	SV_SetConfigstring( CS_SERVERINFO, Cvar_InfoString( CVAR_SERVERINFO, nullptr ) );
 	cvar_modifiedFlags &= ~CVAR_SERVERINFO;
 
+	// this is a normal map spawn, not demo cinema playback
+	Cvar_Set( "sv_playingDemo", "0" );
+
 	// any media configstring setting now should issue a warning
 	// and any configstring changes should be reliably transmitted
 	// to all clients
@@ -717,6 +720,9 @@ void SV_Init( void )
 	//Cvar_Get ("protocol", va("%i", PROTOCOL_VERSION), CVAR_SERVERINFO | CVAR_ROM);
 	sv_mapname = Cvar_Get ("mapname", "nomap", CVAR_SERVERINFO | CVAR_ROM);
 	Cvar_SetDescription( sv_mapname, "Display the name of the current map being used on a server." );
+	Cvar_SetDescription(
+		Cvar_Get( "sv_playingDemo", "0", CVAR_SERVERINFO | CVAR_ROM ),
+		"Read-only: 1 while the server is replaying a demo (sv_playdemo cinema), 0 otherwise." );
 	sv_privateClients = Cvar_Get( "sv_privateClients", "0", CVAR_SERVERINFO );
 	Cvar_CheckRange( sv_privateClients, "0", va( "%i", MAX_CLIENTS-1 ), CV_INTEGER );
 	Cvar_SetDescription( sv_privateClients, "The number of spots, out of sv_maxclients, reserved for players with the server password (sv_privatePassword)." );
@@ -926,6 +932,7 @@ void SV_Shutdown( const char *finalmsg ) {
 	sv.time = 0;
 
 	Cvar_Set( "sv_running", "0" );
+	Cvar_Set( "sv_playingDemo", "0" );
 
 #ifndef DEDICATED
 	Cvar_Set( "ui_singlePlayerActive", "0" );

code/server/sv_main.cpp

@@ -814,6 +814,7 @@ static void SVC_Info( const netadr_t *from ) {
 	Info_SetValueForKey( infostring.data(), "g_humanplayers", va( "%i", humans ) );
 	Info_SetValueForKey( infostring.data(), "sv_maxclients", va( "%i", sv.maxclients - sv_privateClients->integer ) );
 	Info_SetValueForKey( infostring.data(), "gametype", va( "%i", sv_gametype->integer ) );
+	Info_SetValueForKey( infostring.data(), "playingDemo", va( "%i", sv.demoPlayback ) );
 	Info_SetValueForKey( infostring.data(), "pure", va( "%i", sv.pure ) );
 	Info_SetValueForKey( infostring.data(), "g_needpass", va( "%d", Cvar_VariableIntegerValue( "g_needpass" ) ) );
 	gamedir = Cvar_VariableString( "fs_game" );
@@ -1415,11 +1416,24 @@ void SV_Frame( int msec, int realMsec ) {
 		sv.timeResidual -= frameMsec;
 
 		if ( sv.demoPlayback ) {
+			// Before playback starts, hold on the first frame until a real
+			// player is fully in-game so they see the demo from the beginning.
+			if ( sv.demoWaitingForViewer ) {
+				for ( const client_t &client : SV_Clients() ) {
+					if ( client.state == CS_ACTIVE &&
+						 client.netchan.remoteAddress.type != NA_BOT ) {
+						sv.demoWaitingForViewer = qfalse;
+						break;
+					}
+				}
+			}
+
 			// Advance demo by one snapshot per frame tick.
 			// SV_DemoAdvance updates sv.time from the demo's serverTime.
-			// If the demo has ended we still advance sv.time to keep
-			// client connections alive (hold on last frame).
-			if ( sv.demoEnded ) {
+			// While waiting for a viewer, or once the demo has ended, we
+			// still advance sv.time to keep client connections alive
+			// (hold on frame 0 / hold on last frame respectively).
+			if ( sv.demoWaitingForViewer || sv.demoEnded ) {
 				sv.time += frameMsec;
 			} else {
 				SV_DemoAdvance();

docs/fnquake3/CHANGELOG.md

@@ -25,7 +25,8 @@ Keep short user-facing bullets under `Unreleased` as changes land. During releas
 
 ### Fixes
 - Windowed mode no longer confines the mouse cursor to the window while a menu or the console is open, so the pointer can move to the desktop or another monitor and back seamlessly. The cursor stays locked during actual gameplay. In menus and the console the OS cursor is hidden and the in-game cursor snaps 1:1 to the real pointer position within the window.
-- Fixed the expanded vanilla HUD so long pickup and mini-score strings stay together, and the attacker head/name remains aligned to the right edge.
+- Fixed a dedicated-server crash (`VM_Call with NULL vm`) triggered by typing any unrecognized console command while `sv_playdemo` demo cinema playback was active. Also added a new read-only `sv_playingDemo` cvar (visible locally and to remote `getinfo`/`getstatus` queries) so it's now possible to tell whether a server is currently replaying a demo.
+- `sv_playdemo` now checks that the demo's map is actually present on the server before starting cinema playback, instead of starting anyway and leaving every connecting client to discover the missing map on its own and disconnect.
 
 ### Documentation and Tooling
 - _None yet._

tests/demo_cinema_serverinfo_tests.py

@@ -0,0 +1,110 @@
+import unittest
+from pathlib import Path
+
+
+ROOT = Path(__file__).resolve().parents[1]
+
+
+def read_text(relative_path):
+    return (ROOT / relative_path).read_text(encoding="utf-8")
+
+
+class DemoCinemaServerinfoSourceTests(unittest.TestCase):
+    """Source contract for the read-only sv_playingDemo indicator.
+
+    sv.demoPlayback (server-side demo cinema state) is otherwise invisible to
+    the local console and to remote clients/server browsers. sv_playingDemo
+    mirrors it as a CVAR_SERVERINFO | CVAR_ROM cvar, following the same
+    pattern as the existing read-only `mapname` cvar.
+    """
+
+    def test_sv_playingDemo_registered_serverinfo_rom(self):
+        source = read_text("code/server/sv_init.cpp")
+        self.assertIn(
+            'Cvar_Get( "sv_playingDemo", "0", CVAR_SERVERINFO | CVAR_ROM )',
+            source,
+        )
+
+    def test_sv_playingDemo_set_entering_cinema(self):
+        source = read_text("code/server/sv_demo_play.cpp")
+        spawn_start = source.index("void SV_SpawnDemoServer(")
+        spawn_body = source[spawn_start:spawn_start + 6000]
+        demo_playback_true = spawn_body.index("sv.demoPlayback         = qtrue;")
+        cvar_set = spawn_body.index('Cvar_Set( "sv_playingDemo", "1" );', demo_playback_true)
+        self.assertGreater(
+            cvar_set, demo_playback_true,
+            "sv_playingDemo must be set to 1 after sv.demoPlayback is marked active",
+        )
+
+    def test_sv_playingDemo_reset_on_normal_spawn(self):
+        source = read_text("code/server/sv_init.cpp")
+        spawn_start = source.index("void SV_SpawnServer(")
+        reset_call = source.index('Cvar_Set( "sv_playingDemo", "0" );', spawn_start)
+        game_state = source.index("sv.state = SS_GAME;", spawn_start)
+        self.assertLess(
+            reset_call, game_state,
+            "a normal map spawn must clear sv_playingDemo before entering SS_GAME",
+        )
+
+    def test_sv_playingDemo_reset_on_shutdown(self):
+        source = read_text("code/server/sv_init.cpp")
+        shutdown_start = source.index("void SV_Shutdown(")
+        shutdown_body = source[shutdown_start:shutdown_start + 3000]
+        running_reset = shutdown_body.index('Cvar_Set( "sv_running", "0" );')
+        demo_reset = shutdown_body.index('Cvar_Set( "sv_playingDemo", "0" );', running_reset)
+        self.assertGreater(demo_reset, running_reset)
+
+    def test_getinfo_reports_playingDemo(self):
+        source = read_text("code/server/sv_main.cpp")
+        gametype_key = source.index('Info_SetValueForKey( infostring.data(), "gametype",')
+        playing_demo_key = source.index(
+            'Info_SetValueForKey( infostring.data(), "playingDemo", va( "%i", sv.demoPlayback ) );',
+            gametype_key,
+        )
+        self.assertGreater(playing_demo_key, gametype_key)
+
+
+class DemoCinemaMissingMapSourceTests(unittest.TestCase):
+    """Source contract: sv_playdemo must refuse to start cinema for a demo whose
+    map is not present on this server, instead of letting every connecting
+    client discover the missing BSP on its own and disconnect.
+    """
+
+    def test_map_existence_checked_before_demo_playback_flag(self):
+        source = read_text("code/server/sv_demo_play.cpp")
+        spawn_start = source.index("void SV_SpawnDemoServer(")
+        spawn_body = source[spawn_start:spawn_start + 6000]
+
+        probe = spawn_body.index('FS_FOpenFileRead( va( "maps/%s.bsp", mapname )')
+        demo_playback_true = spawn_body.index("sv.demoPlayback         = qtrue;")
+        self.assertLess(
+            probe, demo_playback_true,
+            "the map existence probe must run before sv.demoPlayback is committed",
+        )
+
+    def test_missing_map_aborts_cleanly(self):
+        source = read_text("code/server/sv_demo_play.cpp")
+        spawn_start = source.index("void SV_SpawnDemoServer(")
+        spawn_body = source[spawn_start:spawn_start + 6000]
+
+        probe = spawn_body.index('FS_FOpenFileRead( va( "maps/%s.bsp", mapname )')
+        # the failure branch immediately following the probe must abort via
+        # SV_Shutdown + return, matching the sibling validation failures already
+        # established in this function (corrupt/truncated demo, bad gamestate, ...)
+        failure_branch = spawn_body[probe:probe + 500]
+        self.assertIn('SV_Shutdown( "demo cinema init failed" )', failure_branch)
+        self.assertIn("return;", failure_branch)
+
+    def test_successful_probe_closes_the_handle(self):
+        source = read_text("code/server/sv_demo_play.cpp")
+        spawn_start = source.index("void SV_SpawnDemoServer(")
+        spawn_body = source[spawn_start:spawn_start + 6000]
+
+        probe = spawn_body.index('FS_FOpenFileRead( va( "maps/%s.bsp", mapname )')
+        demo_playback_true = spawn_body.index("sv.demoPlayback         = qtrue;")
+        between = spawn_body[probe:demo_playback_true]
+        self.assertIn("FS_FCloseFile( mapFile );", between)
+
+
+if __name__ == "__main__":
+    unittest.main()