From: bones_was_here <bones_was_here@xonotic.au>
Date: Thu, 5 Sep 2024 20:05:07 +0000 (+1000)
Subject: host: make some startup errors non-fatal, related polishing and cleanup
X-Git-Url: https://git.rm.cloudns.org/?a=commitdiff_plain;h=774e941cb1ddbbe929f8721629bf57a681bc315b;p=xonotic%2Fdarkplaces.git

host: make some startup errors non-fatal, related polishing and cleanup

It's annoying to abort DP just because of a bad menu progs or demo,
especially on Windows where accessing stdout is difficult so console
access is more important.

Shows a correct fallback message when the menu progs crashes (it's not
because of missing files).
Makes the missing files message more concise.

Performs a full cleanup after any QC crash, most importantly
progs->loaded is now false until that VM is restarted successfully.

Prevents menu failure interfering with more important VMs.

Removes the weird "early frame abort" jump point which is redundant now
because Host_Error, now the only code that ever aborts a frame, calls
Sys_Error instead of Host_AbortCurrentFrame during init.

Changes the framecount of the first frame from 0 to 1 so that when it's
>0 that means frame abort is now possible and safe.

Removes an old FIXME that was addressed.

Adds names for the host states.

Signed-off-by: bones_was_here <bones_was_here@xonotic.au>
---

diff --git a/host.c b/host.c
index b92f11b3..f004bb19 100644
--- a/host.c
+++ b/host.c
@@ -105,8 +105,8 @@ void Host_Error (const char *error, ...)
 
 	// LadyHavoc: if crashing very early, or currently shutting down, do
 	// Sys_Error instead
-	if (host.framecount < 3 || host.state == host_shutdown)
-		Sys_Error ("Host_Error during %s: %s", host.framecount < 3 ? "startup" : "shutdown", hosterrorstring1);
+	if (host.framecount < 1 || host.state != host_active)
+		Sys_Error ("Host_Error during %s: %s", host_state_str[host.state], hosterrorstring1);
 
 	if (hosterror)
 		Sys_Error ("Host_Error: recursively entered (original error was: %s    new error is: %s)", hosterrorstring2, hosterrorstring1);
@@ -137,6 +137,7 @@ void Host_Error (const char *error, ...)
 	// restore configured outfd
 	sys.outfd = outfd;
 
+	// can't abort a frame if we didn't start one yet, won't get here in that case (see above)
 	Host_AbortCurrentFrame();
 }
 
@@ -392,9 +393,6 @@ void Host_Init (void)
 	host.realtime = 0;
 	host.dirtytime = Sys_DirtyTime();
 
-	if (setjmp(host.abortframe)) // Huh?!
-		Sys_Error("Engine initialization failed. Check the console (if available) for additional information.\n");
-
 	if (Sys_CheckParm("-profilegameonly"))
 		Sys_AllowProfiling(false);
 
@@ -491,16 +489,11 @@ void Host_Init (void)
 	// NOTE: menu commands are freed by Cmd_RestoreInitState
 	Cmd_SaveInitState();
 
-	// FIXME: put this into some neat design, but the menu should be allowed to crash
-	// without crashing the whole game, so this should just be a short-time solution
-
 	// here comes the not so critical stuff
 
 	Host_AddConfigText(cmd_local);
 	Cbuf_Execute(cmd_local->cbuf); // cannot be in Host_AddConfigText as that would cause Host_LoadConfig_f to loop!
 
-	host.state = host_active;
-
 	CL_StartVideo();
 
 	Log_Start();
@@ -562,6 +555,7 @@ void Host_Init (void)
 	}
 
 	Con_DPrint("========Initialized=========\n");
+	host.state = host_active;
 
 	if (cls.state != ca_dedicated)
 		SV_StartThread();
@@ -625,6 +619,8 @@ double Host_Frame(double time)
 {
 	double cl_wait, sv_wait;
 
+	++host.framecount;
+
 	TaskQueue_Frame(false);
 
 	// keep the random time dependent, but not when playing demos/benchmarking
diff --git a/host.h b/host.h
index def3d0a2..b8dfcf3b 100644
--- a/host.h
+++ b/host.h
@@ -23,22 +23,31 @@ typedef enum host_state_e
 	host_init,
 	host_loading,
 	host_active,
-	// states >= host_shutdown cause graceful shutdown, see Sys_HandleCrash() comments
+	/// states >= host_shutdown cause graceful shutdown, see Sys_HandleCrash() comments
 	host_shutdown,
-	host_failing, ///< crashing
-	host_failed ///< crashed or aborted, SDL dialog open
+	host_failing,  ///< crashing (inside crash handler)
+	host_failed    ///< crashed or aborted, SDL dialog open
 } host_state_t;
+static const char * const host_state_str[] =
+{
+	[host_init]     = "init",
+	[host_loading]  = "loading",
+	[host_active]   = "normal operation",
+	[host_shutdown] = "shutdown",
+	[host_failing]  = "crashing",
+	[host_failed]   = "crashed",
+};
 
 typedef struct host_static_s
 {
 	jmp_buf abortframe;
 	int state;
-	unsigned int framecount; // incremented every frame, never reset (checked by Host_Error and Host_SaveConfig_f)
-	double realtime; // the accumulated mainloop time since application started (with filtering), without any slowmo or clamping
-	double dirtytime; // the main loop wall time for this frame, equal to Sys_DirtyTime() at the start of this host frame
-	double sleeptime; // time spent sleeping after the last frame
-	qbool restless; // don't sleep
-	qbool paused; // global paused state, pauses both client and server
+	unsigned int framecount; ///< incremented every frame, never reset, >0 means Host_AbortCurrentFrame() is possible
+	double realtime;         ///< the accumulated mainloop time since application started (with filtering), without any slowmo or clamping
+	double dirtytime;        ///< the main loop wall time for this frame, equal to Sys_DirtyTime() at the start of this host frame
+	double sleeptime;        ///< time spent sleeping after the last frame
+	qbool restless;          ///< don't sleep
+	qbool paused;            ///< global paused state, pauses both client and server
 	cmd_buf_t *cbuf;
 
 	struct
diff --git a/image.c b/image.c
index 15835797..6833996d 100644
--- a/image.c
+++ b/image.c
@@ -1176,7 +1176,7 @@ unsigned char *loadimagepixelsbgra (const char *filename, qbool complain, qbool
 
 	if (complain)
 	{
-		Con_Printf("Couldn't load %s using ", filename);
+		Con_Printf(CON_ERROR "Couldn't load %s using ", filename);
 		for (format = firstformat;format->formatstring;format++)
 		{
 			dpsnprintf (name, sizeof(name), format->formatstring, basename);
diff --git a/menu.c b/menu.c
index 51cc1277..6c935ef7 100644
--- a/menu.c
+++ b/menu.c
@@ -421,6 +421,7 @@ void M_Menu_Main_f(cmd_state_t *cmd)
 }
 
 
+static bool mp_failed;
 static void M_Main_Draw (void)
 {
 	int		f;
@@ -433,11 +434,20 @@ static void M_Main_Draw (void)
 		const char *s;
 		M_Background(640, 480); //fall back is always to 640x480, this makes it most readable at that.
 		y = 480/3-16;
-		s = "You have reached this menu due to missing or unlocatable content/data";M_PrintRed ((640-strlen(s)*8)*0.5, (480/3)-16, s);y+=8;
-		y+=8;
-		s = "You may consider adding";M_Print ((640-strlen(s)*8)*0.5, y, s);y+=8;
-		s = "-basedir /path/to/game";M_Print ((640-strlen(s)*8)*0.5, y, s);y+=8;
-		s = "to your launch commandline";M_Print ((640-strlen(s)*8)*0.5, y, s);y+=8;
+		if (mp_failed)
+		{
+			s = "The menu QC program has failed.";M_PrintRed ((640-strlen(s)*8)*0.5, y, s);y+=8;
+			y+=8;
+			s = "You should find the specific error(s) in the console.";M_Print ((640-strlen(s)*8)*0.5, y, s);y+=8;
+		}
+		else
+		{
+			s = "The required files were not found.";M_PrintRed ((640-strlen(s)*8)*0.5, y, s);y+=8;
+			y+=8;
+			s = "You may consider adding";M_Print ((640-strlen(s)*8)*0.5, y, s);y+=8;
+			s = "-basedir /path/to/game";M_Print ((640-strlen(s)*8)*0.5, y, s);y+=8;
+			s = "to your launch commandline.";M_Print ((640-strlen(s)*8)*0.5, y, s);y+=8;
+		}
 		M_Print (640/2 - 48, 480/2, "Open Console"); //The console usually better shows errors (failures)
 		M_Print (640/2 - 48, 480/2 + 8, "Quit");
 		M_DrawCharacter(640/2 - 56, 480/2 + (8 * m_main_cursor), 12+((int)(host.realtime*4)&1));
@@ -5214,8 +5224,9 @@ static int m_numrequiredglobals = sizeof(m_required_globals) / sizeof(m_required
 
 void MR_SetRouting (qbool forceold);
 
-void MVM_error_cmd(const char *format, ...) DP_FUNC_PRINTF(1);
-void MVM_error_cmd(const char *format, ...)
+jmp_buf mp_abort;
+static void MVM_error_cmd(const char *format, ...) DP_FUNC_PRINTF(1) DP_FUNC_NORETURN;
+static void MVM_error_cmd(const char *format, ...)
 {
 	static qbool processingError = false;
 	char errorstring[MAX_INPUTLINE];
@@ -5229,9 +5240,6 @@ void MVM_error_cmd(const char *format, ...)
 	dpvsnprintf (errorstring, sizeof(errorstring), format, argptr);
 	va_end (argptr);
 
-	if (host.framecount < 3)
-		Sys_Error("Menu_Error: %s", errorstring);
-
 	Con_Printf(CON_ERROR "Menu_Error: %s\n", errorstring);
 
 	if(!processingError)
@@ -5246,6 +5254,9 @@ void MVM_error_cmd(const char *format, ...)
 	Con_Print("Falling back to engine menu\n");
 	key_dest = key_game;
 	MR_SetRouting (true);
+	mp_failed = true;
+	if (cls.state != ca_connected || key_dest != key_game) // if not disrupting a game
+		MR_ToggleMenu(1); // ensure error screen appears, eg for: menu_progs ""; menu_restart
 
 	// reset the active scene, too (to be on the safe side ;))
 	R_SelectScene( RST_CLIENT );
@@ -5256,8 +5267,8 @@ void MVM_error_cmd(const char *format, ...)
 	// restore configured outfd
 	sys.outfd = outfd;
 
-	// Let video start at least
-	Host_AbortCurrentFrame();
+	// no frame abort: menu failure shouldn't interfere with more important VMs
+	longjmp(mp_abort, 1);
 }
 
 static void MVM_begin_increase_edicts(prvm_prog_t *prog)
@@ -5304,6 +5315,9 @@ static void MP_KeyEvent (int key, int ascii, qbool downevent)
 {
 	prvm_prog_t *prog = MVM_prog;
 
+	if (setjmp(mp_abort))
+		return;
+
 	// pass key
 	prog->globals.fp[OFS_PARM0] = (prvm_vec_t) key;
 	prog->globals.fp[OFS_PARM1] = (prvm_vec_t) ascii;
@@ -5322,6 +5336,9 @@ static void MP_Draw (void)
 	if (!prog->loaded)
 		return;
 
+	if (setjmp(mp_abort))
+		return;
+
 	R_SelectScene( RST_MENU );
 
 	// reset the temp entities each frame
@@ -5352,6 +5369,9 @@ static void MP_ToggleMenu(int mode)
 {
 	prvm_prog_t *prog = MVM_prog;
 
+	if (setjmp(mp_abort))
+		return;
+
 	prog->globals.fp[OFS_PARM0] = (prvm_vec_t) mode;
 	prog->ExecuteProgram(prog, PRVM_menufunction(m_toggle),"m_toggle(float mode) required");
 }
@@ -5359,6 +5379,10 @@ static void MP_ToggleMenu(int mode)
 static void MP_NewMap(void)
 {
 	prvm_prog_t *prog = MVM_prog;
+
+	if (setjmp(mp_abort))
+		return;
+
 	if (PRVM_menufunction(m_newmap))
 		prog->ExecuteProgram(prog, PRVM_menufunction(m_newmap),"m_newmap() required");
 }
@@ -5367,6 +5391,10 @@ const serverlist_entry_t *serverlist_callbackentry = NULL;
 static int MP_GetServerListEntryCategory(const serverlist_entry_t *entry)
 {
 	prvm_prog_t *prog = MVM_prog;
+
+	if (setjmp(mp_abort))
+		return 0;
+
 	serverlist_callbackentry = entry;
 	if (PRVM_menufunction(m_gethostcachecategory))
 	{
@@ -5384,6 +5412,10 @@ static int MP_GetServerListEntryCategory(const serverlist_entry_t *entry)
 static void MP_Shutdown (void)
 {
 	prvm_prog_t *prog = MVM_prog;
+
+	if (setjmp(mp_abort))
+		return;
+
 	if (prog->loaded)
 		prog->ExecuteProgram(prog, PRVM_menufunction(m_shutdown),"m_shutdown() required");
 
@@ -5397,6 +5429,10 @@ static void MP_Shutdown (void)
 static void MP_Init (void)
 {
 	prvm_prog_t *prog = MVM_prog;
+
+	if (setjmp(mp_abort))
+		return;
+
 	PRVM_Prog_Init(prog, cmd_local);
 
 	prog->edictprivate_size = 0; // no private struct used
diff --git a/menu.h b/menu.h
index 2344f444..b9624942 100644
--- a/menu.h
+++ b/menu.h
@@ -87,6 +87,9 @@ extern void (*MR_Shutdown) (void);
 extern void (*MR_NewMap) (void);
 extern int (*MR_GetServerListEntryCategory) (const struct serverlist_entry_s *entry);
 
+// menu QC error handling
+extern jmp_buf mp_abort;
+
 typedef struct video_resolution_s
 {
 	const char *type;
diff --git a/mvm_cmds.c b/mvm_cmds.c
index f04ac294..4dc597d7 100644
--- a/mvm_cmds.c
+++ b/mvm_cmds.c
@@ -56,6 +56,8 @@ NULL
 qbool MP_ConsoleCommand(const char *text, size_t textlen)
 {
 	prvm_prog_t *prog = MVM_prog;
+	if (setjmp(mp_abort))
+		return false;
 	return PRVM_ConsoleCommand(prog, text, textlen, &prog->funcoffsets.GameCommand, false, -1, 0, "QC function GameCommand is missing");
 }
 
diff --git a/palette.c b/palette.c
index eac0ca34..8d66b267 100644
--- a/palette.c
+++ b/palette.c
@@ -303,7 +303,7 @@ static void Palette_Load(void)
 		memcpy(palette_rgb, palfile, 768);
 	else
 	{
-		Con_DPrint("Couldn't load gfx/palette.lmp, falling back on internal palette\n");
+		Con_DPrint(CON_WARN "Couldn't load gfx/palette.lmp, falling back on internal palette\n");
 		memcpy(palette_rgb, host_quakepal, 768);
 	}
 	if (palfile)
diff --git a/progsvm.h b/progsvm.h
index 19cc82d3..cb2511d9 100644
--- a/progsvm.h
+++ b/progsvm.h
@@ -743,7 +743,7 @@ typedef struct prvm_prog_s
 	void				(*init_cmd)(struct prvm_prog_s *prog); ///< [INIT] used by PRVM_InitProg
 	void				(*reset_cmd)(struct prvm_prog_s *prog); ///< [INIT] used by PRVM_ResetProg
 
-	void				(*error_cmd)(const char *format, ...) DP_FUNC_PRINTF(1); ///< [INIT]
+	void				(*error_cmd)(const char *format, ...) DP_FUNC_PRINTF(1) DP_FUNC_NORETURN; ///< [INIT]
 
 	void				(*ExecuteProgram)(struct prvm_prog_s *prog, func_t fnum, const char *errormessage); ///< pointer to one of the *VM_ExecuteProgram functions
 } prvm_prog_t;
diff --git a/prvm_exec.c b/prvm_exec.c
index d29aa7f5..3c68f6d1 100644
--- a/prvm_exec.c
+++ b/prvm_exec.c
@@ -745,8 +745,8 @@ void PRVM_Crash(void)
 	}
 
 	// dump the stack so host_error can shutdown functions
-	prog->depth = 0;
-	prog->localstack_used = 0;
+	// and free memory, unset prog->loaded, etc
+	PRVM_Prog_Reset(prog);
 }
 
 /*
diff --git a/sys_shared.c b/sys_shared.c
index ddf905df..1777353a 100644
--- a/sys_shared.c
+++ b/sys_shared.c
@@ -1158,7 +1158,6 @@ static void Sys_Frame(void)
 	host.dirtytime = newtime;
 
 	sleeptime = Host_Frame(time);
-	++host.framecount;
 	sleeptime -= Sys_DirtyTime() - host.dirtytime; // execution time
 
 #ifdef __EMSCRIPTEN__