- Fixed error reporting for the BSP loader

Using global variables for this is bad, and it didn't even catch all cases. Now a node build is only considered successful if everything is set up successfully.

src/maploader/maploader.cpp

@@ -648,7 +648,7 @@ void MapLoader::LoadZNodes(FileReader &data, int glnodes)
 //
 //===========================================================================
 
-void MapLoader::LoadExtendedNodes (FileReader &dalump, uint32_t id)
+bool MapLoader::LoadExtendedNodes (FileReader &dalump, uint32_t id)
 {
 	int type;
 	bool compressed;
@@ -696,9 +696,11 @@ void MapLoader::LoadExtendedNodes (FileReader &dalump, uint32_t id)
 		break;
 
 	default:
-		return;
+		return false;
 	}
 	
+	try
+	{
 		if (compressed)
 		{
 			FileReader zip;
@@ -706,11 +708,28 @@ void MapLoader::LoadExtendedNodes (FileReader &dalump, uint32_t id)
 			{
 				LoadZNodes(zip, type);
 			}
+			else
+			{
+				Printf("Error loading nodes: Corrupt data.\n");
+				return false;
+			}
 		}
 		else
 		{
 			LoadZNodes(dalump, type);
 		}
+		return true;
+	}
+	catch (CRecoverableError &error)
+	{
+		Printf("Error loading nodes: %s\n", error.GetMessage());
+		
+		Level->subsectors.Clear();
+		Level->segs.Clear();
+		Level->nodes.Clear();
+		return false;
+	}
+
 }
 
 
@@ -748,7 +767,7 @@ struct badseg
 };
 
 template<class segtype>
-void MapLoader::LoadSegs (MapData * map)
+bool MapLoader::LoadSegs (MapData * map)
 {
 	uint32_t numvertexes = Level->vertexes.Size();
 	TArray<uint8_t> vertchanged(numvertexes, true);
@@ -767,8 +786,7 @@ void MapLoader::LoadSegs (MapData * map)
 		Printf ("This map has no segs.\n");
 		Level->subsectors.Clear();
 		Level->nodes.Clear();
-		ForceNodeBuild = true;
+		return false;
-		return;
 	}
 
 	Level->segs.Alloc(numsegs);
@@ -919,8 +937,9 @@ void MapLoader::LoadSegs (MapData * map)
 		Level->segs.Clear();
 		Level->subsectors.Clear();
 		Level->nodes.Clear();
-		ForceNodeBuild = true;
+		return false;
 	}
+	return true;
 }
 
 
@@ -931,7 +950,7 @@ void MapLoader::LoadSegs (MapData * map)
 //===========================================================================
 
 template<class subsectortype, class segtype>
-void MapLoader::LoadSubsectors (MapData * map)
+bool MapLoader::LoadSubsectors (MapData * map)
 {
 	uint32_t maxseg = map->Size(ML_SEGS) / sizeof(segtype);
 
@@ -941,8 +960,7 @@ void MapLoader::LoadSubsectors (MapData * map)
 	{
 		Printf ("This map has an incomplete BSP tree.\n");
 		Level->nodes.Clear();
-		ForceNodeBuild = true;
+		return false;
-		return;
 	}
 
 	auto &subsectors = Level->subsectors;
@@ -963,8 +981,7 @@ void MapLoader::LoadSubsectors (MapData * map)
 			Printf ("Subsector %i is empty.\n", i);
 			Level->subsectors.Clear();
 			Level->nodes.Clear();
-			ForceNodeBuild = true;
+			return false;
-			return;
 		}
 
 		subsectors[i].numlines = subd.numsegs;
@@ -975,22 +992,22 @@ void MapLoader::LoadSubsectors (MapData * map)
 			Printf ("Subsector %d contains invalid segs %u-%u\n"
 				"The BSP will be rebuilt.\n", i, (unsigned)((size_t)subsectors[i].firstline),
 				(unsigned)((size_t)subsectors[i].firstline) + subsectors[i].numlines - 1);
-			ForceNodeBuild = true;
 			Level->nodes.Clear();
 			Level->subsectors.Clear();
-			break;
+			return false;
 		}
 		else if ((size_t)subsectors[i].firstline + subsectors[i].numlines > maxseg)
 		{
 			Printf ("Subsector %d contains invalid segs %u-%u\n"
 				"The BSP will be rebuilt.\n", i, maxseg,
 				(unsigned)((size_t)subsectors[i].firstline) + subsectors[i].numlines - 1);
-			ForceNodeBuild = true;
+
 			Level->nodes.Clear();
 			Level->subsectors.Clear();
-			break;
+			return false;
 		}
 	}
+	return true;
 }
 
 
@@ -1101,7 +1118,7 @@ void MapLoader::LoadSectors (MapData *map, FMissingTextureTracker &missingtex)
 //===========================================================================
 
 template<class nodetype, class subsectortype>
-void MapLoader::LoadNodes (MapData * map)
+bool MapLoader::LoadNodes (MapData * map)
 {
 	FMemLump	data;
 	int 		j;
@@ -1116,8 +1133,7 @@ void MapLoader::LoadNodes (MapData * map)
 
 	if ((numnodes == 0 && maxss != 1) || maxss == 0)
 	{
-		ForceNodeBuild = true;
+		return false;
-		return;
 	}
 	
 	auto &nodes = Level->nodes;
@@ -1145,9 +1161,8 @@ void MapLoader::LoadNodes (MapData * map)
 				{
 					Printf ("BSP node %d references invalid subsector %d.\n"
 						"The BSP will be rebuilt.\n", i, child);
-					ForceNodeBuild = true;
 					Level->nodes.Clear();
-					return;
+					return false;
 				}
 				no->children[j] = (uint8_t *)&Level->subsectors[child] + 1;
 			}
@@ -1155,18 +1170,16 @@ void MapLoader::LoadNodes (MapData * map)
 			{
 				Printf ("BSP node %d references invalid node %d.\n"
 					"The BSP will be rebuilt.\n", i, Index(((node_t *)no->children[j])));
-				ForceNodeBuild = true;
 				Level->nodes.Clear();
-				return;
+				return false;
 			}
 			else if (used[child])
 			{
 				Printf ("BSP node %d references node %d,\n"
 					"which is already used by node %d.\n"
 					"The BSP will be rebuilt.\n", i, child, used[child]-1);
-				ForceNodeBuild = true;
 				Level->nodes.Clear();
-				return;
+				return false;
 			}
 			else
 			{
@@ -1179,6 +1192,7 @@ void MapLoader::LoadNodes (MapData * map)
 			}
 		}
 	}
+	return true;
 }
 
 //===========================================================================
@@ -3039,22 +3053,11 @@ void MapLoader::LoadLevel(MapData *map, const char *lumpname, int position)
 			idcheck6 = MAKE_ID('X', 'G', 'L', '3');
 		}
 
+		bool NodesLoaded = false;
 		if (fr != nullptr && fr->isOpen()) fr->Read(&id, 4);
 		if (id != 0 && (id == idcheck || id == idcheck2 || id == idcheck3 || id == idcheck4 || id == idcheck5 || id == idcheck6))
 		{
-			try
+			NodesLoaded = LoadExtendedNodes(*fr, id);
-			{
-				LoadExtendedNodes(*fr, id);
-			}
-			catch (CRecoverableError &error)
-			{
-				Printf("Error loading nodes: %s\n", error.GetMessage());
-
-				ForceNodeBuild = true;
-				Level->subsectors.Clear();
-				Level->segs.Clear();
-				Level->nodes.Clear();
-			}
 		}
 		else if (!map->isText)	// regular nodes are not supported for text maps
 		{
@@ -3064,29 +3067,26 @@ void MapLoader::LoadLevel(MapData *map, const char *lumpname, int position)
 			{
 				if (!P_CheckV4Nodes(map))
 				{
-					LoadSubsectors<mapsubsector_t, mapseg_t>(map);
+					NodesLoaded = LoadSubsectors<mapsubsector_t, mapseg_t>(map) &&
-					if (!ForceNodeBuild) LoadNodes<mapnode_t, mapsubsector_t>(map);
+						LoadNodes<mapnode_t, mapsubsector_t>(map) &&
-					if (!ForceNodeBuild) LoadSegs<mapseg_t>(map);
+					 	LoadSegs<mapseg_t>(map);
 				}
 				else
 				{
-					LoadSubsectors<mapsubsector4_t, mapseg4_t>(map);
+					NodesLoaded = LoadSubsectors<mapsubsector4_t, mapseg4_t>(map) &&
-					if (!ForceNodeBuild) LoadNodes<mapnode4_t, mapsubsector4_t>(map);
+						LoadNodes<mapnode4_t, mapsubsector4_t>(map) &&
-					if (!ForceNodeBuild) LoadSegs<mapseg4_t>(map);
+						LoadSegs<mapseg4_t>(map);
 				}
 			}
-			else ForceNodeBuild = true;
 		}
-		else ForceNodeBuild = true;
 
 		// If loading the regular nodes failed try GL nodes before considering a rebuild
-		if (ForceNodeBuild)
+		if (!NodesLoaded)
 		{
 			if (LoadGLNodes(map))
-			{
-				ForceNodeBuild = false;
 				reloop = true;
-			}
+			else
+				ForceNodeBuild = true;
 		}
 	}
 	else reloop = true;
@@ -3116,9 +3116,7 @@ void MapLoader::LoadLevel(MapData *map, const char *lumpname, int position)
 			0, 0, 0, 0
 		};
 		leveldata.FindMapBounds();
-		// We need GL nodes if am_textured is on.
+
-		// In case a sync critical game mode is started, also build GL nodes to avoid problems
-		// if the different machines' am_textured setting differs.
 		FNodeBuilder builder(leveldata, polyspots, anchors, BuildGLNodes);
 		builder.Extract(*Level);
 		endTime = I_msTime();
@@ -3217,7 +3215,7 @@ void MapLoader::LoadLevel(MapData *map, const char *lumpname, int position)
 		node.len = (float)g_sqrt(fdx * fdx + fdy * fdy);
 	}
 
-	InitRenderInfo();				// create hardware independent renderer resources for the Level-> This must be done BEFORE the PolyObj Spawn!!!	
+	InitRenderInfo();				// create hardware independent renderer resources for the level. This must be done BEFORE the PolyObj Spawn!!!
 	P_ClearDynamic3DFloorData(Level);	// CreateVBO must be run on the plain 3D floor data.
 	screen->mVertexData->CreateVBO(Level->sectors);
 

src/maploader/maploader.h

@@ -234,10 +234,10 @@ public:
 
 	void FloodZones();
 	void LoadVertexes(MapData * map);
-	void LoadExtendedNodes(FileReader &dalump, uint32_t id);
+	bool LoadExtendedNodes(FileReader &dalump, uint32_t id);
-	template<class segtype> void LoadSegs(MapData * map);
+	template<class segtype> bool LoadSegs(MapData * map);
-	template<class subsectortype, class segtype> void LoadSubsectors(MapData * map);
+	template<class subsectortype, class segtype> bool LoadSubsectors(MapData * map);
-	template<class nodetype, class subsectortype> void LoadNodes(MapData * map);
+	template<class nodetype, class subsectortype> bool LoadNodes(MapData * map);
 	bool LoadGLNodes(MapData * map);
 	bool CheckCachedNodes(MapData *map);
 	bool CheckNodes(MapData * map, bool rebuilt, int buildtime);