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.

Sign up for GitHub

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

Jump to bottom

group robot info into subrecord #1652

Merged
merged 1 commit into from
Nov 22, 2023
Merged

group robot info into subrecord #1652

merged 1 commit into from
Nov 22, 2023

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Nov 22, 2023 •
edited
Loading

Add another subrecord to GameState dedicated to robot fields. This shrinks the size of GameState by 8 more fields. It's getting reasonable now!

The _viewCenter* fields were put there too, for now, because the manually-defined setter of the viewCenterRule lens needs access to the robotMap field.

@kostmo kostmo added the Z-Refactoring This issue is about restructuring the code without changing the behaviour to improve code quality. label Nov 22, 2023
@kostmo kostmo marked this pull request as ready for review November 22, 2023 07:53
@kostmo kostmo requested a review from byorgey November 22, 2023 07:53
byorgey
byorgey approved these changes Nov 22, 2023
View reviewed changes
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.

🎉

@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Nov 22, 2023
@mergify mergify bot merged commit 3c970c1 into main Nov 22, 2023
9 checks passed
@kostmo kostmo mentioned this pull request Nov 30, 2023
Merged
1 task
mergify bot pushed a commit that referenced this pull request Nov 30, 2023
@kostmo
Offload subrecords of GameState to other modules (#1667)
3094abd 
This is a continuation of #1652.

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`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@byorgey byorgey byorgey approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kostmo @byorgey