From f14f327a9aaa437829623073213365c3be877e88 Mon Sep 17 00:00:00 2001 From: Eric Wasylishen Date: Sat, 13 May 2023 00:46:02 -0600 Subject: [PATCH] common: more robust hexen2 detection Fixes #355 --- common/bspfile.cc | 66 ++++++++++++++++++++++++++++++----- testmaps/h2_skip_only.map | 60 ++++++++++++++++++++++++++++++++ testmaps/q1_skip_only.map | 73 +++++++++++++++++++++++++++++++++++++++ tests/test_qbsp.cc | 20 +++++++++++ 4 files changed, 210 insertions(+), 9 deletions(-) create mode 100644 testmaps/h2_skip_only.map create mode 100644 testmaps/q1_skip_only.map diff --git a/common/bspfile.cc b/common/bspfile.cc index 9c4a9a12..07fa2fe0 100644 --- a/common/bspfile.cc +++ b/common/bspfile.cc @@ -2207,15 +2207,63 @@ bool ConvertBSPFormat(bspdata_t *bspdata, const bspversion_t *to_version) return false; } -static bool isHexen2(const dheader_t *header) +static bool isHexen2(const dheader_t *header, const bspversion_t *bspversion) { - /* - the world should always have some face. - however, if the sizes are wrong then we're actually reading headnode[6]. hexen2 only used 5 hulls, so this - should be 0 in hexen2, and not in quake. - */ - const dmodelq1_t *modelsq1 = (const dmodelq1_t *)((const uint8_t *)header + header->lumps[LUMP_MODELS].fileofs); - return !modelsq1->numfaces; + if (0 != (header->lumps[LUMP_MODELS].filelen % sizeof(dmodelh2_t))) { + // definitely not H2 + return false; + } + if (0 != (header->lumps[LUMP_MODELS].filelen % sizeof(dmodelq1_t))) { + // definitely not Q1 + return true; + } + + // models lump is divisible by both the Q1 model size (64 bytes) and the H2 model size (80 bytes). + + const int bytes_per_face = bspversion->lumps.begin()[LUMP_FACES].size; + const int bytes_per_node = bspversion->lumps.begin()[LUMP_NODES].size; + const int bytes_per_leaf = bspversion->lumps.begin()[LUMP_LEAFS].size; + const int bytes_per_clipnode = bspversion->lumps.begin()[LUMP_CLIPNODES].size; + + const int faces_count = header->lumps[LUMP_FACES].filelen / bytes_per_face; + const int nodes_count = header->lumps[LUMP_NODES].filelen / bytes_per_node; + const int leafs_count = header->lumps[LUMP_LEAFS].filelen / bytes_per_leaf; + const int clipnodes_count = header->lumps[LUMP_CLIPNODES].filelen / bytes_per_clipnode; + + // assume H2, and do some basic validation + // FIXME: this potentially does unaligned reads, convert to using streams like the rest of the loading code + + const dmodelh2_t *models_h2 = (const dmodelh2_t *)((const uint8_t *)header + header->lumps[LUMP_MODELS].fileofs); + const int models_h2_count = header->lumps[LUMP_MODELS].filelen / sizeof(dmodelh2_t); + + for (int i = 0; i < models_h2_count; ++i) { + const dmodelh2_t *model = &models_h2[i]; + + // visleafs + if (model->visleafs < 0 || model->visleafs > leafs_count) + return false; + + // numfaces, firstface + if (model->numfaces < 0) + return false; + if (model->firstface < 0) + return false; + if (model->firstface + model->numfaces > faces_count) + return false; + + // headnode[0] + if (model->headnode[0] >= nodes_count) + return false; + + // clipnode headnodes + for (int j = 1; j < 8; ++j) { + if (model->headnode[j] >= clipnodes_count) + return false; + } + } + + // passed all checks, assume H2 + return true; } struct lump_reader @@ -2425,7 +2473,7 @@ void LoadBSPFile(fs::path &filename, bspdata_t *bspdata) Error("Sorry, this bsp version is not supported."); } else { // special case handling for Hexen II - if (bspdata->version->game->id == GAME_QUAKE && isHexen2((dheader_t *)file_data->data())) { + if (bspdata->version->game->id == GAME_QUAKE && isHexen2((dheader_t *)file_data->data(), bspdata->version)) { if (bspdata->version == &bspver_q1) { bspdata->version = &bspver_h2; } else if (bspdata->version == &bspver_bsp2) { diff --git a/testmaps/h2_skip_only.map b/testmaps/h2_skip_only.map new file mode 100644 index 00000000..918bc2b2 --- /dev/null +++ b/testmaps/h2_skip_only.map @@ -0,0 +1,60 @@ +// Game: Quake +// Format: Valve +// entity 0 +{ +"mapversion" "220" +"classname" "worldspawn" +// brush 0 +{ +( 32 -256 112 ) ( 32 -255 112 ) ( 32 -256 113 ) skip [ 0 1 0 -16 ] [ 0 0 -1 0 ] 0 1 1 +( 64 -240 96 ) ( 63 -240 96 ) ( 64 -240 97 ) skip [ -1 0 0 16 ] [ 0 0 -1 0 ] 180 1 1 +( 64 -576 80 ) ( 64 -575 80 ) ( 63 -576 80 ) skip [ 1 0 0 -16 ] [ 0 -1 0 16 ] 180 1 1 +( -16 -256 112 ) ( -17 -256 112 ) ( -16 -255 112 ) skip [ -1 0 0 16 ] [ 0 -1 0 16 ] 180 1 1 +( -16 -144 112 ) ( -16 -144 113 ) ( -17 -144 112 ) skip [ 1 0 0 -16 ] [ 0 0 -1 0 ] 180 1 1 +( 80 -576 96 ) ( 80 -576 97 ) ( 80 -575 96 ) skip [ 0 -1 0 16 ] [ 0 0 -1 0 ] 0 1 1 +} +} +// entity 1 +{ +"classname" "info_player_start" +"origin" "56 -208 136" +} +// entity 2 +{ +"classname" "func_wall" +// brush 0 +{ +( -48 -256 240 ) ( -48 -255 240 ) ( -48 -256 241 ) skip [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +( -16 -240 224 ) ( -17 -240 224 ) ( -16 -240 225 ) skip [ -1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( -16 -576 208 ) ( -16 -575 208 ) ( -17 -576 208 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -256 240 ) ( -97 -256 240 ) ( -96 -255 240 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -144 240 ) ( -96 -144 241 ) ( -97 -144 240 ) skip [ 1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( 0 -576 224 ) ( 0 -576 225 ) ( 0 -575 224 ) skip [ 0 -1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +} +} +// entity 3 +{ +"classname" "func_wall" +// brush 0 +{ +( -48 -256 144 ) ( -48 -255 144 ) ( -48 -256 145 ) skip [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +( -16 -240 128 ) ( -17 -240 128 ) ( -16 -240 129 ) skip [ -1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( -16 -576 112 ) ( -16 -575 112 ) ( -17 -576 112 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -256 144 ) ( -97 -256 144 ) ( -96 -255 144 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -144 144 ) ( -96 -144 145 ) ( -97 -144 144 ) skip [ 1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( 0 -576 128 ) ( 0 -576 129 ) ( 0 -575 128 ) skip [ 0 -1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +} +} +// entity 4 +{ +"classname" "func_wall" +// brush 0 +{ +( -48 -256 192 ) ( -48 -255 192 ) ( -48 -256 193 ) skip [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +( -16 -240 176 ) ( -17 -240 176 ) ( -16 -240 177 ) skip [ -1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( -16 -576 160 ) ( -16 -575 160 ) ( -17 -576 160 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -256 192 ) ( -97 -256 192 ) ( -96 -255 192 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -144 192 ) ( -96 -144 193 ) ( -97 -144 192 ) skip [ 1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( 0 -576 176 ) ( 0 -576 177 ) ( 0 -575 176 ) skip [ 0 -1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +} +} diff --git a/testmaps/q1_skip_only.map b/testmaps/q1_skip_only.map new file mode 100644 index 00000000..b28db6b0 --- /dev/null +++ b/testmaps/q1_skip_only.map @@ -0,0 +1,73 @@ +// Game: Quake +// Format: Valve +// entity 0 +{ +"mapversion" "220" +"classname" "worldspawn" +// brush 0 +{ +( 32 -256 112 ) ( 32 -255 112 ) ( 32 -256 113 ) skip [ 0 1 0 -16 ] [ 0 0 -1 0 ] 0 1 1 +( 64 -240 96 ) ( 63 -240 96 ) ( 64 -240 97 ) skip [ -1 0 0 16 ] [ 0 0 -1 0 ] 180 1 1 +( 64 -576 80 ) ( 64 -575 80 ) ( 63 -576 80 ) skip [ 1 0 0 -16 ] [ 0 -1 0 16 ] 180 1 1 +( -16 -256 112 ) ( -17 -256 112 ) ( -16 -255 112 ) skip [ -1 0 0 16 ] [ 0 -1 0 16 ] 180 1 1 +( -16 -144 112 ) ( -16 -144 113 ) ( -17 -144 112 ) skip [ 1 0 0 -16 ] [ 0 0 -1 0 ] 180 1 1 +( 80 -576 96 ) ( 80 -576 97 ) ( 80 -575 96 ) skip [ 0 -1 0 16 ] [ 0 0 -1 0 ] 0 1 1 +} +} +// entity 1 +{ +"classname" "info_player_start" +"origin" "56 -208 136" +} +// entity 2 +{ +"classname" "func_wall" +// brush 0 +{ +( -48 -256 240 ) ( -48 -255 240 ) ( -48 -256 241 ) skip [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +( -16 -240 224 ) ( -17 -240 224 ) ( -16 -240 225 ) skip [ -1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( -16 -576 208 ) ( -16 -575 208 ) ( -17 -576 208 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -256 240 ) ( -97 -256 240 ) ( -96 -255 240 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -144 240 ) ( -96 -144 241 ) ( -97 -144 240 ) skip [ 1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( 0 -576 224 ) ( 0 -576 225 ) ( 0 -575 224 ) skip [ 0 -1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +} +} +// entity 3 +{ +"classname" "func_wall" +// brush 0 +{ +( -48 -256 288 ) ( -48 -255 288 ) ( -48 -256 289 ) skip [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +( -16 -240 272 ) ( -17 -240 272 ) ( -16 -240 273 ) skip [ -1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( -16 -576 256 ) ( -16 -575 256 ) ( -17 -576 256 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -256 288 ) ( -97 -256 288 ) ( -96 -255 288 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -144 288 ) ( -96 -144 289 ) ( -97 -144 288 ) skip [ 1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( 0 -576 272 ) ( 0 -576 273 ) ( 0 -575 272 ) skip [ 0 -1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +} +} +// entity 4 +{ +"classname" "func_wall" +// brush 0 +{ +( -48 -256 144 ) ( -48 -255 144 ) ( -48 -256 145 ) skip [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +( -16 -240 128 ) ( -17 -240 128 ) ( -16 -240 129 ) skip [ -1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( -16 -576 112 ) ( -16 -575 112 ) ( -17 -576 112 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -256 144 ) ( -97 -256 144 ) ( -96 -255 144 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -144 144 ) ( -96 -144 145 ) ( -97 -144 144 ) skip [ 1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( 0 -576 128 ) ( 0 -576 129 ) ( 0 -575 128 ) skip [ 0 -1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +} +} +// entity 5 +{ +"classname" "func_wall" +// brush 0 +{ +( -48 -256 192 ) ( -48 -255 192 ) ( -48 -256 193 ) skip [ 0 1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +( -16 -240 176 ) ( -17 -240 176 ) ( -16 -240 177 ) skip [ -1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( -16 -576 160 ) ( -16 -575 160 ) ( -17 -576 160 ) skip [ 1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -256 192 ) ( -97 -256 192 ) ( -96 -255 192 ) skip [ -1 0 0 0 ] [ 0 -1 0 0 ] 180 1 1 +( -96 -144 192 ) ( -96 -144 193 ) ( -97 -144 192 ) skip [ 1 0 0 0 ] [ 0 0 -1 0 ] 180 1 1 +( 0 -576 176 ) ( 0 -576 177 ) ( 0 -575 176 ) skip [ 0 -1 0 0 ] [ 0 0 -1 0 ] 0 1 1 +} +} diff --git a/tests/test_qbsp.cc b/tests/test_qbsp.cc index 706a3232..1cba063c 100644 --- a/tests/test_qbsp.cc +++ b/tests/test_qbsp.cc @@ -1762,3 +1762,23 @@ TEST_CASE("textures search relative to current directory") CHECK(64 == bsp.dtex.textures[1].height); CHECK(bsp.dtex.textures[1].data.size() > 0); } + +// specifically designed to break the old isHexen2() +// (has 0 faces, and model lump size is divisible by both Q1 and H2 model struct size) +TEST_CASE("q1_skip_only") +{ + const auto [bsp, bspx, prt] = LoadTestmapQ1("q1_skip_only.map"); + + CHECK(bsp.loadversion == &bspver_q1); + CHECK(0 == bsp.dfaces.size()); +} + +// specifically designed to break the old isHexen2() +// (has 0 faces, and model lump size is divisible by both Q1 and H2 model struct size) +TEST_CASE("h2_skip_only") +{ + const auto [bsp, bspx, prt] = LoadTestmap("h2_skip_only.map", {"-hexen2"}); + + CHECK(bsp.loadversion == &bspver_h2); + CHECK(0 == bsp.dfaces.size()); +}