Add helper function to apply arbitrary GUI tags (such as font and color).
Description
Details
Event TimelineComment Actions Hi, I wanted to add a comment about this function, even though I'm not part of the project in any way. I hope I didn't step on any toes in the process. "rosievers raised a concern" sound like a harsh action for an outsider with a fresh account.
Comment Actions My fail, I have added the helper variable result (since overwriting input variables should only be done when writing the output value is intended so that the reader sees on first sight that it is impossible that this call can change a reference outside of the function (even though people should know how JS works)), but blatantly incorrectly. Index: binaries/data/mods/mod/gui/common/guitags.js =================================================================== --- binaries/data/mods/mod/gui/common/guitags.js (revision 20697) +++ binaries/data/mods/mod/gui/common/guitags.js (working copy) @@ -12,12 +12,12 @@ function coloredText(text, color) * @param {string} string - String to apply tags to. * @param {object} tags - Object containing the tags, for instance { "color": "white" } or { "font": "sans-13" }. */ function setStringTags(text, tags) { - let result = ""; + let result = text; for (let tag in tags) - result = '[' + tag + '="' + tags[tag] + '"]' + text + '[/' + tag + ']'; + result = '[' + tag + '="' + tags[tag] + '"]' + result + '[/' + tag + ']'; return result; } Comment Actions Looks better this way, thank you. Do you think coloredText should be defined via setStringTags to avoid code duplication? return setStringTags(text, {"color" : color })Comment Actions Thanks for the review. Yes I noticed that too (see review), but it seems a little hurtful to create an object and deconstruct it right after (the function can be called very often, on each onTick I believe), | ||||||||||||||