Skip to content
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

Move GameState and UIGameplay into new subrecord in AppState #2276

Open
kostmo opened this issue Jan 6, 2025 · 1 comment
Open

Move GameState and UIGameplay into new subrecord in AppState #2276

kostmo opened this issue Jan 6, 2025 · 1 comment
Labels
Z-Refactoring This issue is about restructuring the code without changing the behaviour to improve code quality.

Comments

@kostmo
Copy link
Member

kostmo commented Jan 6, 2025

Currently, AppState has a dedicated field RuntimeState for scenario-independent state, and a field GameState for scenario-dependent state. However, UIState contains an awkward mix of active-play-dependent widget states and play-independent states (see #2265).

data AppState = AppState
{ _gameState :: GameState
, _uiState :: UIState
, _keyEventHandling :: KeyEventHandlingState
, _runtimeState :: RuntimeState
}

When embarking on #2265 I noticed that GameState tends to be accessed often alongside UIGameplay. I propose a new toplevel member PlayingState of the AppState that shall contain both GameState and UIGameplay. Eventually, PlayingState should be made nullable.

This refactoring may be accomplished in phases:

  1. Move GameState underneath a new toplevel record PlayingState.
  2. Move UIGameplay out of UIState into a second field of PlayingState.
  3. Wrap the PlayingState field in a Maybe
  4. Make scenarioRef mandatory in PlayingState (scenarioRef should be a non-optional member of UIGameplay #2265)
@kostmo kostmo added the Z-Refactoring This issue is about restructuring the code without changing the behaviour to improve code quality. label Jan 6, 2025
@byorgey
Copy link
Member

byorgey commented Jan 9, 2025

Sounds good to me. When we first created the AppState record it seemed like a sensible organization to put all the gameplay stuff in one record and the UI stuff in another. However, I think you're right that the better top-level organizational principle should be "stuff that persists across scenarios" and "stuff that only exists while playing a scenario".

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.
Projects
None yet
Development

No branches or pull requests

2 participants