DummyDisplaySystem infinite loop when initializing Timer

I get the following with DummyDisplaySystem.getTimer():



java.lang.StackOverflowError

at com.jmex.model.XMLparser.Converters.DummyDisplaySystem$1.getTimer(DummyDisplaySystem.java:106)

at com.jme.util.Timer.getTimer(Timer.java:106)

at com.jmex.model.XMLparser.Converters.DummyDisplaySystem$1.getTimer(DummyDisplaySystem.java:106)

at com.jme.util.Timer.getTimer(Timer.java:106)

        …



As you can see both methods are calling each other.

Mmm… there is a big mess with all the timers issue.



It is unclear to me where the Timers should be created.



I also don't see reasons (except historical ones) why DummyDisplaySystem belongs to the com.jmex.model.XMLparser.Converters package. It should be a quite important DisplaySystem.



There are many solutions to this, because a lot of subsystems are involved, and I don't understand what roles the timers play and where.



I'll try to suggest something when I have studied this well.

I've studied this and the more correct way of solving this.



Timer.getTimer() relies in the DisplaySystem.getSystemProvider().getTimer() to return a Timer.



As LWJGL creates its own Timer when requested, I think that the problem now is that StandardGame creates a timer for its own use, but Timer.getTimer() fails when using the DummyDisplaySystem (which does not create its own Timer, but relies on Timer.getTimer() instead).



So, the solution seems that DummyDisplaySystem has to create a correct DummySystemProvider (currently it uses an anonymous class) with a correct Timer. I've arranged all this and tested it for a while. I post a patch, which includes the other problems that StandardGame has when running headless (so the other patches are deprecated now).



This is the patch. Unified format and applies to the jme project.



I also suggest moving DummyDisplaySystem and related classes to the new package com.jme.system.dummy.



Questions, suggestions, errors, cons?



Index: src/com/jmex/game/StandardGame.java
===================================================================
RCS file: /cvs/jme/src/com/jmex/game/StandardGame.java,v
retrieving revision 1.6
diff -u -r1.6 StandardGame.java
--- src/com/jmex/game/StandardGame.java   17 Jan 2007 21:35:09 -0000   1.6
+++ src/com/jmex/game/StandardGame.java   24 Jan 2007 20:33:19 -0000
@@ -45,6 +45,7 @@
 import com.jme.renderer.ColorRGBA;
 import com.jme.system.DisplaySystem;
 import com.jme.system.GameSettings;
+import com.jme.system.JmeException;
 import com.jme.system.PreferencesGameSettings;
 import com.jme.util.GameTaskQueue;
 import com.jme.util.GameTaskQueueManager;
@@ -129,13 +130,9 @@
    public void run() {
       lock();
       initSystem();
-      assertDisplayCreated();
+      if (type == GameType.GRAPHICAL) assertDisplayCreated();
       initGame();
-      if (type == GameType.GRAPHICAL) {
-         timer = Timer.getTimer();
-      } else if (type == GameType.HEADLESS) {
-         timer = new NanoTimer();
-      }
+      timer = Timer.getTimer();
 
       // Configure frame rate
       int preferredFPS = settings.getFramerate();
@@ -148,7 +145,7 @@
       }
 
       // Default the mouse cursor to off
-      MouseInput.get().setCursorVisible(false);
+      if (type == GameType.GRAPHICAL) MouseInput.get().setCursorVisible(false);
       
       // Main game loop
       float tpf;
Index: src/com/jmex/model/XMLparser/Converters/DummyDisplaySystem.java
===================================================================
RCS file: /cvs/jme/src/com/jmex/model/XMLparser/Converters/DummyDisplaySystem.java,v
retrieving revision 1.28
diff -u -r1.28 DummyDisplaySystem.java
--- src/com/jmex/model/XMLparser/Converters/DummyDisplaySystem.java   16 Nov 2006 19:54:56 -0000   1.28
+++ src/com/jmex/model/XMLparser/Converters/DummyDisplaySystem.java   24 Jan 2007 20:33:19 -0000
@@ -76,6 +76,7 @@
 import com.jme.scene.state.lwjgl.records.StateRecord;
 import com.jme.system.DisplaySystem;
 import com.jme.system.SystemProvider;
+import com.jme.system.dummy.DummySystemProvider;
 import com.jme.util.Timer;
 import com.jmex.awt.JMECanvas;
 
@@ -93,22 +94,7 @@
 public class DummyDisplaySystem extends DisplaySystem {
 
     public DummyDisplaySystem() {
-        system = new SystemProvider() {
-            public String getProviderIdentifier() {
-                return "dummy";
-            }
-
-            public DisplaySystem getDisplaySystem() {
-                return DummyDisplaySystem.this;
-            }
-
-            public Timer getTimer() {
-                return Timer.getTimer();
-            }
-
-            public void installLibs() {
-            }
-        };
+        system = new DummySystemProvider(this);
     }
    
     public boolean isValidDisplayMode( int width, int height, int bpp, int freq ) {
Index: src/com/jme/system/dummy/DummySystemProvider.java
===================================================================
RCS file: src/com/jme/system/dummy/DummySystemProvider.java
diff -N src/com/jme/system/dummy/DummySystemProvider.java
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ src/com/jme/system/dummy/DummySystemProvider.java   1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,77 @@
+package com.jme.system.dummy;
+
+import com.jme.system.DisplaySystem;
+import com.jme.system.SystemProvider;
+import com.jme.util.NanoTimer;
+import com.jme.util.Timer;
+
+/**
+ * <p>SystemProvider for DummyDisplaySystem.</p>
+ * <p>It is a basic SystemProvider which allows to specify which
+ * DisplaySystem owns it and what Timer to use. If
+ * a Timer is not providen, a NanoTimer will
+ * be used.<p>
+ */
+public class DummySystemProvider implements SystemProvider {
+
+   /**
+    * The DummySystemProvider identifier
+    */
+   private final static String DUMMY_SYSTEM_IDENTIFIER = "dummy";
+   
+   /**
+    * The timer hold by this SystemProvider.
+    */
+   protected Timer timer = null;
+   
+   /**
+    * The DisplaySystem that this SystemProvider belongs to.
+    */
+   protected DisplaySystem displaySystem = null;
+   
+   /**
+    * Creates a new DummySystemProvider
+    * @param displaySystem The DisplaySystem that this SystemProvider belongs to.
+    */
+   public DummySystemProvider(DisplaySystem displaySystem) {
+      this(displaySystem, new NanoTimer());
+   }
+   
+   /**
+    * Creates a new DummySystemProvider
+    * @param displaySystem The DisplaySystem that this SystemProvider belongs to.
+    * @param timer The timer hold by this SystemProvider.
+    */
+   public DummySystemProvider(DisplaySystem displaySystem, Timer timer) {
+      this.timer = timer;
+      this.displaySystem = displaySystem;
+   }
+   
+   /* (non-Javadoc)
+    * @see com.jme.system.SystemProvider#getProviderIdentifier()
+    */
+   public String getProviderIdentifier() {
+      return DUMMY_SYSTEM_IDENTIFIER;
+   }
+
+   /* (non-Javadoc)
+    * @see com.jme.system.SystemProvider#getDisplaySystem()
+    */
+   public DisplaySystem getDisplaySystem() {
+      return displaySystem;
+   }
+
+   /* (non-Javadoc)
+    * @see com.jme.system.SystemProvider#getTimer()
+    */
+   public Timer getTimer() {
+      return timer;
+   }
+
+   /* (non-Javadoc)
+    * @see com.jme.system.SystemProvider#installLibs()
+    */
+   public void installLibs() {
+   }
+
+}

Thanks, admittedly DummyDisplaySystem hasn't gotten much love lately, and has probably fallen behind in regards to the new SystemProvider. The patch looks fine from initial first glance, I haven't tried using it. I'll play with it some on Monday.

I like the patch, but I would suggest we also move DummyDisplaySystem into com.jme.system.dummy as well.  It should not be where it is and has long needed to be moved anyway.