Changeset View
Standalone View
binaries/data/mods/public/gui/gamesettings/attributes/Rating.js
Show All 30 Lines | GameSettings.prototype.Attributes.Rating = class Rating extends GameSetting | ||||
maybeUpdate() | maybeUpdate() | ||||
{ | { | ||||
// This setting is activated by default if it's possible. | // This setting is activated by default if it's possible. | ||||
this.available = this.hasXmppClient && | this.available = this.hasXmppClient && | ||||
this.settings.playerCount.nbPlayers === 2 && | this.settings.playerCount.nbPlayers === 2 && | ||||
!this.settings.cheats.enabled; | !this.settings.cheats.enabled; | ||||
this.enabled = this.available; | this.enabled = this.available; | ||||
} | } | ||||
onFinalizeAttributes() | |||||
{ | |||||
Engine.SetRankedGame(this.enabled); | |||||
} | |||||
wraitii: Likewise, I dislike this. You're changing global state in a class that's ostensibly about the… | |||||
Done Inline ActionsThis is exactly the point of this having this function. launchgame in Gamesettings shouldn't have a clue of what settings there are and just let the settings handle themselves. It shouldn't even know if it possibly is rated. Removing the Rating.js files should disable all the rating. Gamesettings shouldn't even check whether it needs to do something. bb: This is exactly the point of this having this function. launchgame in Gamesettings shouldn't… | |||||
Not Done Inline ActionsMh, what you wrote makes a lot of sense, I'm not entirely sure I agree though. The 'logical' order of operation seems kinda backwards, because it says the game is ranked before it's launched, which doesn't make that much sense (e.g. the game has no match ID so far). That being said, that's perhaps for the future. This is changing global state in a function that isn't ostensibly about that (as you wrote in the summary, it's currently 'anything goes', which doesn't seem great). Feels like this shouldn't have side-effects to me, hence why I'd prefer it called toFinalizedAttributes and just being a serialisation function. I'd probably prefer a separate "OnGameLaunched" function that takes the finalised arguments as a parameter, and then sets up rating. It can still live in Rating.js, but it'd be more explicit. wraitii: Mh, what you wrote makes a lot of sense, I'm not entirely sure I agree though.
The 'logical'… | |||||
}; | }; |
Likewise, I dislike this. You're changing global state in a class that's ostensibly about the rating _setting_ in the GUI setup state. This change should be done in the 'controller-equivalent' class, which is launchGame in GameSettings.js where I originally had it.