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

scenarioRef should be a non-optional member of UIGameplay #2265

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

scenarioRef should be a non-optional member of UIGameplay #2265

kostmo opened this issue Jan 5, 2025 · 1 comment
Assignees
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 5, 2025

scenarioRef should be a non-optional member of UIGameplay, and uiGameplay should be an optional member of UIState.

This would encode the invariant that UIGameplay pertains to an actively-running scenario. That is, the uiGameplay member should be Nothing when not playing a particular scenario (i.e. in the toplevel menu), and a Just UIGameplay while playing. With that, we should be able to guarantee that the UIGameplay record always has a non-null reference to a Scenario.

This would simplify #2264.

@kostmo kostmo added the Z-Refactoring This issue is about restructuring the code without changing the behaviour to improve code quality. label Jan 5, 2025
@byorgey
Copy link
Member

byorgey commented Jan 5, 2025

This makes a lot of sense. Currently, every time we start a scenario we manually reset all of the UIGameplay fields within the UIState to default values (see scenarioToUIState:

& uiGameplay . uiDialogs . uiGoal .~ emptyGoalDisplay
& uiGameplay . uiIsAutoPlay .~ isAutoplaying
& uiGameplay . uiFocusRing .~ initFocusRing
& uiGameplay . uiInventory . uiInventorySearch .~ Nothing
& uiGameplay . uiInventory . uiInventoryList .~ Nothing
& uiGameplay . uiInventory . uiInventorySort .~ defaultSortOptions
& uiGameplay . uiInventory . uiShowZero .~ True
& uiGameplay . uiTiming . uiShowFPS .~ False
& uiGameplay . uiREPL .~ initREPLState (u ^. uiGameplay . uiREPL . replHistory)
& uiGameplay . uiREPL . replHistory %~ restartREPLHistory
& uiGameplay . scenarioRef ?~ siPair
& uiGameplay . uiTiming . lastFrameTime .~ curTime
& uiGameplay . uiWorldEditor . EM.entityPaintList %~ BL.listReplace entityList Nothing
& uiGameplay . uiWorldEditor . EM.editingBounds . EM.boundsRect %~ setNewBounds
& uiGameplay . uiDialogs . uiStructure
.~ StructureDisplay
(SR.makeListWidget . M.elems $ gs ^. landscape . recognizerAutomatons . originalStructureDefinitions)
(focusSetCurrent (StructureWidgets StructuresList) $ focusRing $ map StructureWidgets enumerate)
), but this is largely duplicated in initUIState. The one exception to this (that I have noticed; there could be others) is the REPL history: we want to be careful to preserve that across scenarios.

Note we could also get rid of the _uiPlaying field in UIState: whether we are playing a scenario would just be determined by whether the uiGameplay is Nothing or Just. If we want, we could still define a uiPlaying Getter as a convenience.

@kostmo kostmo self-assigned this Jan 5, 2025
mergify bot pushed a commit that referenced this issue Jan 6, 2025
Towards #2265.

This will be a multi-part refactoring, since `uiGamestate` is accessed in hundreds of places.  The more repetition that can be eliminated, the easier it will be to eventually modify the type of the record fields.

In this PR, a few no-op refactorings:
* use a narrower type in many places (e.g. pass `UIGameplay` rather than `AppState` as function parameter
* define local aliases for long lens chains
* Use `zoom` for blocks of operations on deeply nested state
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