From c3c08ad2cbdb70b60d56d8952bedf67039acdac4 Mon Sep 17 00:00:00 2001 From: harryob Date: Sat, 9 Sep 2023 11:57:37 +0100 Subject: [PATCH] icon2html optimizations (#4287) thanks kylerace on https://github.com/tgstation/tgstation/pull/67429 thanks lemoninthedark on https://github.com/tgstation/tgstation/pull/58607 :cl: kylerace, lemoninthedark, harry refactor: optimized icon-in-chat code fix: weird icons on the smoothed walls when examining or alt+clicking /:cl: --- code/__DEFINES/equipment.dm | 2 + code/__HELPERS/icons.dm | 146 +++++++++++++++--- code/game/turfs/walls/wall_icon.dm | 2 + code/modules/asset_cache/asset_cache_item.dm | 13 +- .../asset_cache/transports/asset_transport.dm | 20 ++- 5 files changed, 154 insertions(+), 29 deletions(-) diff --git a/code/__DEFINES/equipment.dm b/code/__DEFINES/equipment.dm index ccfc9e4773da..210aee450406 100644 --- a/code/__DEFINES/equipment.dm +++ b/code/__DEFINES/equipment.dm @@ -42,6 +42,8 @@ #define ATOM_DECORATED (1<<16) /// Whether or not the object uses hearing #define USES_HEARING (1<<17) +/// Should we use the initial icon for display? Mostly used by overlay only objects +#define HTML_USE_INITAL_ICON (1<<18) //========================================================================================== diff --git a/code/__HELPERS/icons.dm b/code/__HELPERS/icons.dm index b2ac00098c69..99f621919771 100644 --- a/code/__HELPERS/icons.dm +++ b/code/__HELPERS/icons.dm @@ -571,12 +571,100 @@ world I.color = color I.flick_overlay(src, time) -/proc/icon2html(thing, target, icon_state, dir = SOUTH, frame = 1, moving = FALSE, sourceonly = FALSE) +/// generates a filename for a given asset. +/// like generate_asset_name(), except returns the rsc reference and the rsc file hash as well as the asset name (sans extension) +/// used so that certain asset files dont have to be hashed twice +/proc/generate_and_hash_rsc_file(file, dmi_file_path) + var/rsc_ref = fcopy_rsc(file) + var/hash + //if we have a valid dmi file path we can trust md5'ing the rsc file because we know it doesnt have the bug described in http://www.byond.com/forum/post/2611357 + if(dmi_file_path) + hash = md5(rsc_ref) + else //otherwise, we need to do the expensive fcopy() workaround + hash = md5asfile(rsc_ref) + + return list(rsc_ref, hash, "asset.[hash]") + +///given a text string, returns whether it is a valid dmi icons folder path +/proc/is_valid_dmi_file(icon_path) + if(!istext(icon_path) || !length(icon_path)) + return FALSE + + var/is_in_icon_folder = findtextEx(icon_path, "icons/") + var/is_dmi_file = findtextEx(icon_path, ".dmi") + + if(is_in_icon_folder && is_dmi_file) + return TRUE + return FALSE + +/// given an icon object, dmi file path, or atom/image/mutable_appearance, attempts to find and return an associated dmi file path. +/// a weird quirk about dm is that /icon objects represent both compile-time or dynamic icons in the rsc, +/// but stringifying rsc references returns a dmi file path +/// ONLY if that icon represents a completely unchanged dmi file from when the game was compiled. +/// so if the given object is associated with an icon that was in the rsc when the game was compiled, this returns a path. otherwise it returns "" +/proc/get_icon_dmi_path(icon/icon) + /// the dmi file path we attempt to return if the given object argument is associated with a stringifiable icon + /// if successful, this looks like "icons/path/to/dmi_file.dmi" + var/icon_path = "" + + if(isatom(icon) || istype(icon, /image) || istype(icon, /mutable_appearance)) + var/atom/atom_icon = icon + icon = atom_icon.icon + //atom icons compiled in from 'icons/path/to/dmi_file.dmi' are weird and not really icon objects that you generate with icon(). + //if theyre unchanged dmi's then they're stringifiable to "icons/path/to/dmi_file.dmi" + + if(isicon(icon) && isfile(icon)) + //icons compiled in from 'icons/path/to/dmi_file.dmi' at compile time are weird and arent really /icon objects, + ///but they pass both isicon() and isfile() checks. theyre the easiest case since stringifying them gives us the path we want + var/icon_ref = text_ref(icon) + var/locate_icon_string = "[locate(icon_ref)]" + + icon_path = locate_icon_string + + else if(isicon(icon) && "[icon]" == "/icon") + // icon objects generated from icon() at runtime are icons, but they ARENT files themselves, they represent icon files. + // if the files they represent are compile time dmi files in the rsc, then + // the rsc reference returned by fcopy_rsc() will be stringifiable to "icons/path/to/dmi_file.dmi" + var/rsc_ref = fcopy_rsc(icon) + + var/icon_ref = text_ref(rsc_ref) + + var/icon_path_string = "[locate(icon_ref)]" + + icon_path = icon_path_string + + else if(istext(icon)) + var/rsc_ref = fcopy_rsc(icon) + //if its the text path of an existing dmi file, the rsc reference returned by fcopy_rsc() will be stringifiable to a dmi path + + var/rsc_ref_ref = text_ref(rsc_ref) + var/rsc_ref_string = "[locate(rsc_ref_ref)]" + + icon_path = rsc_ref_string + + if(is_valid_dmi_file(icon_path)) + return icon_path + + return FALSE + +/** + * generate an asset for the given icon or the icon of the given appearance for [thing], and send it to any clients in target. + * Arguments: + * * thing - either a /icon object, or an object that has an appearance (atom, image, mutable_appearance). + * * target - either a reference to or a list of references to /client's or mobs with clients + * * icon_state - string to force a particular icon_state for the icon to be used + * * dir - dir number to force a particular direction for the icon to be used + * * frame - what frame of the icon_state's animation for the icon being used + * * moving - whether or not to use a moving state for the given icon + * * sourceonly - if TRUE, only generate the asset and send back the asset url, instead of tags that display the icon to players + * * extra_clases - string of extra css classes to use when returning the icon string + */ +/proc/icon2html(atom/thing, client/target, icon_state, dir = SOUTH, frame = 1, moving = FALSE, sourceonly = FALSE, extra_classes = null) if (!thing) return var/key - var/icon/I = thing + var/icon/icon2collapse = thing if (!target) return @@ -588,46 +676,64 @@ world targets = list(target) else targets = target - if (!targets.len) - return - if (!isicon(I)) + if(!length(targets)) + return + + //check if the given object is associated with a dmi file in the icons folder. if it is then we dont need to do a lot of work + //for asset generation to get around byond limitations + var/icon_path = get_icon_dmi_path(thing) + + if (!isicon(icon2collapse)) if (isfile(thing)) //special snowflake - var/name = sanitize_filename("[generate_asset_name(thing)].png") + var/name = SANITIZE_FILENAME("[generate_asset_name(thing)].png") if (!SSassets.cache[name]) SSassets.transport.register_asset(name, thing) for (var/thing2 in targets) SSassets.transport.send_assets(thing2, name) if(sourceonly) return SSassets.transport.get_asset_url(name) - return "" - var/atom/A = thing + return "" + + //its either an atom, image, or mutable_appearance, we want its icon var + icon2collapse = thing.icon - I = A.icon if (isnull(icon_state)) - icon_state = A.icon_state - if (!icon_state) - icon_state = initial(A.icon_state) + icon_state = thing.icon_state + //Despite casting to atom, this code path supports mutable appearances, so let's be nice to them + if(isnull(icon_state) || (isatom(thing) && thing.flags_atom & HTML_USE_INITAL_ICON)) + icon_state = initial(thing.icon_state) if (isnull(dir)) - dir = initial(A.dir) + dir = initial(thing.dir) if (isnull(dir)) - dir = A.dir + dir = thing.dir + + if (ishuman(thing)) // Shitty workaround for a BYOND issue. + var/icon/temp = icon2collapse + icon2collapse = icon() + icon2collapse.Insert(temp, dir = SOUTH) + dir = SOUTH else if (isnull(dir)) dir = SOUTH if (isnull(icon_state)) icon_state = "" - I = icon(I, icon_state, dir, frame, moving) + icon2collapse = icon(icon2collapse, icon_state, dir, frame, moving) + + var/list/name_and_ref = generate_and_hash_rsc_file(icon2collapse, icon_path)//pretend that tuples exist + + var/rsc_ref = name_and_ref[1] //weird object thats not even readable to the debugger, represents a reference to the icons rsc entry + var/file_hash = name_and_ref[2] + key = "[name_and_ref[3]].png" - key = "[generate_asset_name(I)].png" if(!SSassets.cache[key]) - SSassets.transport.register_asset(key, I) - for (var/thing2 in targets) - SSassets.transport.send_assets(thing2, key) + SSassets.transport.register_asset(key, rsc_ref, file_hash, icon_path) + for (var/client_target in targets) + SSassets.transport.send_assets(client_target, key) if(sourceonly) return SSassets.transport.get_asset_url(key) - return "" + return "" //Costlier version of icon2html() that uses getFlatIcon() to account for overlays, underlays, etc. Use with extreme moderation, ESPECIALLY on mobs. /proc/costly_icon2html(thing, target, sourceonly = FALSE) diff --git a/code/game/turfs/walls/wall_icon.dm b/code/game/turfs/walls/wall_icon.dm index 8b8ee00bcf53..9e47612964c4 100644 --- a/code/game/turfs/walls/wall_icon.dm +++ b/code/game/turfs/walls/wall_icon.dm @@ -19,6 +19,8 @@ icon_state = "blank" var/image/I + flags_atom |= HTML_USE_INITAL_ICON + if(!density) I = image(icon, "[walltype]fwall_open") overlays += I diff --git a/code/modules/asset_cache/asset_cache_item.dm b/code/modules/asset_cache/asset_cache_item.dm index 72d976bf11f1..ade73f79c7ca 100644 --- a/code/modules/asset_cache/asset_cache_item.dm +++ b/code/modules/asset_cache/asset_cache_item.dm @@ -21,11 +21,20 @@ /// TRUE for keeping local asset names when browse_rsc backend is used var/keep_local_name = FALSE -/datum/asset_cache_item/New(name, file) +///pass in a valid file_hash if you have one to save it from needing to do it again. +///pass in a valid dmi file path string e.g. "icons/path/to/dmi_file.dmi" to make generating the hash less expensive +/datum/asset_cache_item/New(name, file, file_hash, dmi_file_path) if (!isfile(file)) file = fcopy_rsc(file) - hash = md5asfile(file) //icons sent to the rsc sometimes md5 incorrectly + hash = file_hash + + //the given file is directly from a dmi file and is thus in the rsc already, we know that its file_hash will be correct + if(!hash) + if(dmi_file_path) + hash = md5(file) + else + hash = md5asfile(file) //icons sent to the rsc md5 incorrectly when theyre given incorrect data if (!hash) CRASH("invalid asset sent to asset cache") src.name = name diff --git a/code/modules/asset_cache/transports/asset_transport.dm b/code/modules/asset_cache/transports/asset_transport.dm index a8d45e4cd21f..2e165229f19b 100644 --- a/code/modules/asset_cache/transports/asset_transport.dm +++ b/code/modules/asset_cache/transports/asset_transport.dm @@ -22,15 +22,21 @@ addtimer(CALLBACK(src, PROC_REF(send_assets_slow), C, preload), 1 SECONDS) -/// Register a browser asset with the asset cache system -/// asset_name - the identifier of the asset -/// asset - the actual asset file (or an asset_cache_item datum) -/// returns a /datum/asset_cache_item. -/// mutiple calls to register the same asset under the same asset_name return the same datum -/datum/asset_transport/proc/register_asset(asset_name, asset) +/** + * Register a browser asset with the asset cache system. + * returns a /datum/asset_cache_item. + * mutiple calls to register the same asset under the same asset_name return the same datum. + * + * Arguments: + * * asset_name - the identifier of the asset. + * * asset - the actual asset file (or an asset_cache_item datum). + * * file_hash - optional, a hash of the contents of the asset files contents. used so asset_cache_item doesnt have to hash it again + * * dmi_file_path - optional, means that the given asset is from the rsc and thus we dont need to do some expensive operations + */ +/datum/asset_transport/proc/register_asset(asset_name, asset, file_hash, dmi_file_path) var/datum/asset_cache_item/ACI = asset if (!istype(ACI)) - ACI = new(asset_name, asset) + ACI = new(asset_name, asset, file_hash, dmi_file_path) if (!ACI || !ACI.hash) CRASH("ERROR: Invalid asset: [asset_name]:[asset]:[ACI]") if (SSassets.cache[asset_name])