fix for issue #1178 (BulletAppState violates AppState contract) (#1187)

* fix for issue #1178 (BulletAppState violates AppState contract)

* BulletAppState: make isRunning volatile (potential multithreaded access)

Author: stephengold

jme3-bullet/src/common/java/com/jme3/bullet/BulletAppState.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009-2018 jMonkeyEngine
+ * Copyright (c) 2009-2019 jMonkeyEngine
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -32,7 +32,7 @@
 package com.jme3.bullet;
 
 import com.jme3.app.Application;
-import com.jme3.app.state.AppState;
+import com.jme3.app.state.AbstractAppState;
 import com.jme3.app.state.AppStateManager;
 import com.jme3.bullet.PhysicsSpace.BroadphaseType;
 import com.jme3.bullet.debug.BulletDebugAppState;
@@ -49,27 +49,16 @@
  *
  * @author normenhansen
  */
-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
+public class BulletAppState
+        extends AbstractAppState
+        implements PhysicsTickListener {
 
     /**
      * true if-and-only-if the physics simulation is running (started but not
      * yet stopped)
      */
-    protected boolean initialized = false;
+    protected volatile boolean isRunning = false;
     protected Application app;
-    private String id;
-    
     /**
      * manager that manages this state, set during attach
      */
@@ -245,7 +234,7 @@ public PhysicsSpace getPhysicsSpace() {
      * sooner, invoke this method.
      */
     public void startPhysics() {
-        if (initialized) {
+        if (isRunning) {
             return;
         }
 
@@ -265,14 +254,14 @@ public void startPhysics() {
                 throw new IllegalStateException(threadingType.toString());
         }
 
-        initialized = true;
+        isRunning = true;
     }
 
     /**
      * Stop physics after this state is detached.
      */
     public void stopPhysics() {
-        if(!initialized){
+        if (!isRunning) {
             return;
         }
         if (executor != null) {
@@ -281,7 +270,7 @@ public void stopPhysics() {
         }
         pSpace.removeTickListener(this);
         pSpace.destroy();
-        initialized = false;
+        isRunning = false;
     }
 
     /**
@@ -291,54 +280,14 @@ public void stopPhysics() {
      * @param stateManager the manager for this state (not null)
      * @param app the application which owns this state (not null)
      */
+    @Override
     public void initialize(AppStateManager stateManager, Application app) {
+        super.initialize(stateManager, app);
         this.app = app;
         this.stateManager = stateManager;
         startPhysics();
     }
 
-    /**
-     * Test whether the physics simulation is running (started but not yet
-     * stopped).
-     *
-     * @return true if running, otherwise false
-     */
-    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;
-    }
-
-    /**
-     * Enable or disable this state.
-     *
-     * @param enabled true → enable, false → disable
-     */
-    public void setEnabled(boolean enabled) {
-        this.active = enabled;
-    }
-
-    /**
-     * Test whether this state is enabled.
-     *
-     * @return true if enabled, otherwise false
-     */
-    public boolean isEnabled() {
-        return active;
-    }
-
     /**
      * Alter whether debug visualization is enabled.
      *
@@ -364,8 +313,10 @@ public boolean isDebugEnabled() {
      *
      * @param stateManager (not null)
      */
+    @Override
     public void stateAttached(AppStateManager stateManager) {
-        if (!initialized) {
+        super.stateAttached(stateManager);
+        if (!isRunning) {
             startPhysics();
         }
         if (threadingType == ThreadingType.PARALLEL) {
@@ -377,23 +328,16 @@ public void stateAttached(AppStateManager stateManager) {
         }
     }
 
-    /**
-     * Transition this state from running to terminating. Should be invoked only
-     * by a subclass or by the AppStateManager.
-     *
-     * @param stateManager (not null)
-     */
-    public void stateDetached(AppStateManager stateManager) {
-    }
-
     /**
      * Update this state prior to rendering. Should be invoked only by a
      * subclass or by the AppStateManager. Invoked once per frame, provided the
      * state is attached and enabled.
      *
      * @param tpf the time interval between frames (in seconds, ≥0)
      */
+    @Override
     public void update(float tpf) {
+        super.update(tpf);
         if (debugEnabled && debugAppState == null && pSpace != null) {
             debugAppState = new BulletDebugAppState(pSpace);
             stateManager.attach(debugAppState);
@@ -415,7 +359,9 @@ public void update(float tpf) {
      *
      * @param rm the render manager (not null)
      */
+    @Override
     public void render(RenderManager rm) {
+        super.render(rm);
         if (!active) {
             return;
         }
@@ -432,7 +378,9 @@ public void render(RenderManager rm) {
      * invoked only by a subclass or by the AppStateManager. Invoked once per
      * frame, provided the state is attached and enabled.
      */
+    @Override
     public void postRender() {
+        super.postRender();
         if (physicsFuture != null) {
             try {
                 physicsFuture.get();
@@ -451,12 +399,14 @@ public void postRender() {
      * {@link #initialize(com.jme3.app.state.AppStateManager, com.jme3.app.Application)}
      * is invoked.
      */
+    @Override
     public void cleanup() {
         if (debugAppState != null) {
             stateManager.detach(debugAppState);
             debugAppState = null;
         }
         stopPhysics();
+        super.cleanup();
     }
 
     /**