Replace coloredText() to setColor().
Details
Diff Detail
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Event Timeline
Before we change the name of everything we have, I would first like to get a third opinion, for instance the one of @bb.
We need three functions: setColor, setFont, setColorFont is one proposal
From https://code.wildfiregames.com/P99#740:
we could also use coloredText(), fontText(), guiTags().
or maybe the third one could be coloredFontText()
changing a name is fine as long as it results in a better name, I don't (yet) see why the proposed name is any better than the previous one.
We need three functions: setColor, setFont, setColorFont is one proposal
From https://code.wildfiregames.com/P99#740:
we could also use coloredText(), fontText(), guiTags().
So what is the benefit of having 3 function, when just one has all the functionality of both others and more? ok one doesn't have to create an object and unpack it, so a tiny performance gain, but is that worth having 3 functions instead of 1?
or maybe the third one could be coloredFontText()
Nah, keep it general, who knows if/when we split the size from the font, or create subfonts (Bolt italic etc.) then those need to find a way in aswell (yes currently that are all different fonts, but it can be changed ofc). Those things also run into trouble when having separate functions for every setting, since every setting will come with an own function, and when there are functions for combinations also, we get an exponential increase...
Aiming to only use one guiTags function wouldn't come with any cost once the arguments are globals everywhere (and gui tags should be split from the code anyway.)
So the other two functions would only be useful as a transition if the final state is going to be having 0 calls to them.
Okay, that seems like a good final vision (and I agree on it), but then I utterly don't understand why we then need a function rename like this. One could:
- Implement the new function
- Swap the calls to using the new function (can/should be multiple commits!!)
- Remove the old function
So I don't see the need for a rename in that process.
Just my two cents here but why is not 'setTextColor' ? 'coloredText' doesn't make sense as to what it does to the text, but 'setColor' seems a bit too generic like it would apply to everything. Which in this case could be something like 'setObjectColor'
Yea thats true, its just setColor as name because its not to long and in our mainly purposes in code used for textcolor setting for rather short but often used tag scenario, which is cleaning up this scenario by an easy function. :)
I am also a fan now of this general used setFunctions, because they are very easy to read and to work also easy with. So setTags(text, tags) would also be nice to have. setColorFont(text, color, font) can be commited to complete this functions, but at the moment not really needed, so its more a completion here. setFont(text, font) same easiness in usage and term here.
all tags should become globals and use the new function from D1167
binaries/data/mods/public/gui/lobby/lobby.js | ||
---|---|---|
33 | var g_GameStateTags = { "init": { "color": "foo" }, }; | |
48 | var g_PlayerStatuses = { "available": { "tags": { "color": "foo" }, "label": translate("Online") } }; | |
75 | these ought to become tag objects |