Improve gamedata codegen and fix spurious cases

The old GAMESPECIFIC mechanism had worked in practice but was
technically a little bit incorrect, oops.

9 files changed, 55 insertions, 43 deletions

diff --git a/src/build/gluegen.c b/src/build/gluegen.c
index 3d1eef8..5d02f48 100644
--- a/src/build/gluegen.c
+++ b/src/build/gluegen.c
@@ -767,48 +767,41 @@ F( "	%sif (!GAMETYPE_MATCHES(%.*s)) status_%.*s = FEAT_SKIP;", else_,
 			else_ = "else ";
 		}
 		list_foreach (struct cmeta_slice, gamedata, mod_gamedata + mod) {
-			// this is not a *totally* ideal way of doing this, but it's easy.
-			// if we had some info about what gamedata was doing, we could avoid
-			// having to ifdef these cases and could just directly generate the
-			// right thing. but that'd be quite a bit of work, so... we don't!
 			if (mod_gamespecific[mod].s) {
-F( "#ifdef _GAMES_WITH_%.*s", gamedata.len, gamedata.s)
-F( "	%sif (!(_gametype_tag_%.*s & _GAMES_WITH_%.*s) && !has_%.*s) {", else_,
-				mod_gamespecific[mod].len, mod_gamespecific[mod].s,
-				gamedata.len, gamedata.s, gamedata.len, gamedata.s)
+F( "	%sif (!_HAS_%.*s(_gametype_tag_%.*s)) {", else_,
+		gamedata.len, gamedata.s,
+		mod_gamespecific[mod].len, mod_gamespecific[mod].s)
 F( "		status_%.*s = NOGD;", mod_names[mod].len, mod_names[mod].s)
 _( "	}")
-_( "#else")
 			}
-F( "	%sif (!has_%.*s) status_%.*s = NOGD;", else_,
-			gamedata.len, gamedata.s, mod_names[mod].len, mod_names[mod].s)
-			if (mod_gamespecific[mod].s) {
-_( "#endif")
+			else {
+F( "	%sif (!_HAS_%.*s(0)) status_%.*s = NOGD;", else_,
+		gamedata.len, gamedata.s, mod_names[mod].len, mod_names[mod].s)
 			}
 			else_ = "else ";
 		}
 		list_foreach (struct cmeta_slice, global, mod_globals + mod) {
 F( "	%sif (!(%.*s)) status_%.*s = NOGLOBAL;", else_,
-				global.len, global.s, mod_names[mod].len, mod_names[mod].s)
+		global.len, global.s, mod_names[mod].len, mod_names[mod].s)
 			else_ = "else ";
 		}
 		list_foreach (s16, dep, mod_needs + mod) {
 F( "	%sif (status_%.*s != FEAT_OK) status_%.*s = REQFAIL;", else_,
-				mod_names[dep].len, mod_names[dep].s,
-				mod_names[mod].len, mod_names[mod].s)
+		mod_names[dep].len, mod_names[dep].s,
+		mod_names[mod].len, mod_names[mod].s)
 			else_ = "else ";
 		}
 		if (mod_flags[mod] & (HAS_END | HAS_EVENTS | HAS_OPTDEPS)) {
 F( "	%sif ((status_%.*s = _feat_init_%.*s()) == FEAT_OK) has_%.*s = true;",
-			else_,
-			mod_names[mod].len, mod_names[mod].s,
-			mod_names[mod].len, mod_names[mod].s,
-			mod_names[mod].len, mod_names[mod].s)
+		else_,
+		mod_names[mod].len, mod_names[mod].s,
+		mod_names[mod].len, mod_names[mod].s,
+		mod_names[mod].len, mod_names[mod].s)
 		}
 		else {
 F( "	%sstatus_%.*s = _feat_init_%.*s();", else_,
-			mod_names[mod].len, mod_names[mod].s,
-			mod_names[mod].len, mod_names[mod].s)
+		mod_names[mod].len, mod_names[mod].s,
+		mod_names[mod].len, mod_names[mod].s)
 		}
 	}
 _( "")
diff --git a/src/build/mkentprops.c b/src/build/mkentprops.c
index 17d9aa0..7c195d0 100644
--- a/src/build/mkentprops.c
+++ b/src/build/mkentprops.c
@@ -350,6 +350,7 @@ static inline void dodecls(FILE *out) {
 		const char *s = sbase + decls[i];
 F( "extern int %s;", s);
 F( "#define has_%s (!!%s)", s, s); // offsets will NEVER be 0, due to vtable!
+F( "#define _HAS_%s(x) has_%s", s, s); // HACK: stupid dupe for gluegen
 	}
 }
 
diff --git a/src/build/mkgamedata.c b/src/build/mkgamedata.c
index de70c0b..2ae9228 100644
--- a/src/build/mkgamedata.c
+++ b/src/build/mkgamedata.c
@@ -163,8 +163,7 @@ _( "")
 static inline void knowngames(FILE *out) {
 	// kind of tricky optimisation: if a gamedata entry has no default but
 	// does have game-specific values which match a feature's GAMESPECIFIC()
-	// macro, load-time conditional checks resulting from REQUIRE_GAMEDATA() can
-	// be elided at compile-time.
+	// macro, we can elide has_* and REQUIRE_GAMEDATA() checks at compile time.
 	for (int i = 0, j; i < nents - 1; i = j) {
 		while (exprs[i]) { // if there's a default value, we don't need this
 			// skip to next unindented thing, return if there isn't one with at
@@ -174,21 +173,19 @@ static inline void knowngames(FILE *out) {
 			} while (indents[i] != 0);
 		}
 F( "#line %d \"%" fS "\"", srclines[i], srcnames[srcfiles[i]])
-		if_cold (fprintf(out, "#define _GAMES_WITH_%s (", sbase + tags[i]) < 0) {
+		if_cold (fprintf(out, "#define _GAMES_WITH_%s (0", sbase + tags[i]) < 0) {
 			diewrite();
 		}
-		const char *pipe = "";
 		for (j = i + 1; j < nents && indents[j] != 0; ++j) {
 			// don't attempt to optimise for nested conditionals because that's
 			// way more complicated and also basically defeats the purpose.
-			if (indents[j] != 1) continue;
+			if (indents[j] != 1 || !exprs[j]) continue;
 			bool neg = sbase[tags[j]] == '!';
 			const char *tilde = (const char *)"~" + !neg; // cast away warning
-			if_cold (fprintf(out, "%s \\\n\t %s_gametype_tag_%s", pipe, tilde,
+			if_cold (fprintf(out, " | \\\n\t%s_gametype_tag_%s", tilde,
 					sbase + tags[j] + neg) < 0) {
 				diewrite();
 			}
-			pipe = " |";
 		}
 		fputs(" \\\n)\n", out);
 	}
@@ -199,17 +196,22 @@ static inline void decls(FILE *out) {
 		if (indents[i] != 0) continue;
 F( "#line %d \"%" fS "\"", srclines[i], srcnames[srcfiles[i]])
 		if (exprs[i]) { // default value is specified - entry always exists
-			// *technically* this case is redundant - the other has_ macro would
-			// still work. however, having a distinct case makes the generated
-			// header a little easier to read at a glance.
-F( "#define has_%s 1", sbase + tags[i])
+F( "#define _HAS_%s(feattags) 1", sbase + tags[i])
 		}
 		else { // entry is missing unless a tag is matched
 			// implementation detail: INT_MIN is reserved for missing gamedata!
 			// XXX: for max robustness, should probably check for this in input?
-F( "#define has_%s (%s != -2147483648)", sbase + tags[i], sbase + tags[i])
+F( "#define _HAS_%s(feattags) ( \\", sbase + tags[i])
+_( "	!!feattags && \\")
+F( "		(feattags & _GAMES_WITH_%s) == feattags || \\",
+		sbase + tags[i])
+F( "	%s != -2147483648 \\", sbase + tags[i])
+_(")")
 		}
 F( "#line %d \"%" fS "\"", srclines[i], srcnames[srcfiles[i]])
+F( "#define has_%s _HAS_%s(_gamedata_feattags)",
+		sbase + tags[i], sbase + tags[i])
+F( "#line %d \"%" fS "\"", srclines[i], srcnames[srcfiles[i]])
 		if_cold (i == nents - 1 || !indents[i + 1]) { // no tags - it's constant
 F( "enum { %s = (%s) };", sbase + tags[i], sbase + exprs[i])
 		}
diff --git a/src/chatrate.c b/src/chatrate.c
index 9c1f40c..06c16e9 100644
--- a/src/chatrate.c
+++ b/src/chatrate.c
@@ -17,6 +17,7 @@
 #include "con_.h"
 #include "errmsg.h"
 #include "feature.h"
+#include "gametype.h"
 #include "intdefs.h"
 #include "langext.h"
 #include "os.h"
diff --git a/src/feature.h b/src/feature.h
index aa3ab9d..af593cf 100644
--- a/src/feature.h
+++ b/src/feature.h
@@ -39,15 +39,19 @@
 
 /*
  * Declares that this feature should only be loaded for games matching the given
- * gametype tag (see gametype.h). Console variables and commands created using
- * DEF_FEAT_* macros will not be registered if SST is loaded by some other game.
- *
- * As an optimisation, REQUIRE_GAMEDATA() checks (see below) can also be elided
- * in cases where gamedata is always present for this particular game. As such,
- * it is wise to still specify gamedata dependencies correctly, so that the
- * definitions can be changed in the data files without breaking code.
+ * gametype tag. gametype.h must be included to use this as it defines the tag
+ * values. Console variables and commands created using DEF_FEAT_* macros will
+ * not be registered if SST is loaded by some other game.
+ *
+ * This also enables a build-time optimisation to elide REQUIRE_GAMEDATA()
+ * checks as well as has_* conditionals. As such, it is wise to still specify
+ * gamedata dependencies correctly, so that the definitions can be changed in
+ * the data files without breaking code.
  */
-#define GAMESPECIFIC(tag)
+#define GAMESPECIFIC(tag) \
+	/* impl note: see comment in gamedata.h */ \
+	__attribute((unused)) \
+	static const int _gamedata_feattags = _gametype_tag_##tag;
 
 /*
  * Indicates that the specified feature is required for this feature to
diff --git a/src/fov.c b/src/fov.c
index 2ef13e9..c822584 100644
--- a/src/fov.c
+++ b/src/fov.c
@@ -24,6 +24,7 @@
 #include "ent.h"
 #include "event.h"
 #include "feature.h"
+#include "gametype.h"
 #include "hook.h"
 #include "intdefs.h"
 #include "langext.h"
diff --git a/src/gamedata.h b/src/gamedata.h
index 7d91373..c14c83c 100644
--- a/src/gamedata.h
+++ b/src/gamedata.h
@@ -17,6 +17,15 @@
 #ifndef INC_GAMEDATA_H
 #define INC_GAMEDATA_H
 
+#include "gametype.h"
+
+// this defaults to zero (tentative definition), but gets defined to a value by
+// GAMESPECIFIC() in feature.h. static const int variables get constant-folded
+// even in -O0. so, this lets us short-circuit has_ checks inside features.
+// we also check if a gamedata entry's
+__attribute((unused))
+static const int _gamedata_feattags;
+
 // STUPID HACK to avoid pollution if abi.h not already included (only because
 // generated gamedata stuff relies on this being defined)
 #ifndef NVDTOR
diff --git a/src/gametype.h b/src/gametype.h
index 5b9336b..4e2dc55 100644
--- a/src/gametype.h
+++ b/src/gametype.h
@@ -30,7 +30,7 @@ extern u32 _gametype_tag;
 	ALL(2013) \
 \
 	/* specific games with dedicated branches / engine changes */ \
-	/* TODO(compat): detect dmomm, if only to fail (VEngineServer broke) */ \
+	/* TODO(compat): dmomm seems to fail currently (VEngineServer broke?) */ \
 	WINDOWSONLY(DMoMM) \
 	WINDOWSONLY(L4D1) \
 	ALL(L4D2) \
diff --git a/src/l4d1democompat.c b/src/l4d1democompat.c
index eff6ad0..3c9b604 100644
--- a/src/l4d1democompat.c
+++ b/src/l4d1democompat.c
@@ -20,6 +20,7 @@
 #include "con_.h"
 #include "errmsg.h"
 #include "feature.h"
+#include "gametype.h"
 #include "hook.h"
 #include "intdefs.h"
 #include "mem.h"