refactor(GameState): extract common code from MainMenu and HeadlessSetup #4809
Conversation
keturn
added
Size: S
| // TODO: Reduce coupling between Input system and CameraTargetSystem, | ||
| // TODO: potentially eliminating the following lines. See Issue #1126 | ||
| CameraTargetSystem cameraTargetSystem = new CameraTargetSystem(); | ||
| context.put(CameraTargetSystem.class, cameraTargetSystem); |
There was a problem hiding this comment.
It looks like I've removed this use of CameraTargetSystem in the main menu, but that's not really the case. It still happens within RegisterInputSystem called in this method.
| protected EventSystem eventSystem; | ||
| protected ComponentSystemManager componentSystemManager; | ||
|
|
||
| protected void initEntityAndComponentManagers() { |
There was a problem hiding this comment.
This does a lot with the context and strictly requires it. Would it make sense to make the context and argument of this method, similar to how we pass it to createLocalPlayer explcitly?
There was a problem hiding this comment.
createLocalPlayer is the method I extracted first. Passing context let me make it static, which I figured would make it easier to move the method later if there were more copies of it.
Then I realized if I shuffled some things around I could also extract all this stuff that went in to initEntityAndComponentManagers, but because it assigns things to multiple instance fields, it wouldn't work as static.
Upon review, I see that all these fields are direct references to a thing in the context, so it probably could be split in to a "populate context with this stuff" static method and then a "pull some references out of context and cache them in fields" non-static method, but I worried that would make it more difficult to review if I included it in the same PR.
I was working with the StateHeadlessSetup class and noticing it has a lot of code duplicated from another class. To make it easier to see what the differences between the various classes are, I extracted the common code to some methods.
No change in functionality is intended by this PR, but I did re-order the execution of some of these components, which has the potential to break something.
Notes to Reviewers
I'm not thrilled about the state of things here, but it's a refactor, so please try to limit the scope of your requests to the refactoring. I'd also like to avoid getting pulled down the rabbit hole of putting all the potentially related refactors we can think of into this PR.
Things it would be useful to have feedback on:
AbstractStateis a particularly unhelpful name.