Agree it is nice to have them colorized.
Thank you for picking up nani's report and writing a patch for it!
On the feature description of this diff:
It wasn't obvious to me whether it was an oversight or intended.
I remember two aspects while working with that code:
- The status chat messages are not fully consistently formatted. For example: Ready message is not bold, but Join messages are
- One of the differential revision iterations of D2483 had that ClientReady message worng. (It had shown "(playerID) is ready").
So it seems like it indeed was an oversight and not intended to drop the color.
Style consistency for those who opt into that:
- No global keyword (not done elsewhere except when its not possible to avoid it, or in music.js which has always had unique style, grep -R global | grep js | sort).
- Not that newline.
I didn't apply the patch but I assume it is easy enough to test before committing.
The mistake could have been systematic and one should also not only consider this gamesetup commit, but the current code base without prejudice from possible historical debt.
- One consistency that had been there since many years(?) is that the Join message is not colorized, but the Leave message is.
That must be due to assigning the player from -1 to an empty slot after the Join chat message.
That could be changed by broadcasting the Join in the place after the empty-slot assign routine had been performed to make events explicit, ensuring the order of events (instead of intransparently and fragily changing the order of subscription insertion).
- Related chat formatting inconsistency was mentioned in the memories above ("ready message is not bold, but Join messages are"). I didn't decide about that.
(I didn't check further inconsistencies, they could be found by comparing all chat message types, perhaps also in the session. You can chose the scope and what you commit as long as its not bugged and reasonable. Also I offer the whatever card.)