diff --git a/include/common/log.hh b/include/common/log.hh index 329372a0..faa4ca7c 100644 --- a/include/common/log.hh +++ b/include/common/log.hh @@ -182,11 +182,13 @@ struct stat_tracker_t { const char *name; bool show_even_if_zero; + bool is_warning; std::atomic_size_t count = 0; - inline stat(const char *name, bool show_even_if_zero) : + inline stat(const char *name, bool show_even_if_zero, bool is_warning) : name(name), - show_even_if_zero(show_even_if_zero) + show_even_if_zero(show_even_if_zero), + is_warning(is_warning) { } @@ -200,16 +202,16 @@ struct stat_tracker_t std::list stats; - inline stat ®ister_stat(const char *name, bool show_even_if_zero = false) + inline stat ®ister_stat(const char *name, bool show_even_if_zero = false, bool is_warning = false) { - return stats.emplace_back(name, show_even_if_zero); + return stats.emplace_back(name, show_even_if_zero, is_warning); } ~stat_tracker_t() { for (auto &stat : stats) { if (stat.show_even_if_zero || stat.count) { - print(flag::STAT, " {:8} {}\n", stat.count, stat.name); + print(flag::STAT, "{}{:{}} {}\n", stat.is_warning ? "WARNING: " : "", stat.count, stat.is_warning ? 0 : 13, stat.name); } } } diff --git a/include/qbsp/map.hh b/include/qbsp/map.hh index 8ea3b3c3..d4663289 100644 --- a/include/qbsp/map.hh +++ b/include/qbsp/map.hh @@ -329,7 +329,23 @@ extern mapdata_t map; void CalculateWorldExtent(void); -bool ParseEntity(parser_t &parser, mapentity_t &entity); +struct texture_def_issues_t : logging::stat_tracker_t +{ + // number of faces that have SKY | NODRAW mixed. this is a Q2-specific issue + // that is a bit weird, because NODRAW indicates that the face should not be + // emitted at all in Q1 compilers, whereas in qbsp3 it only left out a texinfo + // reference (in theory...); this meant that sky brushes would disappear. It + // doesn't really make sense to have these two mixed, because sky is drawn in-game + // and the texture is still referenced on them. + stat &num_sky_nodraw = register_stat("faces have SKY | NODRAW flags mixed; NODRAW removed as this combo makes no sense. Use -verbose to display affected faces.", false, true); + + // Q2 specific: TRANSLUCENT is an internal compiler flag and should never + // be set directly. In older tools, the only side effect this has is to + // turn it into DETAIL effectively. + stat &num_translucent = register_stat("faces have TRANSLUCENT flag swapped to DETAIL; TRANSLUCENT is an internal flag. Use -verbose to display affected faces.", false, true); +}; + +bool ParseEntity(parser_t &parser, mapentity_t &entity, texture_def_issues_t &issues_stats); void ProcessExternalMapEntity(mapentity_t &entity); void ProcessAreaPortal(mapentity_t &entity); diff --git a/qbsp/map.cc b/qbsp/map.cc index 73321468..d71bce65 100644 --- a/qbsp/map.cc +++ b/qbsp/map.cc @@ -1384,7 +1384,7 @@ parse_error: } static void ParseTextureDef(parser_t &parser, mapface_t &mapface, const mapbrush_t &brush, maptexinfo_t *tx, - std::array &planepts, const qplane3d &plane) + std::array &planepts, const qplane3d &plane, texture_def_issues_t &issue_stats) { vec_t rotate; qmat texMat, axis; @@ -1477,14 +1477,23 @@ static void ParseTextureDef(parser_t &parser, mapface_t &mapface, const mapbrush if (!(extinfo.info->flags.native & (Q2_SURF_TRANS33 | Q2_SURF_TRANS66))) { extinfo.info->contents.native |= Q2_CONTENTS_DETAIL; - logging::print("WARNING: {}: swapped TRANSLUCENT for DETAIL\n", mapface.line); + if (qbsp_options.verbose.value()) { + logging::print("WARNING: {}: swapped TRANSLUCENT for DETAIL\n", mapface.line); + } else { + issue_stats.num_translucent++; + } } } // This fixes a bug in some old maps. if ((extinfo.info->flags.native & (Q2_SURF_SKY | Q2_SURF_NODRAW)) == (Q2_SURF_SKY | Q2_SURF_NODRAW)) { extinfo.info->flags.native &= ~Q2_SURF_NODRAW; - logging::print("WARNING: {}: SKY | NODRAW mixed. Removing NODRAW.\n", mapface.line); + + if (qbsp_options.verbose.value()) { + logging::print("WARNING: {}: SKY | NODRAW mixed. Removing NODRAW.\n", mapface.line); + } else { + issue_stats.num_sky_nodraw++; + } } } @@ -1604,7 +1613,7 @@ static void ValidateTextureProjection(mapface_t &mapface, maptexinfo_t *tx) } } -static std::optional ParseBrushFace(parser_t &parser, const mapbrush_t &brush, const mapentity_t &entity) +static std::optional ParseBrushFace(parser_t &parser, const mapbrush_t &brush, const mapentity_t &entity, texture_def_issues_t &issue_stats) { std::array planepts; bool normal_ok; @@ -1618,7 +1627,7 @@ static std::optional ParseBrushFace(parser_t &parser, const mapbrush_ normal_ok = face.set_planepts(planepts); - ParseTextureDef(parser, face, brush, &tx, face.planepts, face.get_plane()); + ParseTextureDef(parser, face, brush, &tx, face.planepts, face.get_plane(), issue_stats); if (!normal_ok) { logging::print("WARNING: {}: Brush plane with no normal\n", parser.location); @@ -2083,7 +2092,7 @@ static contentflags_t Brush_GetContents(const mapentity_t &entity, const mapbrus } if (!contents.types_equal(base_contents, qbsp_options.target_game)) { - logging::print("WARNING: {}: mixed face contents ({} != {})\n", + logging::print("WARNING: {}: brush has multiple face contents ({} vs {}), the former will be used.\n", mapface.line, base_contents.to_string(qbsp_options.target_game), contents.to_string(qbsp_options.target_game)); break; } @@ -2114,7 +2123,7 @@ static contentflags_t Brush_GetContents(const mapentity_t &entity, const mapbrus return base_contents; } -static mapbrush_t ParseBrush(parser_t &parser, mapentity_t &entity) +static mapbrush_t ParseBrush(parser_t &parser, mapentity_t &entity, texture_def_issues_t &issue_stats) { mapbrush_t brush; @@ -2150,7 +2159,7 @@ static mapbrush_t ParseBrush(parser_t &parser, mapentity_t &entity) if (parser.token == "}") break; - std::optional face = ParseBrushFace(parser, brush, entity); + std::optional face = ParseBrushFace(parser, brush, entity, issue_stats); if (!face) { continue; @@ -2193,7 +2202,7 @@ static mapbrush_t ParseBrush(parser_t &parser, mapentity_t &entity) return brush; } -bool ParseEntity(parser_t &parser, mapentity_t &entity) +bool ParseEntity(parser_t &parser, mapentity_t &entity, texture_def_issues_t &issue_stats) { entity.location = parser.location; @@ -2235,7 +2244,7 @@ bool ParseEntity(parser_t &parser, mapentity_t &entity) } } while (parser.token != "}"); } else { - entity.mapbrushes.emplace_back(ParseBrush(parser, entity)); + entity.mapbrushes.emplace_back(ParseBrush(parser, entity, issue_stats)); } } else { ParseEpair(parser, entity); @@ -2358,9 +2367,10 @@ static mapentity_t LoadExternalMap(const std::string &filename) } parser_t parser(file, { filename }); + texture_def_issues_t issue_stats; // parse the worldspawn - if (!ParseEntity(parser, dest)) { + if (!ParseEntity(parser, dest, issue_stats)) { FError("'{}': Couldn't parse worldspawn entity\n", filename); } const std::string &classname = dest.epairs.get("classname"); @@ -2370,7 +2380,7 @@ static mapentity_t LoadExternalMap(const std::string &filename) // parse any subsequent entities, move any brushes to worldspawn mapentity_t dummy{}; - while (ParseEntity(parser, dummy)) { + while (ParseEntity(parser, dummy, issue_stats)) { // move the brushes to the worldspawn dest.mapbrushes.insert(dest.mapbrushes.end(), std::make_move_iterator(dummy.mapbrushes.begin()), std::make_move_iterator(dummy.mapbrushes.end())); @@ -2802,55 +2812,59 @@ void LoadMapFile(void) logging::funcheader(); { - auto file = fs::load(qbsp_options.map_path); + texture_def_issues_t issue_stats; - if (!file) { - FError("Couldn't load map file \"{}\".\n", qbsp_options.map_path); - return; - } + { + auto file = fs::load(qbsp_options.map_path); - parser_t parser(file, { qbsp_options.map_path.string() }); - - for (int i = 0;; i++) { - mapentity_t &entity = map.entities.emplace_back(); - - if (!ParseEntity(parser, entity)) { - break; - } - } - - // Remove dummy entity inserted above - assert(!map.entities.back().epairs.size()); - map.entities.pop_back(); - } - - // -add function - if (!qbsp_options.add.value().empty()) { - auto file = fs::load(qbsp_options.add.value()); - - if (!file) { - FError("Couldn't load map file \"{}\".\n", qbsp_options.add.value()); - return; - } - - parser_t parser(file, { qbsp_options.add.value() }); - - for (int i = 0;; i++) { - mapentity_t &entity = map.entities.emplace_back(); - - if (!ParseEntity(parser, entity)) { - break; + if (!file) { + FError("Couldn't load map file \"{}\".\n", qbsp_options.map_path); + return; } - if (entity.epairs.get("classname") == "worldspawn") { - // The easiest way to get the additional map's worldspawn brushes - // into the base map's is to rename the additional map's worldspawn classname to func_group - entity.epairs.set("classname", "func_group"); + parser_t parser(file, { qbsp_options.map_path.string() }); + + for (int i = 0;; i++) { + mapentity_t &entity = map.entities.emplace_back(); + + if (!ParseEntity(parser, entity, issue_stats)) { + break; + } } + + // Remove dummy entity inserted above + assert(!map.entities.back().epairs.size()); + map.entities.pop_back(); + } + + // -add function + if (!qbsp_options.add.value().empty()) { + auto file = fs::load(qbsp_options.add.value()); + + if (!file) { + FError("Couldn't load map file \"{}\".\n", qbsp_options.add.value()); + return; + } + + parser_t parser(file, { qbsp_options.add.value() }); + + for (int i = 0;; i++) { + mapentity_t &entity = map.entities.emplace_back(); + + if (!ParseEntity(parser, entity, issue_stats)) { + break; + } + + if (entity.epairs.get("classname") == "worldspawn") { + // The easiest way to get the additional map's worldspawn brushes + // into the base map's is to rename the additional map's worldspawn classname to func_group + entity.epairs.set("classname", "func_group"); + } + } + // Remove dummy entity inserted above + assert(!map.entities.back().epairs.size()); + map.entities.pop_back(); } - // Remove dummy entity inserted above - assert(!map.entities.back().epairs.size()); - map.entities.pop_back(); } logging::print(logging::flag::STAT, " {:8} entities\n", map.entities.size()); diff --git a/qbsp/qbsp.cc b/qbsp/qbsp.cc index 1716dde2..4bf54a1d 100644 --- a/qbsp/qbsp.cc +++ b/qbsp/qbsp.cc @@ -457,7 +457,7 @@ static void ProcessEntity(mapentity_t &entity, hull_index_t hullnum) Brush_LoadEntity(entity, hullnum, brushes, num_clipped); if (num_clipped && !qbsp_options.verbose.value()) { - logging::print("WARNING: {} faces were clipped away. This is normal for expanded hulls; use -verbose if you need more info.\n", num_clipped); + logging::print(logging::flag::STAT, "WARNING: {} faces were crunched away by being too small. {}Use -verbose to see which faces were affected.\n", num_clipped, hullnum.value_or(0) ? "This is normal for the hulls. " : ""); } size_t num_sides = 0; diff --git a/tests/test_qbsp.cc b/tests/test_qbsp.cc index a3840db3..3b09c4dc 100644 --- a/tests/test_qbsp.cc +++ b/tests/test_qbsp.cc @@ -43,9 +43,10 @@ static mapentity_t &LoadMap(const char *map) parser_t parser(map, { Catch::getResultCapture().getCurrentTestName() }); mapentity_t &entity = ::map.entities.emplace_back(); + texture_def_issues_t issue_stats; // FIXME: adds the brush to the global map... - Q_assert(ParseEntity(parser, entity)); + Q_assert(ParseEntity(parser, entity, issue_stats)); CalculateWorldExtent();