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 )
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)]:
- Start a non-rated lobby game with late-observer-joins enabled
- Join the lobby with another account
- Join that running game
- The developer overlay can be opened for the host, but not for the rejoined client.
Reproduce [making a rated game non-rated (not tested)]:
- Join the lobby
- Join a rated game, let the game start
- Leave the game after it started
- Join a non-rated game that has not started yet and leave it
- Rejoin the rated game
- 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 , 8 years ago
Keywords: | beta added |
---|
comment:2 by , 4 years ago
Description: | modified (diff) |
---|---|
Keywords: | beta removed |
Milestone: | Backlog → Alpha 24 |
Priority: | Must Have → Should 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 accessg_GameAttributes.settings.RatingEnabled
andg_IsReplay
to fix the anti-pattern. I discovered it's broken becauseSetRankedGame
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):
.
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.)