Validate map templates uploaded by admins (#39674)

* Move the preloader datum to its own file * Prettify some of the map loader docs * Use src rather than usr in map template verbs * Cache parsed templates between upload and first use * Validate map templates uploaded by admins before use * Add href token to validation report links
2025-02-26 09:04:50 +00:00 · 2018-08-16 01:09:21 -07:00
parent 8b9b311dc1
commit 05335f1c86
6 changed files with 191 additions and 70 deletions
--- a/code/modules/admin/verbs/map_template_loadverb.dm
+++ b/code/modules/admin/verbs/map_template_loadverb.dm
@@ -4,7 +4,7 @@

 	var/datum/map_template/template

-	var/map = input(usr, "Choose a Map Template to place at your CURRENT LOCATION","Place Map Template") as null|anything in SSmapping.map_templates
+	var/map = input(src, "Choose a Map Template to place at your CURRENT LOCATION","Place Map Template") as null|anything in SSmapping.map_templates
 	if(!map)
 		return
 	template = SSmapping.map_templates[map]
@@ -18,35 +18,50 @@
 		var/image/item = image('icons/turf/overlays.dmi',S,"greenOverlay")
 		item.plane = ABOVE_LIGHTING_PLANE
 		preview += item
-	usr.client.images += preview
-	if(alert(usr,"Confirm location.","Template Confirm","Yes","No") == "Yes")
+	images += preview
+	if(alert(src,"Confirm location.","Template Confirm","Yes","No") == "Yes")
 		if(template.load(T, centered = TRUE))
-			message_admins("<span class='adminnotice'>[key_name_admin(usr)] has placed a map template ([template.name]) at [ADMIN_COORDJMP(T)]</span>")
+			message_admins("<span class='adminnotice'>[key_name_admin(src)] has placed a map template ([template.name]) at [ADMIN_COORDJMP(T)]</span>")
 		else
-			to_chat(usr, "Failed to place map")
-	usr.client.images -= preview
+			to_chat(src, "Failed to place map")
+	images -= preview

 /client/proc/map_template_upload()
 	set category = "Debug"
 	set name = "Map Template - Upload"

-	var/map = input(usr, "Choose a Map Template to upload to template storage","Upload Map Template") as null|file
+	var/map = input(src, "Choose a Map Template to upload to template storage","Upload Map Template") as null|file
 	if(!map)
 		return
 	if(copytext("[map]",-4) != ".dmm")
-		to_chat(usr, "Bad map file: [map]")
+		to_chat(src, "<span class='warning'>Filename must end in '.dmm': [map]</span>")
 		return
 	var/datum/map_template/M
-	switch(alert(usr, "What kind of map is this?", "Map type", "Normal", "Shuttle", "Cancel"))
+	switch(alert(src, "What kind of map is this?", "Map type", "Normal", "Shuttle", "Cancel"))
 		if("Normal")
-			M = new /datum/map_template(map, "[map]")
+			M = new /datum/map_template(map, "[map]", TRUE)
 		if("Shuttle")
-			M = new /datum/map_template/shuttle(map, "[map]")
+			M = new /datum/map_template/shuttle(map, "[map]", TRUE)
 		else
 			return
-	if(M.preload_size(map))
-		to_chat(usr, "Map template '[map]' ready to place ([M.width]x[M.height])")
-		SSmapping.map_templates[M.name] = M
-		message_admins("<span class='adminnotice'>[key_name_admin(usr)] has uploaded a map template ([map])</span>")
-	else
-		to_chat(usr, "Map template '[map]' failed to load properly")
+	if(!M.cached_map)
+		to_chat(src, "<span class='warning'>Map template '[map]' failed to parse properly.</span>")
+		return
+
+	var/datum/map_report/report = M.cached_map.check_for_errors()
+	var/report_link
+	if(report)
+		report.show_to(src)
+		report_link = " - <a href='?src=[REF(report)];[HrefToken(TRUE)];show=1'>validation report</a>"
+		to_chat(src, "<span class='warning'>Map template '[map]' <a href='?src=[REF(report)];[HrefToken()];show=1'>failed validation</a>.</span>")
+		if(report.loadable)
+			var/response = alert(src, "The map failed validation, would you like to load it anyways?", "Map Errors", "Cancel", "Upload Anyways")
+			if(response != "Upload Anyways")
+				return
+		else
+			alert(src, "The map failed validation and cannot be loaded.", "Map Errors", "Oh Darn")
+			return
+
+	SSmapping.map_templates[M.name] = M
+	message_admins("<span class='adminnotice'>[key_name_admin(src)] has uploaded a map template '[map]' ([M.width]x[M.height])[report_link].</span>")
+	to_chat(src, "<span class='notice'>Map template '[map]' ready to place ([M.width]x[M.height])</span>")
--- a/code/modules/mapping/map_template.dm
+++ b/code/modules/mapping/map_template.dm
@@ -4,21 +4,25 @@
 	var/height = 0
 	var/mappath = null
 	var/loaded = 0 // Times loaded this round
+	var/datum/parsed_map/cached_map
+	var/keep_cached_map = FALSE

-/datum/map_template/New(path = null, rename = null)
+/datum/map_template/New(path = null, rename = null, cache = FALSE)
 	if(path)
 		mappath = path
 	if(mappath)
-		preload_size(mappath)
+		preload_size(mappath, cache)
 	if(rename)
 		name = rename

-/datum/map_template/proc/preload_size(path)
-	var/datum/parsed_map/parsed = load_map(file(path), 1, 1, 1, cropMap=FALSE, measureOnly=TRUE)
+/datum/map_template/proc/preload_size(path, cache = FALSE)
+	var/datum/parsed_map/parsed = new(file(path))
 	var/bounds = parsed?.bounds
 	if(bounds)
 		width = bounds[MAP_MAXX] // Assumes all templates are rectangular, have a single Z level, and begin at 1,1,1
 		height = bounds[MAP_MAXY]
+		if(cache)
+			cached_map = parsed
 	return bounds

 /datum/parsed_map/proc/initTemplateBounds()
@@ -77,8 +81,13 @@
 	if(T.y+height > world.maxy)
 		return

-	var/datum/parsed_map/parsed = load_map(file(mappath), T.x, T.y, T.z, cropMap=TRUE, no_changeturf=(SSatoms.initialized == INITIALIZATION_INSSATOMS), placeOnTop=TRUE)
-	var/list/bounds = parsed?.bounds
+	// Accept cached maps, but don't save them automatically - we don't want
+	// ruins clogging up memory for the whole round.
+	var/datum/parsed_map/parsed = cached_map || new(file(mappath))
+	cached_map = keep_cached_map ? parsed : null
+	if(!parsed.load(T.x, T.y, T.z, cropMap=TRUE, no_changeturf=(SSatoms.initialized == INITIALIZATION_INSSATOMS), placeOnTop=TRUE))
+		return
+	var/list/bounds = parsed.bounds
 	if(!bounds)
 		return

--- a/code/modules/mapping/preloader.dm
+++ b/code/modules/mapping/preloader.dm
@@ -0,0 +1,31 @@
+// global datum that will preload variables on atoms instanciation
+GLOBAL_VAR_INIT(use_preloader, FALSE)
+GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new)
+
+/// Preloader datum
+/datum/map_preloader
+	parent_type = /datum
+	var/list/attributes
+	var/target_path
+
+/datum/map_preloader/proc/setup(list/the_attributes, path)
+	if(the_attributes.len)
+		GLOB.use_preloader = TRUE
+		attributes = the_attributes
+		target_path = path
+
+/datum/map_preloader/proc/load(atom/what)
+	GLOB.use_preloader = FALSE
+	for(var/attribute in attributes)
+		var/value = attributes[attribute]
+		if(islist(value))
+			value = deepCopyList(value)
+		what.vars[attribute] = value
+
+/area/template_noop
+	name = "Area Passthrough"
+
+/turf/template_noop
+	name = "Turf Passthrough"
+	icon_state = "noop"
+	bullet_bounce_sound = null
--- a/code/modules/mapping/reader.dm
+++ b/code/modules/mapping/reader.dm
@@ -2,9 +2,6 @@
 //SS13 Optimized Map loader
 //////////////////////////////////////////////////////////////
 #define SPACE_KEY "space"
-//global datum that will preload variables on atoms instanciation
-GLOBAL_VAR_INIT(use_preloader, FALSE)
-GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new)

 /datum/grid_set
 	var/xcrd
@@ -19,7 +16,6 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new)
 	var/list/gridSets = list()

 	var/list/modelCache
-	var/list/bad_paths

 	/// Unoffset bounds. Null on parse failure.
 	var/list/parsed_bounds
@@ -38,13 +34,13 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new)

 /// Shortcut function to parse a map and apply it to the world.
 ///
-/// - dmm_file: A .dmm file to load (Required).
-/// - x_offset, y_offset, z_offset: Positions representign where to load the map (Optional).
-/// - cropMap: When true, the map will be cropped to fit the existing world dimensions (Optional).
-/// - measureOnly: When true, no changes will be made to the world (Optional).
-/// - no_changeturf: When true, turf/AfterChange won't be called on loaded turfs
-/// - x_lower, x_upper, y_lower, y_upper: Coordinates (relative to the map) to crop to (Optional).
-/// - placeOnTop: Whether to use turf/PlaceOnTop rather than turf/ChangeTurf (Optional).
+/// - `dmm_file`: A .dmm file to load (Required).
+/// - `x_offset`, `y_offset`, `z_offset`: Positions representign where to load the map (Optional).
+/// - `cropMap`: When true, the map will be cropped to fit the existing world dimensions (Optional).
+/// - `measureOnly`: When true, no changes will be made to the world (Optional).
+/// - `no_changeturf`: When true, [turf/AfterChange] won't be called on loaded turfs
+/// - `x_lower`, `x_upper`, `y_lower`, `y_upper`: Coordinates (relative to the map) to crop to (Optional).
+/// - `placeOnTop`: Whether to use [turf/PlaceOnTop] rather than [turf/ChangeTurf] (Optional).
 /proc/load_map(dmm_file as file, x_offset as num, y_offset as num, z_offset as num, cropMap as num, measureOnly as num, no_changeturf as num, x_lower = -INFINITY as num, x_upper = INFINITY as num, y_lower = -INFINITY as num, y_upper = INFINITY as num, placeOnTop = FALSE as num)
 	var/datum/parsed_map/parsed = new(dmm_file, x_lower, x_upper, y_lower, y_upper, measureOnly)
 	if(parsed.bounds && !measureOnly)
@@ -133,7 +129,7 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new)
 		bounds = null
 	parsed_bounds = bounds

-/// Load the parsed map into the world. See /proc/load_map for arguments.
+/// Load the parsed map into the world. See [/proc/load_map] for arguments.
 /datum/parsed_map/proc/load(x_offset, y_offset, z_offset, cropMap, no_changeturf, x_lower, x_upper, y_lower, y_upper, placeOnTop)
 	//How I wish for RAII
 	Master.StartLoadingMap()
@@ -218,8 +214,8 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new)

 	return TRUE

-/datum/parsed_map/proc/build_cache(no_changeturf)
-	if(modelCache)
+/datum/parsed_map/proc/build_cache(no_changeturf, bad_paths=null)
+	if(modelCache && !bad_paths)
 		return modelCache
 	. = modelCache = list()
 	var/list/grid_models = src.grid_models
@@ -246,8 +242,9 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new)
 			var/atom_def = text2path(path_text) //path definition, e.g /obj/foo/bar
 			old_position = dpos + 1

-			if(!atom_def) // Skip the item if the path does not exist.  Fix your crap, mappers!
-				LAZYADD(bad_paths, path_text)
+			if(!ispath(atom_def, /atom)) // Skip the item if the path does not exist.  Fix your crap, mappers!
+				if(bad_paths)
+					LAZYOR(bad_paths[path_text], model_key)
 				continue
 			members.Add(atom_def)

@@ -467,34 +464,3 @@ GLOBAL_DATUM_INIT(_preloader, /datum/map_preloader, new)
 /datum/parsed_map/Destroy()
 	..()
 	return QDEL_HINT_HARDDEL_NOW
-
-//////////////////
-//Preloader datum
-//////////////////
-
-/datum/map_preloader
-	parent_type = /datum
-	var/list/attributes
-	var/target_path
-
-/datum/map_preloader/proc/setup(list/the_attributes, path)
-	if(the_attributes.len)
-		GLOB.use_preloader = TRUE
-		attributes = the_attributes
-		target_path = path
-
-/datum/map_preloader/proc/load(atom/what)
-	GLOB.use_preloader = FALSE
-	for(var/attribute in attributes)
-		var/value = attributes[attribute]
-		if(islist(value))
-			value = deepCopyList(value)
-		what.vars[attribute] = value
-
-/area/template_noop
-	name = "Area Passthrough"
-
-/turf/template_noop
-	name = "Turf Passthrough"
-	icon_state = "noop"
-	bullet_bounce_sound = null
--- a/code/modules/mapping/verify.dm
+++ b/code/modules/mapping/verify.dm
@@ -0,0 +1,98 @@
+/// An error report generated by [parsed_map/check_for_errors].
+/datum/map_report
+	var/original_path
+	var/list/bad_paths = list()
+	var/list/bad_keys = list()
+	/// Whether this map can be loaded safely despite the errors.
+	var/loadable = TRUE
+	var/crashed = TRUE
+
+	var/static/tag_number = 0
+
+/datum/map_report/New(datum/parsed_map/map)
+	original_path = map.original_path || "Untitled"
+
+/// Show a rendered version of this report to a client.
+/datum/map_report/proc/show_to(client/C)
+	var/list/html = list()
+	html += "<p>Report for map file <tt>[original_path]</tt></p>"
+	if(crashed)
+		html += "<p><b>Validation crashed</b>: check the runtime logs.</p>"
+	if(!loadable)
+		html += "<p><b>Not loadable</b>: some tiles are missing their turfs or areas.</p>"
+
+	if(bad_paths.len)
+		html += "<p>Bad paths: <ol>"
+		for(var/path in bad_paths)
+			var/list/keys = bad_paths[path]
+			html += "<li><tt>[path]</tt>: used in ([keys.len]): <tt>[keys.Join("</tt>, <tt>")]</tt>"
+		html += "</ol></p>"
+
+	if(bad_keys.len)
+		html += "<p>Bad keys: <ul>"
+		for(var/key in bad_keys)
+			var/list/messages = bad_keys[key]
+			html += "<li><tt>[key]</tt>"
+			if(messages.len == 1)
+				html += ": [bad_keys[key][1]]"
+			else
+				html += "<ul><li>[messages.Join("</li><li>")]</li></ul>"
+			html += "</li>"
+		html += "</ul></p>"
+	C << browse(html.Join(), "window=[tag];size=600x400")
+
+/datum/map_report/Topic(href, href_list)
+	. = ..()
+	if(. || !check_rights(R_ADMIN, FALSE) || !usr.client.holder.CheckAdminHref(href, href_list))
+		return
+
+	if (href_list["show"])
+		show_to(usr)
+
+
+/// Check a parsed but not yet loaded map for errors.
+///
+/// Returns a [/datum/map_report] if there are errors or `FALSE` otherwise.
+/datum/parsed_map/proc/check_for_errors()
+	var/datum/map_report/report = new(src)
+	. = report
+
+	// build_cache will check bad paths for us
+	var/list/modelCache = build_cache(TRUE, report.bad_paths)
+
+	for(var/path in report.bad_paths)
+		if(copytext(path, 1, 7) == "/turf/" || copytext(path, 1, 7) == "/area/")
+			report.loadable = FALSE
+
+	// check for tiles with the wrong number of turfs or areas
+	for(var/key in modelCache)
+		if(key == SPACE_KEY)
+			continue
+		var/model = modelCache[key]
+		var/list/members = model[1]
+
+		var/turfs = 0
+		var/areas = 0
+		for(var/i in 1 to members.len)
+			var/atom/path = members[i]
+
+			turfs += ispath(path, /turf)
+			areas += ispath(path, /area)
+
+		if(turfs == 0)
+			report.loadable = FALSE
+			LAZYADD(report.bad_keys[key], "no turf")
+		else if(turfs > 1)
+			LAZYADD(report.bad_keys[key], "[turfs] stacked turfs")
+
+		if(areas != 1)
+			report.loadable = FALSE
+			LAZYADD(report.bad_keys[key], "[areas] areas instead of 1")
+
+	// return the report
+	if(report.bad_paths.len || report.bad_keys.len || !report.loadable)
+		// keep the report around so it can be referenced later
+		report.tag = "mapreport_[++report.tag_number]"
+		report.crashed = FALSE
+	else
+		return FALSE
--- a/tgstation.dme
+++ b/tgstation.dme
@@ -1745,8 +1745,10 @@
 #include "code\modules\lighting\lighting_turf.dm"
 #include "code\modules\mapping\map_template.dm"
 #include "code\modules\mapping\mapping_helpers.dm"
+#include "code\modules\mapping\preloader.dm"
 #include "code\modules\mapping\reader.dm"
 #include "code\modules\mapping\ruins.dm"
+#include "code\modules\mapping\verify.dm"
 #include "code\modules\mapping\space_management\space_level.dm"
 #include "code\modules\mapping\space_management\space_reservation.dm"
 #include "code\modules\mapping\space_management\space_transition.dm"