-
Notifications
You must be signed in to change notification settings - Fork 55
Move GameState
and UIGameplay
into new subrecord in AppState
#2276
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Labels
Z-Refactoring
This issue is about restructuring the code without changing the behaviour to improve code quality.
Comments
Sounds good to me. When we first created the |
Merged
mergify bot
pushed a commit
that referenced
this issue
Mar 30, 2025
Performs phases 1 and 2 of #2276: > 1. Move GameState underneath a new toplevel record PlayState. > 2. Move UIGameplay out of UIState into a second field of PlayState. This results in many instances of an extra `playState .` lens layer. These will be incrementally removed again in subsequent PRs (e.g. #2362) as functions are refined to narrower state scope.
mergify bot
pushed a commit
that referenced
this issue
Mar 31, 2025
Continues toward #2276. This PR incrementally pushes the state scope to the narrower `PlayState` rather than `AppState` for many utility functions, re-simplifying the code in many cases where an extra `playState` lens layer was added in #2282. Counting the occurrences of the `playState` lens has been a good metric for the progress on this refactoring. For functions where this is not yet feasible, common reasons are: * need access to the `uiState . uiMenu` * In particular, `toggleModal`/`openModal` need access to the `Menu` for use by `generateModal`. * And more specifically, for the "win", "lose", and "quit" modals, either to determine if it is `NoMenu` or to determine which scenario comes next. * need access to `uiState . uiDebugOptions` * need access to the `ScenarioCollection` (by way of `normalizeScenarioPath`) for: * `saveScenarioInfoOnFinishNocheat` * `saveScenarioInfoOnQuit` * Need access to `uiState . uiAchievements` for: * `runGameTick` This exercise is informative as to which record members we may want to re-organize.
mergify bot
pushed a commit
that referenced
this issue
Apr 5, 2025
Toward #2276. # In this PR * Decompose `updateUI` into smaller functions * add documentation for `UIGameplay` and `UIState` records * As described in #2362, the `uiState . uiMenu` is used in a number of places, often just to check whether it is set to `NoMenu`. We instead precompute this condition and pass it as a parameter. * Instead of retrieving from the implicit `AppState`, we pass the `ScenarioCollection` ( `runtimeState . scenarios` ) explicitly to `getNormalizedCurrentScenarioPath`. # Long-term goal The "main layers" of the UI can either comprise the active playfield (`drawGameUI`) or the out-of-game menu (`drawMenuUI`): https://github.com/swarm-game/swarm/blob/68e57fc040283b0d2659a6336ed6521b20c684f5/src/swarm-tui/Swarm/TUI/View.hs#L150-L154 Since these are mutually exclusive, we should extricate the state needed for either one, rather than passing the complete `AppState` to both. In fact there are a couple of places in the code that describe "unreachable combinations" of state: https://github.com/swarm-game/swarm/blob/68e57fc040283b0d2659a6336ed6521b20c684f5/src/swarm-tui/Swarm/TUI/View.hs#L158-L160 and https://github.com/swarm-game/swarm/blob/68e57fc040283b0d2659a6336ed6521b20c684f5/src/swarm-tui/Swarm/TUI/Controller.hs#L155-L158 As part of the deconflation of state, we should also be able to solve this: #1064 (comment)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Z-Refactoring
This issue is about restructuring the code without changing the behaviour to improve code quality.
Currently,
AppState
has a dedicated fieldRuntimeState
for scenario-independent state, and a fieldGameState
for scenario-dependent state. However,UIState
contains an awkward mix of active-play-dependent widget states and play-independent states (see #2265).swarm/src/swarm-tui/Swarm/TUI/Model.hs
Lines 176 to 181 in e7b4d65
When embarking on #2265 I noticed that
GameState
tends to be accessed often alongsideUIGameplay
. I propose a new toplevel memberPlayState
of theAppState
that shall contain bothGameState
andUIGameplay
. Eventually,PlayState
should be made nullable.This refactoring may be accomplished in phases:
GameState
underneath a new toplevel recordPlayState
.UIGameplay
out ofUIState
into a second field ofPlayState
.PlayState
field in aMaybe
scenarioRef
mandatory inPlayState
(scenarioRef should be a non-optional member of UIGameplay #2265)The text was updated successfully, but these errors were encountered: