Separate our command callbacks from Source's

This improves the ergonomics of a few different things, and sets us up
somewhat for the fact OE had a different interface for commands too
(it was v1 only and had a separate API call for getting the args).

9 files changed, 53 insertions, 55 deletions

diff --git a/src/alias.c b/src/alias.c
index 300ea10..697e97c 100644
--- a/src/alias.c
+++ b/src/alias.c
@@ -51,7 +51,7 @@ void alias_rm(const char *name) {
 }
 
 DEF_FEAT_CCMD_HERE(sst_alias_clear, "Remove all command aliases", 0) {
-	if (cmd->argc != 1) {
+	if (argc != 1) {
 		con_warn("usage: sst_alias_clear\n");
 		return;
 	}
@@ -59,19 +59,18 @@ DEF_FEAT_CCMD_HERE(sst_alias_clear, "Remove all command aliases", 0) {
 }
 
 DEF_FEAT_CCMD_HERE(sst_alias_remove, "Remove a command alias", 0) {
-	if (cmd->argc != 2) {
+	if (argc != 2) {
 		con_warn("usage: sst_alias_remove name\n");
 		return;
 	}
-	if (strlen(cmd->argv[1]) > 31) {
+	if (strlen(argv[1]) > 31) {
 		con_warn("invalid alias name (too long)\n");
 		return;
 	}
-	alias_rm(cmd->argv[1]);
+	alias_rm(argv[1]);
 }
 
-static bool find_alias_head(con_cmdcb alias_cb) {
-	const uchar *insns = (const uchar *)alias_cb;
+static bool find_alias_head(const uchar *insns) {
 #ifdef _WIN32
 	for (const uchar *p = insns; p - insns < 64;) {
 		// alias command with no args calls ConMsg() then loads the head pointer
@@ -94,7 +93,7 @@ INIT {
 	if (GAMETYPE_MATCHES(Portal2)) return FEAT_INCOMPAT;
 
 	struct con_cmd *cmd_alias = con_findcmd("alias");
-	if_cold (!find_alias_head(con_getcmdcb(cmd_alias))) {
+	if_cold (!find_alias_head(cmd_alias->cb_insns)) {
 		errmsg_warnx("couldn't find alias list");
 		return FEAT_INCOMPAT;
 	}
diff --git a/src/bind.c b/src/bind.c
index a746ef5..7fafdb1 100644
--- a/src/bind.c
+++ b/src/bind.c
@@ -36,9 +36,8 @@ static struct keyinfo *keyinfo; // engine keybinds list (s_pKeyInfo[])
 
 const char *bind_get(int keycode) { return keyinfo[keycode].binding; }
 
-static bool find_keyinfo(con_cmdcb klbc_cb) {
+static bool find_keyinfo(const uchar *insns) {
 #ifdef _WIN32
-	const uchar *insns = (const uchar *)klbc_cb;
 	for (const uchar *p = insns; p - insns < 64;) {
 		// key_listboundkeys loops through each index, moving into a register:
 		// mov <reg>, dword ptr [<reg> * 8 + s_pKeyInfo]
@@ -57,8 +56,7 @@ static bool find_keyinfo(con_cmdcb klbc_cb) {
 
 INIT {
 	struct con_cmd *cmd_key_listboundkeys = con_findcmd("key_listboundkeys");
-	con_cmdcb cb = con_getcmdcb(cmd_key_listboundkeys);
-	if_cold (!find_keyinfo(cb)) {
+	if_cold (!find_keyinfo(cmd_key_listboundkeys->cb_insns)) {
 		errmsg_warnx("couldn't find key binding list");
 		return FEAT_INCOMPAT;
 	}
diff --git a/src/con_.c b/src/con_.c
index ddebf78..b2d7e29 100644
--- a/src/con_.c
+++ b/src/con_.c
@@ -136,8 +136,7 @@ bool VCALLCONV CanAutoComplete(struct con_cmd *this) {
 	return false;
 }
 void VCALLCONV Dispatch(struct con_cmd *this, const struct con_cmdargs *args) {
-	// just assuming we *always* define a (V2) callback, per the DEF_CCMD macros
-	this->cb(args);
+	this->cb(args->argc, args->argv);
 }
 
 static void VCALLCONV ChangeStringValue(struct con_var *this, const char *s,
@@ -484,8 +483,8 @@ SETTER(float, vtidx_SetValue_f, con_setvarf)
 SETTER(int, vtidx_SetValue_i, con_setvari)
 #undef SETTER
 
-con_cmdcb con_getcmdcb(const struct con_cmd *cmd) {
-	return !cmd->use_newcmdiface && cmd->use_newcb ? cmd->cb : 0;
+con_cmdcbv2 con_getcmdcbv2(const struct con_cmd *cmd) {
+	return !cmd->use_newcmdiface && cmd->use_newcb ? cmd->cb_v2 : 0;
 }
 
 con_cmdcbv1 con_getcmdcbv1(const struct con_cmd *cmd) {
diff --git a/src/con_.h b/src/con_.h
index 8fc9756..b491aad 100644
--- a/src/con_.h
+++ b/src/con_.h
@@ -66,10 +66,13 @@ enum {
 	CON_CCMDEXEC = 1 << 30		/* ClientCmd() function may run the command */
 };
 
-/* A callback function invoked to execute a command. */
-typedef void (*con_cmdcb)(const struct con_cmdargs *cmd);
+/* A callback function invoked by SST to execute its own commands. */
+typedef void (*con_cmdcb)(int argc, const char *const *argv);
 
-/* Obsolete callback; not used by SST, but might still exist in the engine. */
+/* A callback function used by most commands in most versions of the engine. */
+typedef void (*con_cmdcbv2)(const struct con_cmdargs *cmd);
+
+/* An older style of callback function used by some old commands, and in OE. */
 typedef void (*con_cmdcbv1)();
 
 /*
@@ -101,9 +104,11 @@ struct con_cmdbase { // ConCommandBase in engine
 struct con_cmd { // ConCommand in engine
 	struct con_cmdbase base;
 	union {
+		con_cmdcb cb; // N.B.: only used by *our* commands!
 		con_cmdcbv1 cb_v1;
-		con_cmdcb cb;
-		/*ICommandCallback*/ void *cb_iface; // does source even use this?
+		con_cmdcbv2 cb_v2;
+		const uchar *cb_insns; // for the sake of instruction-scanning and such
+		/*ICommandCallback*/ void *cb_iface; // what in Source even uses this?
 	};
 	union {
 		con_complcb complcb;
@@ -160,7 +165,7 @@ void con_setvari(struct con_var *v, int i);
  * callback being requested. If this is already known, consider just grabbing
  * the member directly to avoid the small amount of unnecessary work.
  */
-con_cmdcb con_getcmdcb(const struct con_cmd *cmd);
+con_cmdcbv2 con_getcmdcbv2(const struct con_cmd *cmd);
 con_cmdcbv1 con_getcmdcbv1(const struct con_cmd *cmd);
 
 /*
@@ -264,7 +269,6 @@ extern struct _con_vtab_iconvar_wrap {
 			.name = "" #name_, .help = "" desc, .flags = (flags_) \
 		}, \
 		.cb = &func, \
-		.use_newcb = true \
 	}; \
 	struct con_cmd *varname = &_ccmd_##varname;
 
@@ -282,13 +286,13 @@ extern struct _con_vtab_iconvar_wrap {
 
 /*
  * Defines a console command with the handler function body immediately
- * following the macro (like in Source itself). The function takes the argument
- * `struct con_cmdargs *cmd` for command arguments.
+ * following the macro (like in Source itself). The function takes the implicit
+ * arguments `int argc` and `const char *const *argv` for command arguments.
  */
 #define DEF_CCMD_HERE(name, desc, flags) \
-	static void _cmdf_##name(const struct con_cmdargs *cmd); \
+	static void _cmdf_##name(int argc, const char *const *argv); \
 	_DEF_CCMD(name, name, desc, _cmdf_##name, flags) \
-	static void _cmdf_##name(const struct con_cmdargs *cmd) \
+	static void _cmdf_##name(int argc, const char *const *argv) \
 	/* { body here } */
 
 /*
diff --git a/src/dbg.c b/src/dbg.c
index 180c686..af995ce 100644
--- a/src/dbg.c
+++ b/src/dbg.c
@@ -82,18 +82,18 @@ void dbg_asmdump(const char *name, const void *p, int len) {
 }
 
 DEF_CCMD_HERE(sst_dbg_getcmdcb, "Get the address of a command callback", 0) {
-	if (cmd->argc != 2) {
+	if (argc != 2) {
 		con_warn("usage: sst_dbg_getcmdcb command\n");
 		return;
 	}
-	struct con_cmd *thecmd = con_findcmd(cmd->argv[1]);
+	struct con_cmd *thecmd = con_findcmd(argv[1]);
 	if (!thecmd) {
-		errmsg_errorstd("couldn't find command %s\n", cmd->argv[1]);
+		errmsg_errorstd("couldn't find command %s\n", argv[1]);
 		return;
 	}
 #ifdef _WIN32
-	con_msg("addr: %p\nghidra: %p\n", (void *)thecmd->cb,
-			(void *)dbg_toghidra((void *)thecmd->cb)); // ugh
+	con_msg("addr: %p\nghidra: %p\n", (void *)thecmd->cb_insns,
+			(void *)dbg_toghidra((void *)thecmd->cb_insns)); // ugh
 #else
 	con_msg("addr: %p\n", (void *)thecmd->cb);
 #endif
diff --git a/src/demorec.c b/src/demorec.c
index 7bdc517..6f1b2a7 100644
--- a/src/demorec.c
+++ b/src/demorec.c
@@ -101,7 +101,7 @@ static void VCALLCONV hook_StopRecording(struct CDemoRecorder *this) {
 DECL_VFUNC_DYN(struct CDemoRecorder, void, StartRecording)
 
 static struct con_cmd *cmd_record, *cmd_stop;
-static con_cmdcb orig_record_cb, orig_stop_cb;
+static con_cmdcbv2 orig_record_cb, orig_stop_cb;
 
 static void hook_record_cb(const struct con_cmdargs *args) {
 	if_cold (!CHECK_DemoControlAllowed()) return;
@@ -260,9 +260,9 @@ int demorec_demonum() {
 
 INIT {
 	cmd_record = con_findcmd("record");
-	orig_record_cb = con_getcmdcb(cmd_record);
+	orig_record_cb = con_getcmdcbv2(cmd_record);
 	cmd_stop = con_findcmd("stop");
-	orig_stop_cb = con_getcmdcb(cmd_stop);
+	orig_stop_cb = con_getcmdcbv2(cmd_stop);
 	if_cold (!find_demorecorder()) {
 		errmsg_errorx("couldn't find demo recorder instance");
 		return FEAT_INCOMPAT;
@@ -287,8 +287,8 @@ INIT {
 	orig_StopRecording = (StopRecording_func)hook_vtable(vtable,
 			vtidx_StopRecording, (void *)&hook_StopRecording);
 
-	cmd_record->cb = &hook_record_cb;
-	cmd_stop->cb = &hook_stop_cb;
+	cmd_record->cb_v2 = &hook_record_cb;
+	cmd_stop->cb_v2 = &hook_stop_cb;
 
 	return FEAT_OK;
 }
@@ -300,8 +300,8 @@ END {
 	void **vtable = demorecorder->vtable;
 	unhook_vtable(vtable, vtidx_SetSignonState, (void *)orig_SetSignonState);
 	unhook_vtable(vtable, vtidx_StopRecording, (void *)orig_StopRecording);
-	cmd_record->cb = orig_record_cb;
-	cmd_stop->cb = orig_stop_cb;
+	cmd_record->cb_v2 = orig_record_cb;
+	cmd_stop->cb_v2 = orig_stop_cb;
 }
 
 // vi: sw=4 ts=4 noet tw=80 cc=80
diff --git a/src/l4dreset.c b/src/l4dreset.c
index 207e487..725c763 100644
--- a/src/l4dreset.c
+++ b/src/l4dreset.c
@@ -349,7 +349,7 @@ static int *FinaleEscapeState;
 
 DEF_FEAT_CCMD_HERE(sst_l4d_quickreset,
 		"Reset (or switch) campaign and clear all vote cooldowns", 0) {
-	if (cmd->argc > 2) {
+	if (argc > 2) {
 		con_warn("usage: sst_l4d_quickreset [campaignid]\n");
 		return;
 	}
@@ -358,9 +358,9 @@ DEF_FEAT_CCMD_HERE(sst_l4d_quickreset,
 		return;
 	}
 	const char *campaign = l4dmm_curcampaign();
-	if (cmd->argc == 2 && (!campaign || strcasecmp(campaign, cmd->argv[1]))) {
-		change(cmd->argv[1]);
-		campaign = cmd->argv[1];
+	if (argc == 2 && (!campaign || strcasecmp(campaign, argv[1]))) {
+		change(argv[1]);
+		campaign = argv[1];
 		nextmapnum = gameserver_spawncount() + 1; // immediate next changelevel
 	}
 	else {
@@ -381,8 +381,7 @@ DEF_FEAT_CCMD_HERE(sst_l4d_quickreset,
 }
 
 // Note: this returns a pointer to subsequent bytes for find_voteissues() below
-static inline const uchar *find_votecontroller(con_cmdcbv1 listissues_cb) {
-	const uchar *insns = (const uchar *)listissues_cb;
+static inline const uchar *find_votecontroller(const uchar *insns) {
 #ifdef _WIN32
 	// The "listissues" command calls CVoteController::ListIssues, loading
 	// g_voteController into ECX
@@ -510,8 +509,7 @@ INIT {
 		errmsg_errorx("couldn't find \"listissues\" command");
 		return FEAT_INCOMPAT;
 	}
-	con_cmdcbv1 listissues_cb = con_getcmdcbv1(cmd_listissues);
-	const uchar *nextinsns = find_votecontroller(listissues_cb);
+	const uchar *nextinsns = find_votecontroller(cmd_listissues->cb_insns);
 	if_cold (!nextinsns) {
 		errmsg_errorx("couldn't find vote controller variable");
 		return FEAT_INCOMPAT;
diff --git a/src/l4dwarp.c b/src/l4dwarp.c
index 03edb9e..462239d 100644
--- a/src/l4dwarp.c
+++ b/src/l4dwarp.c
@@ -110,10 +110,10 @@ DEF_FEAT_CCMD_HERE(sst_l4d_testwarp, "Simulate a bot warping to you "
 		CON_SERVERSIDE | CON_CHEAT) {
 	bool staystuck = false;
 	// TODO(autocomplete): suggest this argument
-	if (cmd->argc == 2 && !strcmp(cmd->argv[1], "staystuck")) {
+	if (argc == 2 && !strcmp(argv[1], "staystuck")) {
 		staystuck = true;
 	}
-	else if (cmd->argc != 1) {
+	else if (argc != 1) {
 		clientcon_reply("usage: sst_l4d_testwarp [staystuck]\n");
 		return;
 	}
diff --git a/src/sst.c b/src/sst.c
index ef14aca..4f9445d 100644
--- a/src/sst.c
+++ b/src/sst.c
@@ -419,7 +419,7 @@ DEF_EVENT(PluginLoaded)
 DEF_EVENT(PluginUnloaded)
 
 static struct con_cmd *cmd_plugin_load, *cmd_plugin_unload;
-static con_cmdcb orig_plugin_load_cb, orig_plugin_unload_cb;
+static con_cmdcbv2 orig_plugin_load_cb, orig_plugin_unload_cb;
 
 static int ownidx; // XXX: super hacky way of getting this to do_unload()
 
@@ -518,11 +518,11 @@ static bool do_load(ifacefactory enginef, ifacefactory serverf) {
 	if (!deferinit()) { do_featureinit(); fixes_apply(); }
 	if_hot (pluginhandler) {
 		cmd_plugin_load = con_findcmd("plugin_load");
-		orig_plugin_load_cb = cmd_plugin_load->cb;
-		cmd_plugin_load->cb = &hook_plugin_load_cb;
+		orig_plugin_load_cb = cmd_plugin_load->cb_v2;
+		cmd_plugin_load->cb_v2 = &hook_plugin_load_cb;
 		cmd_plugin_unload = con_findcmd("plugin_unload");
-		orig_plugin_unload_cb = cmd_plugin_unload->cb;
-		cmd_plugin_unload->cb = &hook_plugin_unload_cb;
+		orig_plugin_unload_cb = cmd_plugin_unload->cb_v2;
+		cmd_plugin_unload->cb_v2 = &hook_plugin_unload_cb;
 	}
 	return true;
 }
@@ -530,8 +530,8 @@ static bool do_load(ifacefactory enginef, ifacefactory serverf) {
 static void do_unload() {
 	// slow path: reloading shouldn't happen all the time, prioritise fast exit
 	if_cold (sst_userunloaded) { // note: if we're here, pluginhandler is set
-		cmd_plugin_load->cb = orig_plugin_load_cb;
-		cmd_plugin_unload->cb = orig_plugin_unload_cb;
+		cmd_plugin_load->cb_v2 = orig_plugin_load_cb;
+		cmd_plugin_unload->cb_v2 = orig_plugin_unload_cb;
 #ifdef _WIN32 // this bit is only relevant in builds that predate linux support
 		struct CPlugin **plugins = pluginhandler->plugins.m.mem;
 		// see comment in CPlugin struct. setting this to the real handle right