-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
refactor(GameState): extract common code from MainMenu and HeadlessSetup #4809
Merged
Merged
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
b2d4124
refactor(GameState): extract common code from MainMenu and HeadlessSetup
keturn dbc92bd
chore(StateLoading): address lint messages 🚮
keturn a13dd14
Merge branch 'develop' into refactor/mainState
pollend ea5b13f
Merge branch 'develop' into refactor/mainState
skaldarnar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
84 changes: 84 additions & 0 deletions
84
engine/src/main/java/org/terasology/engine/core/modes/AbstractState.java
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,84 @@ | ||
// Copyright 2021 The Terasology Foundation | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package org.terasology.engine.core.modes; | ||
|
||
import org.terasology.engine.context.Context; | ||
import org.terasology.engine.core.ComponentSystemManager; | ||
import org.terasology.engine.core.bootstrap.EntitySystemSetupUtil; | ||
import org.terasology.engine.entitySystem.entity.EntityRef; | ||
import org.terasology.engine.entitySystem.entity.internal.EngineEntityManager; | ||
import org.terasology.engine.entitySystem.event.internal.EventSystem; | ||
import org.terasology.engine.logic.console.Console; | ||
import org.terasology.engine.logic.console.ConsoleImpl; | ||
import org.terasology.engine.logic.console.ConsoleSystem; | ||
import org.terasology.engine.logic.console.commands.CoreCommands; | ||
import org.terasology.engine.logic.players.LocalPlayer; | ||
import org.terasology.engine.network.ClientComponent; | ||
import org.terasology.engine.recording.DirectionAndOriginPosRecorderList; | ||
import org.terasology.engine.recording.RecordAndReplayCurrentStatus; | ||
import org.terasology.engine.registry.CoreRegistry; | ||
import org.terasology.engine.rendering.nui.NUIManager; | ||
import org.terasology.engine.rendering.nui.internal.NUIManagerInternal; | ||
import org.terasology.engine.rendering.nui.internal.TerasologyCanvasRenderer; | ||
import org.terasology.nui.canvas.CanvasRenderer; | ||
|
||
import static com.google.common.base.Verify.verifyNotNull; | ||
|
||
public abstract class AbstractState implements GameState { | ||
protected Context context; | ||
protected EngineEntityManager entityManager; | ||
protected EventSystem eventSystem; | ||
protected ComponentSystemManager componentSystemManager; | ||
|
||
protected void initEntityAndComponentManagers() { | ||
verifyNotNull(context); | ||
CoreRegistry.setContext(context); | ||
|
||
// let's get the entity event system running | ||
EntitySystemSetupUtil.addEntityManagementRelatedClasses(context); | ||
entityManager = context.get(EngineEntityManager.class); | ||
|
||
eventSystem = context.get(EventSystem.class); | ||
context.put(Console.class, new ConsoleImpl(context)); | ||
|
||
NUIManager nuiManager = new NUIManagerInternal((TerasologyCanvasRenderer) context.get(CanvasRenderer.class), context); | ||
context.put(NUIManager.class, nuiManager); | ||
|
||
componentSystemManager = new ComponentSystemManager(context); | ||
context.put(ComponentSystemManager.class, componentSystemManager); | ||
|
||
componentSystemManager.register(new ConsoleSystem(), "engine:ConsoleSystem"); | ||
componentSystemManager.register(new CoreCommands(), "engine:CoreCommands"); | ||
} | ||
|
||
protected static void createLocalPlayer(Context context) { | ||
EngineEntityManager entityManager = context.get(EngineEntityManager.class); | ||
EntityRef localPlayerEntity = entityManager.create(new ClientComponent()); | ||
LocalPlayer localPlayer = new LocalPlayer(); | ||
localPlayer.setRecordAndReplayClasses(context.get(DirectionAndOriginPosRecorderList.class), | ||
context.get(RecordAndReplayCurrentStatus.class)); | ||
context.put(LocalPlayer.class, localPlayer); | ||
localPlayer.setClientEntity(localPlayerEntity); | ||
} | ||
|
||
@Override | ||
public void dispose(boolean shuttingDown) { | ||
// Apparently this can be disposed of before it is completely initialized? Probably only during | ||
// crashes, but crashing again during shutdown complicates the diagnosis. | ||
if (eventSystem != null) { | ||
eventSystem.process(); | ||
} | ||
if (componentSystemManager != null) { | ||
componentSystemManager.shutdown(); | ||
} | ||
if (entityManager != null) { | ||
entityManager.clear(); | ||
} | ||
} | ||
|
||
@Override | ||
public Context getContext() { | ||
return context; | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,52 +5,27 @@ | |
import org.terasology.engine.audio.AudioManager; | ||
import org.terasology.engine.config.Config; | ||
import org.terasology.engine.config.TelemetryConfig; | ||
import org.terasology.engine.context.Context; | ||
import org.terasology.engine.core.ComponentSystemManager; | ||
import org.terasology.engine.core.GameEngine; | ||
import org.terasology.engine.core.LoggingContext; | ||
import org.terasology.engine.core.bootstrap.EntitySystemSetupUtil; | ||
import org.terasology.engine.core.modes.loadProcesses.RegisterInputSystem; | ||
import org.terasology.engine.entitySystem.entity.EntityRef; | ||
import org.terasology.engine.entitySystem.entity.internal.EngineEntityManager; | ||
import org.terasology.engine.entitySystem.event.internal.EventSystem; | ||
import org.terasology.engine.i18n.TranslationSystem; | ||
import org.terasology.engine.identity.storageServiceClient.StorageServiceWorker; | ||
import org.terasology.engine.input.InputSystem; | ||
import org.terasology.engine.input.cameraTarget.CameraTargetSystem; | ||
import org.terasology.engine.logic.console.Console; | ||
import org.terasology.engine.logic.console.ConsoleImpl; | ||
import org.terasology.engine.logic.console.ConsoleSystem; | ||
import org.terasology.engine.logic.console.commands.CoreCommands; | ||
import org.terasology.engine.logic.players.LocalPlayer; | ||
import org.terasology.engine.network.ClientComponent; | ||
import org.terasology.engine.recording.DirectionAndOriginPosRecorderList; | ||
import org.terasology.engine.recording.RecordAndReplayCurrentStatus; | ||
import org.terasology.engine.registry.CoreRegistry; | ||
import org.terasology.engine.rendering.nui.NUIManager; | ||
import org.terasology.engine.rendering.nui.editor.systems.NUIEditorSystem; | ||
import org.terasology.engine.rendering.nui.editor.systems.NUISkinEditorSystem; | ||
import org.terasology.engine.rendering.nui.internal.NUIManagerInternal; | ||
import org.terasology.engine.rendering.nui.internal.TerasologyCanvasRenderer; | ||
import org.terasology.engine.rendering.nui.layers.mainMenu.LaunchPopup; | ||
import org.terasology.engine.rendering.nui.layers.mainMenu.MessagePopup; | ||
import org.terasology.engine.telemetry.TelemetryScreen; | ||
import org.terasology.engine.telemetry.TelemetryUtils; | ||
import org.terasology.engine.telemetry.logstash.TelemetryLogstashAppender; | ||
import org.terasology.engine.utilities.Assets; | ||
import org.terasology.nui.canvas.CanvasRenderer; | ||
|
||
/** | ||
* The class implements the main game menu. | ||
* <br><br> | ||
* | ||
* @version 0.3 | ||
*/ | ||
public class StateMainMenu implements GameState { | ||
private Context context; | ||
private EngineEntityManager entityManager; | ||
private EventSystem eventSystem; | ||
private ComponentSystemManager componentSystemManager; | ||
public class StateMainMenu extends AbstractState { | ||
private NUIManager nuiManager; | ||
private InputSystem inputSystem; | ||
private Console console; | ||
|
@@ -69,33 +44,15 @@ public StateMainMenu(String showMessageOnLoad) { | |
@Override | ||
public void init(GameEngine gameEngine) { | ||
context = gameEngine.createChildContext(); | ||
CoreRegistry.setContext(context); | ||
initEntityAndComponentManagers(); | ||
|
||
//let's get the entity event system running | ||
EntitySystemSetupUtil.addEntityManagementRelatedClasses(context); | ||
entityManager = context.get(EngineEntityManager.class); | ||
createLocalPlayer(context); | ||
|
||
eventSystem = context.get(EventSystem.class); | ||
console = new ConsoleImpl(context); | ||
context.put(Console.class, console); | ||
|
||
nuiManager = new NUIManagerInternal((TerasologyCanvasRenderer) context.get(CanvasRenderer.class), context); | ||
context.put(NUIManager.class, nuiManager); | ||
// TODO: REMOVE this and handle refreshing of core game state at the engine level - see Issue #1127 | ||
new RegisterInputSystem(context).step(); | ||
|
||
nuiManager = context.get(NUIManager.class); | ||
eventSystem.registerEventHandler(nuiManager); | ||
|
||
componentSystemManager = new ComponentSystemManager(context); | ||
context.put(ComponentSystemManager.class, componentSystemManager); | ||
|
||
// 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); | ||
Comment on lines
-90
to
-93
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
||
componentSystemManager.register(cameraTargetSystem, "engine:CameraTargetSystem"); | ||
componentSystemManager.register(new ConsoleSystem(), "engine:ConsoleSystem"); | ||
componentSystemManager.register(new CoreCommands(), "engine:CoreCommands"); | ||
|
||
NUIEditorSystem nuiEditorSystem = new NUIEditorSystem(); | ||
context.put(NUIEditorSystem.class, nuiEditorSystem); | ||
componentSystemManager.register(nuiEditorSystem, "engine:NUIEditorSystem"); | ||
|
@@ -106,17 +63,9 @@ public void init(GameEngine gameEngine) { | |
|
||
inputSystem = context.get(InputSystem.class); | ||
|
||
// TODO: REMOVE this and handle refreshing of core game state at the engine level - see Issue #1127 | ||
new RegisterInputSystem(context).step(); | ||
|
||
EntityRef localPlayerEntity = entityManager.create(new ClientComponent()); | ||
LocalPlayer localPlayer = new LocalPlayer(); | ||
localPlayer.setRecordAndReplayClasses(context.get(DirectionAndOriginPosRecorderList.class), context.get(RecordAndReplayCurrentStatus.class)); | ||
context.put(LocalPlayer.class, localPlayer); | ||
localPlayer.setClientEntity(localPlayerEntity); | ||
|
||
componentSystemManager.initialise(); | ||
|
||
console = context.get(Console.class); | ||
storageServiceWorker = context.get(StorageServiceWorker.class); | ||
|
||
playBackgroundMusic(); | ||
|
@@ -164,22 +113,11 @@ private void pushLaunchPopup() { | |
|
||
@Override | ||
public void dispose(boolean shuttingDown) { | ||
// Apparently this can be disposed of before it is completely initialized? Probably only during | ||
// crashes, but crashing again during shutdown complicates the diagnosis. | ||
if (eventSystem != null) { | ||
eventSystem.process(); | ||
} | ||
if (componentSystemManager != null) { | ||
componentSystemManager.shutdown(); | ||
} | ||
stopBackgroundMusic(); | ||
|
||
if (nuiManager != null) { | ||
nuiManager.clear(); | ||
} | ||
if (entityManager != null) { | ||
entityManager.clear(); | ||
} | ||
super.dispose(shuttingDown); | ||
} | ||
|
||
private void playBackgroundMusic() { | ||
|
@@ -214,11 +152,6 @@ public String getLoggingPhase() { | |
return LoggingContext.MENU; | ||
} | ||
|
||
@Override | ||
public Context getContext() { | ||
return context; | ||
} | ||
|
||
@Override | ||
public boolean isHibernationAllowed() { | ||
return true; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does a lot with the
context
and strictly requires it. Would it make sense to make thecontext
and argument of this method, similar to how we pass it tocreateLocalPlayer
explcitly?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createLocalPlayer
is the method I extracted first. Passingcontext
let me make itstatic
, 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.