Page MenuHomeWildfire Games

Colorize username in chat when is ready and leaves match
Needs ReviewPublic

Authored by Angen on Mon, Mar 23, 10:14 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

When user leaves lobby he is colorized also when he clicks is ready button in a23b.
After rP23374 it is not case anymore.

@elexis since it was your patch, do you agree we should revert to that behaviour or are you strongly against ?

Test Plan

Agree it is nice to have them colorized.

Event Timeline

Angen created this revision.Mon, Mar 23, 10:14 PM
Owners added a subscriber: Restricted Owners Package.Mon, Mar 23, 10:15 PM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1886/display/redirect

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.

Correctness:
I didn't apply the patch but I assume it is easy enough to test before committing.

Completeness:
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.

  1. 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).

  1. 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.)

Angen added a comment.Wed, Mar 25, 9:47 PM

I wanted to avoid copying the same function to another two classes.
That line was not intentional. I planed to remove it in next update or commit.

I already tested changes.