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

Offload subrecords of GameState to other modules #1667

Merged
merged 2 commits into from
Nov 30, 2023

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Nov 30, 2023

This is a continuation of #1652 and towards #707.

Most of the sub-records are bundled into Swarm.Game.State.Substate, but we create a Swarm.Game.State.Robot module just for robots.

We introduce a zoomRobots function so that applicable functions can operate directly on Robots state instead of GameState.

Size comparison

Before

File Lines
State.hs 1569

After

File Lines
State.hs 812
Substate.hs 497
Robot.hs 395
Config.hs 21

For follow-up PR:

  • Remove exports of _viewCenter and _focusedRobotID from Swarm.Game.State.Robot

@kostmo kostmo added the Z-Refactoring This issue is about restructuring the code without changing the behaviour to improve code quality. label Nov 30, 2023
@kostmo kostmo force-pushed the refactor/offload-sub-state-records branch from abd1bf5 to 6a0a960 Compare November 30, 2023 02:02
Introduce 'zoomRobots' so that applicable functions can operate
directly on 'Robots' state instead of 'GameState'.
@kostmo kostmo force-pushed the refactor/offload-sub-state-records branch from 6a0a960 to 31691b5 Compare November 30, 2023 02:31
@kostmo kostmo marked this pull request as ready for review November 30, 2023 04:43
@kostmo kostmo requested a review from byorgey November 30, 2023 04:43
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. My only minor gripe is that the name Swarm.Game.State.Substate seems rather generic. But I don't have any better suggestions. I suppose the alternative would be to make lots of tiny Swarm.Game.State.X modules, one for each substate record X, but that sounds tedious.

@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Nov 30, 2023
@mergify mergify bot merged commit 3094abd into main Nov 30, 2023
9 checks passed
@mergify mergify bot deleted the refactor/offload-sub-state-records branch November 30, 2023 21:57
mergify bot pushed a commit that referenced this pull request Dec 3, 2023
A continuation of #1667.

* Do not export `_viewCenter` or `_focusedRobotID`
* Move record initialization code to `Robot.hs` as `setRobotInfo`.
* `modifyViewCenter`, `unfocus`, `recalcViewCenter` now operate directly on `Robots` instead of `GameState`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request. Z-Refactoring This issue is about restructuring the code without changing the behaviour to improve code quality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants