Refactor game mode system to fix level data synchronization#713
Conversation
Fixes panel-attack#698 Various problems existed with the style system and classic endless mode and updating settings. The style wasn't updated all the time, and it wasn't preserved when opening the game again. In addition style settings could carry over to other game types. To fix this I have made a formal GameMode class that makes sure all the derived settings are updated when settings like style update. I also added a preferred style for tracking what the user picked, similar to how stages and characters work. The other clients only know about your "style" not preferred style, and thats only derived since we don't currently send style through the server messages. - Convert GameMode to proper class with methods - Separate UI style preference from game state style setting - Centralize level data updates in GameMode callbacks - Ensure consistent settings between client and server - Also fixed a challenge mode JSON serialization crash by using our JSON sanitizer again.
There was a problem hiding this comment.
Semantically it does not seem right to have references to the client in this file. common/data declares the intent for this to be interoperable data or a data class with no direct dependencies; adding functions that directly refer to client-side data and functions breaks with that distinction.
I think we should keep GameModes as data and instead utilize these style based update functions on the client-side only.
With their current usage it would seem most intuitive to move updateLocalPlayersDerivedSettings to BattleRoom. If GameModes keep the style flag for now, that function could simply check the flag to select the appropriate update function rather than them being baked into the GameMode itself, e.g.
function BattleRoom:updateLocalPlayersDerivedSettings(player)
if self.mode.style == GameModes.Styles.CHOOSE then
if self.mode.name == "endless" then
-- updateEndlessStyleBasedSettings
else
-- updateStyleBasedSettings
end
elseif self.mode.style == GameModes.Styles.MODERN then
-- updateModernSettings
end
endI think this also makes sense semantically as for the GameMode itself, the style is effectively irrelevant - style distinction is a pure client-side concern and we only communicate it server side for display purposes on spectator clients, not because it is actually relevant to the GameMode itself.
Arguing that way I realize that maybe it shouldn't be present as a flag on GameMode at all but for now it ended up there out of pragmatic concerns alongside a few other magic strings and we should try to only entangle it less with client-side concerns, not more.
UI changes → setLevelData() → levelDataChanged signal → auto-updates style/difficulty/level/UI/config Each character select subclass knows what difficulty / level / style mean and update accordingly. The server doesn't need to know about style at all. LevelPresets.lua : Added classicEndless presets (5 colors for easy), getStyleAndPreset() to detect which preset a levelData matches This lets us support more presets later and update the UI to show something reasonable when level data doesn't match a preset.
Endaris
left a comment
Endaris
left a comment
There was a problem hiding this comment.
Didn't really look over it in detail, thanks for the adjustments to the architecture.
Feel free to land if you think it's good.
2 participants
Fixes #698
Various problems existed with the style system and classic endless mode and updating settings. The style wasn't updated all the time, and it wasn't preserved when opening the game again. In addition style settings could carry over to other game types.
To fix this I have made a formal GameMode class that makes sure all the derived settings are updated when settings like style update. I also added a preferred style for tracking what the user picked, similar to how stages and characters work. The other clients only know about your "style" not preferred style, and thats only derived since we don't currently send style through the server messages.