Clear sm_nextmap so we don't get stuck in a loop (#1545)

* Pure C++ solution

* Pure SourcePawn solution

core/NextMap.cpp

@@ -109,6 +109,8 @@ bool NextMapManager::SetNextMap(const char *map)
 	return true;
 }
 
+static char g_nextMap[PLATFORM_MAX_PATH];
+
 #if SOURCE_ENGINE != SE_DARKMESSIAH
 void NextMapManager::HookChangeLevel(const char *map, const char *unknown)
 #else
@@ -122,8 +124,16 @@ void NextMapManager::HookChangeLevel(const char *map, const char *unknown, const
 	}
 
 	const char *newmap = sm_nextmap.GetString();
+	if (newmap[0] != '\0') {
+		ke::SafeStrcpy(g_nextMap, sizeof(g_nextMap), newmap);
+		newmap = g_nextMap;
 
-	if (newmap[0] == 0 || !g_HL2.IsMapValid(newmap))
+		// Clear the value so that if the map load fails later we don't get stuck in a loop.
+		// This might cause us to go off-cycle for a map, but nextmap will get us back on track.
+		sm_nextmap.SetValue("");
+	}
+
+	if (newmap[0] == '\0' || !g_HL2.IsMapValid(newmap))
 	{
 		RETURN_META(MRES_IGNORED);
 	}
@@ -142,6 +152,15 @@ void NextMapManager::HookChangeLevel(const char *map, const char *unknown, const
 
 void NextMapManager::OnSourceModLevelChange( const char *mapName )
 {
+	// If we were controlling the map change, reset sm_nextmap to be the name of the map we successfully changed to.
+	// This maintains an old API contract on the plugin side. We use the real map name even if it was different from
+	// the expected map name as if the expected map failed to load we let the game take over instead, but the nextmap
+	// plugin compares the sm_nextmap value to the current map to decide if it should advance the mapcycle.
+	if (g_nextMap[0] != '\0') {
+		sm_nextmap.SetValue(mapName);
+		g_nextMap[0] = '\0';
+	}
+
 	/* Skip the first 'mapchange' when the server starts up */
 	if (m_tempChangeInfo.startTime != 0)
 	{

plugins/nextmap.sp

@@ -88,7 +88,9 @@ public void OnPluginStart()
 	RegAdminCmd("sm_maphistory", Command_MapHistory, ADMFLAG_CHANGEMAP, "Shows the most recent maps played");
 	RegConsoleCmd("listmaps", Command_List);
 
-	// Set to the current map so OnMapStart() will know what to do
+	HookEventEx("server_changelevel_failed", OnChangelevelFailed, EventHookMode_Pre);
+
+	// Set to the current map so OnConfigsExecuted() will know what to do
 	char currentMap[PLATFORM_MAX_PATH];
 	GetCurrentMap(currentMap, sizeof(currentMap));
 	SetNextMap(currentMap);
@@ -110,10 +112,18 @@ public void OnConfigsExecuted()
 	// not in mapcyclefile. So we keep it set to the last expected nextmap. - ferret
 	if (strcmp(lastMap, currentMap) == 0)
 	{
-		FindAndSetNextMap();
+		FindAndSetNextMap(currentMap);
 	}
 }
 
+public void OnChangelevelFailed(Event event, const char[] name, bool dontBroadcast)
+{
+	char failedMap[PLATFORM_MAX_PATH];
+	event.GetString("levelname", failedMap, sizeof(failedMap));
+
+	FindAndSetNextMap(failedMap);
+}
+
 public Action Command_List(int client, int args) 
 {
 	PrintToConsole(client, "Map Cycle:");
@@ -129,7 +139,7 @@ public Action Command_List(int client, int args)
 	return Plugin_Handled;
 }
   
-void FindAndSetNextMap()
+void FindAndSetNextMap(char[] currentMap)
 {
 	if (ReadMapList(g_MapList, 
 			g_MapListSerial, 
@@ -149,14 +159,11 @@ void FindAndSetNextMap()
 	
 	if (g_MapPos == -1)
 	{
-		char current[PLATFORM_MAX_PATH];
-		GetCurrentMap(current, sizeof(current));
-
 		for (int i = 0; i < mapCount; i++)
 		{
 			g_MapList.GetString(i, mapName, sizeof(mapName));
 			if (FindMap(mapName, mapName, sizeof(mapName)) != FindMap_NotFound && 
-				strcmp(current, mapName, false) == 0)
+				strcmp(currentMap, mapName, false) == 0)
 			{
 				g_MapPos = i;
 				break;