Opened 9 years ago

Last modified 4 years ago

#3550 new defect

Rated game variable not set correctly — at Version 2

Reported by: elexis Owned by:
Priority: Should Have Milestone: Backlog
Component: Multiplayer lobby Keywords:
Cc: Patch:

Description (last modified by elexis)

Whether or not the current game is considered a ranked game is not updated in the correct places, resulting in some rated games being considered not rated and some non-rated games being considered rated.

Code: The variable indicating if the current game is rated is g_rankedGame in JSInterface_Lobby.cpp.

It will be set true whenever one joins the lobby (JSI_Lobby::StartXmppClient) and it will be set from the gamesetup only (JSI_Lobby::SetRankedGame which is only called from gamesetup.js whenever that checkbox changes).

However that's not sufficient to set the variable correctly.

Reproduce [making a non-rated game rated (tested)]:

  1. Start a non-rated lobby game with late-observer-joins enabled
  2. Join the lobby with another account
  3. Join that running game
  4. The developer overlay can be opened for the host, but not for the rejoined client.

Reproduce [making a rated game non-rated (not tested)]:

  1. Join the lobby
  2. Join a rated game, let the game start
  3. Leave the game after it started
  4. Join a non-rated game that has not started yet and leave it
  5. Rejoin the rated game
  6. Should be able to open the dev overlay

Since the gamesetup part is skipped when rejoining, SetRankedGame will never be set correctly.

See also #3547

Change History (2)

comment:1 by scythetwirler, 8 years ago

Keywords: beta added

comment:2 by elexis, 4 years ago

Description: modified (diff)
Keywords: beta removed
Milestone: BacklogAlpha 24
Priority: Must HaveShould Have

broken code rant:
What mostly stuck in memory was that there are two variables that store whether the game is rated and are set from different places with different logic, when they ought to have the same value at all times. That's basically a https://en.wikipedia.org/wiki/Single_source_of_truth violation.

While writing Phab:D2483: SetRankedGame is basically only necessary to disable the wireframe hotkey in rated games (the rest can directly access g_GameAttributes.settings.RatingEnabled and g_IsReplay to fix the anti-pattern. I discovered it's broken because SetRankedGame is only called from the gamesetup, but not for rejoining players. So if someone hosts with ratedgame=false and the other client rejoins, the wireframe hotkey will be disabled instead of enabled and the SetRankedGame c++ value will be set to true (default) when it should be false.

Then I realized I wrote precisely that 4 years ago here already and it didn't fix itself since, welcome to 2020 with the bugs from 2013- (though one can say we at least got some new bugs too):

resulting in some rated games being considered not rated and some non-rated games being considered rated.

.

The developer overlay can be opened for the host, but not for the rejoined client.

That bug was fixed in r19558 coincidentally.

beta keyword rant:
the beta keyword seems logically redundant with ">= should have", but it's tagged completely differently so what's the point of the keyword? That a ticket with the beta keyword is something that should be done for beta but should not necessarily be done the alpha stage? (Then why is it on "should-have" priority?) Or that a should-have ticket without a beta keyword should be done in the final release but not necessarily in the alpha or beta releases? Then it would be a matter of urgency, i.e. the same as the priority. So the only difference that I can see is addition of noise and using the word beta. The urgency of the bug should be indicated by the priority, otherwise there is no point in having the priority. beta as a milestone would also seem odd.)

Note: See TracTickets for help on using tickets.