From 3b8314a36f942939f8e12c7a5b0469dbf11f41ec Mon Sep 17 00:00:00 2001 From: Paul Speed Date: Sun, 15 Sep 2019 19:38:50 -0400 Subject: [PATCH] Part 1 of small refactoring to give AppState a unique ID for lookup purposes. This is a breaking change for direct implementers of AppState as they must add a getId() method. (Which is also a good time to evaluate if they should be extending AbstractAppState or BaseAppState and/or if their life cycle is implemented correctly. e.g.: BulletAppState is breaking the AppState contract a bit.) --- .../java/com/jme3/bullet/BulletAppState.java | 27 +++++++++++++++ .../com/jme3/app/state/AbstractAppState.java | 34 ++++++++++++++++++- .../java/com/jme3/app/state/AppState.java | 6 ++++ .../java/com/jme3/app/state/BaseAppState.java | 22 ++++++++++++ .../com/jme3/app/state/RootNodeAppState.java | 18 ++++++++++ .../java/com/jme3/cinematic/Cinematic.java | 15 ++++++++ 6 files changed, 121 insertions(+), 1 deletion(-) diff --git a/jme3-bullet/src/common/java/com/jme3/bullet/BulletAppState.java b/jme3-bullet/src/common/java/com/jme3/bullet/BulletAppState.java index 9fb5b85f1..07a6ff608 100644 --- a/jme3-bullet/src/common/java/com/jme3/bullet/BulletAppState.java +++ b/jme3-bullet/src/common/java/com/jme3/bullet/BulletAppState.java @@ -51,12 +51,25 @@ import java.util.logging.Logger; */ public class BulletAppState implements AppState, PhysicsTickListener { + // FIXME: the bullet app state doesn't follow the proper AppState + // contract as it messes with its initialized state independently + // of when initialize()/cleanup() is actually called. This means + // that it's quite likely that the state manager will think the + // app state is initialized when it, itself, doesn't. This is + // a good example of why extending the abstract app state classes + // is better than implementing app state directly. If it wants + // to support a separate stated/not-started concept then that's + // separate from initialized/not-initialized but way more refactoring + // than I want to think about today. -pspeed:2019-09-15 + /** * true if-and-only-if the physics simulation is running (started but not * yet stopped) */ protected boolean initialized = false; protected Application app; + private String id; + /** * manager that manages this state, set during attach */ @@ -294,6 +307,20 @@ public class BulletAppState implements AppState, PhysicsTickListener { return initialized; } + /** + * Sets the unique ID of this app state. Note: that setting + * this while an app state is attached to the state manager will + * have no effect on ID-based lookups. + */ + protected void setId( String id ) { + this.id = id; + } + + @Override + public String getId() { + return id; + } + /** * Enable or disable this state. * diff --git a/jme3-core/src/main/java/com/jme3/app/state/AbstractAppState.java b/jme3-core/src/main/java/com/jme3/app/state/AbstractAppState.java index c2e3642da..96b7e6b6c 100644 --- a/jme3-core/src/main/java/com/jme3/app/state/AbstractAppState.java +++ b/jme3-core/src/main/java/com/jme3/app/state/AbstractAppState.java @@ -40,7 +40,7 @@ import com.jme3.renderer.RenderManager; * @author Kirill Vainer * @see com.jme3.app.state.BaseAppState */ -public class AbstractAppState implements AppState { +public abstract class AbstractAppState implements AppState { /** * initialized is set to true when the method @@ -50,38 +50,70 @@ public class AbstractAppState implements AppState { */ protected boolean initialized = false; private boolean enabled = true; + private String id; + + protected AbstractAppState() { + } + + protected AbstractAppState( String id ) { + this.id = id; + } + @Override public void initialize(AppStateManager stateManager, Application app) { initialized = true; } + @Override public boolean isInitialized() { return initialized; } + /** + * Sets the unique ID of this app state. Note: that setting + * this while an app state is attached to the state manager will + * have no effect on ID-based lookups. + */ + protected void setId( String id ) { + this.id = id; + } + + @Override + public String getId() { + return id; + } + + @Override public void setEnabled(boolean enabled) { this.enabled = enabled; } + @Override public boolean isEnabled() { return enabled; } + @Override public void stateAttached(AppStateManager stateManager) { } + @Override public void stateDetached(AppStateManager stateManager) { } + @Override public void update(float tpf) { } + @Override public void render(RenderManager rm) { } + @Override public void postRender(){ } + @Override public void cleanup() { initialized = false; } diff --git a/jme3-core/src/main/java/com/jme3/app/state/AppState.java b/jme3-core/src/main/java/com/jme3/app/state/AppState.java index 72988c0ef..33a6d28d5 100644 --- a/jme3-core/src/main/java/com/jme3/app/state/AppState.java +++ b/jme3-core/src/main/java/com/jme3/app/state/AppState.java @@ -90,6 +90,12 @@ public interface AppState { */ public boolean isInitialized(); + /** + * Returns the unique ID for this AppState or null if it has no + * unique ID. + */ + public String getId(); + /** * Enable or disable the functionality of the AppState. * The effect of this call depends on implementation. An diff --git a/jme3-core/src/main/java/com/jme3/app/state/BaseAppState.java b/jme3-core/src/main/java/com/jme3/app/state/BaseAppState.java index cee045355..a74521561 100644 --- a/jme3-core/src/main/java/com/jme3/app/state/BaseAppState.java +++ b/jme3-core/src/main/java/com/jme3/app/state/BaseAppState.java @@ -78,6 +78,14 @@ public abstract class BaseAppState implements AppState { private Application app; private boolean initialized; private boolean enabled = true; + private String id; + + protected BaseAppState() { + } + + protected BaseAppState( String id ) { + this.id = id; + } /** * Called during initialization once the app state is @@ -133,6 +141,20 @@ public abstract class BaseAppState implements AppState { return initialized; } + /** + * Sets the unique ID of this app state. Note: that setting + * this while an app state is attached to the state manager will + * have no effect on ID-based lookups. + */ + protected void setId( String id ) { + this.id = id; + } + + @Override + public String getId() { + return id; + } + public final Application getApplication() { return app; } diff --git a/jme3-core/src/main/java/com/jme3/app/state/RootNodeAppState.java b/jme3-core/src/main/java/com/jme3/app/state/RootNodeAppState.java index 8daa9dc3a..f7406ba55 100644 --- a/jme3-core/src/main/java/com/jme3/app/state/RootNodeAppState.java +++ b/jme3-core/src/main/java/com/jme3/app/state/RootNodeAppState.java @@ -85,6 +85,18 @@ public class RootNodeAppState extends AbstractAppState { this.rootNode = rootNode; } + /** + * Creates the AppState with the given unique ID, ViewPort, and root Node, attaches + * the root Node to the ViewPort and updates it. + * @param viewPort An existing ViewPort + * @param rootNode An existing root Node + */ + public RootNodeAppState( String id, ViewPort viewPort, Node rootNode ) { + super(id); + this.viewPort = viewPort; + this.rootNode = rootNode; + } + @Override public void initialize(AppStateManager stateManager, Application app) { if (rootNode == null) { @@ -101,6 +113,12 @@ public class RootNodeAppState extends AbstractAppState { public void update(float tpf) { super.update(tpf); rootNode.updateLogicalState(tpf); + + // FIXME: I'm 99% sure that updateGeometricState() should be + // called in render() so that it is done as late as possible. + // In complicated app state setups, cross-state chatter could + // cause nodes (or their children) to be updated after this + // app state's update has been called. -pspeed:2019-09-15 rootNode.updateGeometricState(); } diff --git a/jme3-core/src/main/java/com/jme3/cinematic/Cinematic.java b/jme3-core/src/main/java/com/jme3/cinematic/Cinematic.java index d9e9d5b11..3bcbc17a7 100644 --- a/jme3-core/src/main/java/com/jme3/cinematic/Cinematic.java +++ b/jme3-core/src/main/java/com/jme3/cinematic/Cinematic.java @@ -99,6 +99,7 @@ public class Cinematic extends AbstractCinematicEvent implements AppState { private boolean initialized = false; private Map> eventsData; private float nextEnqueue = 0; + private String id; /** * Used for serialization creates a cinematic, don't use this constructor @@ -291,6 +292,20 @@ public class Cinematic extends AbstractCinematicEvent implements AppState { return initialized; } + /** + * Sets the unique ID of this app state. Note: that setting + * this while an app state is attached to the state manager will + * have no effect on ID-based lookups. + */ + protected void setId( String id ) { + this.id = id; + } + + @Override + public String getId() { + return id; + } + /** * passing true has the same effect as play() you should use play(), * pause(), stop() to handle the cinematic playing state.