Greetings,
I'm currently doing a bit of refactoring in a project, organizing our GameStates in tree structures.
Currently I am having some trouble doing the design and wanted to discuss some questions and suggestions about the current implementation of GameStates and GameStateNodes.
- Tree structures are used most effectively if all elements in the node can be treated uniformly (see "Composite" design pattern). Is there any particular reason to have GameStates (which are not GameStateNodes) at all, apart from backward compatibility?
- When using BasicGameStateNode, I cannot attach a JMEDesktopState, because BasicGameStateNode extends GameStateNode<BasicGameState> and JMEDesktopState is not instanceof BasicGameState. This is rather unfortunate. I am missing the treating-nodes-uniformly aspect once again.
Comments?
Concerning the compatibility with JMEDesktopState:
Is there any particular reason, why JMEDesktopState does not extend BasicGameState instead of GameState or alternatively BasicGameStateNode extends GameStateNode<GameState> instead of GameStateNode<BasicGameState>?
Is there any functionality (that I am missing) that reqires JMEDesktopState and BasicGameState being treated differently?
i think it would make sense when jMEDesktopState extends BasicGamestate, since jMEDesktopState also creates a root node and implements the Method getRootNode().
The root node in jMEDesktopstate is not used at all anyway by default.
This would be the patch to extend BasicGamestate.
Index: src/com/jmex/awt/swingui/JMEDesktopState.java
===================================================================
RCS file: /cvs/jme/src/com/jmex/awt/swingui/JMEDesktopState.java,v
retrieving revision 1.7
diff -u -r1.7 JMEDesktopState.java
--- src/com/jmex/awt/swingui/JMEDesktopState.java 14 Sep 2007 01:37:04 -0000 1.7
+++ src/com/jmex/awt/swingui/JMEDesktopState.java 5 Apr 2008 17:47:00 -0000
@@ -45,12 +45,12 @@
import com.jme.scene.state.LightState;
import com.jme.system.DisplaySystem;
import com.jme.util.GameTaskQueueManager;
-import com.jmex.game.state.GameState;
+import com.jmex.game.state.BasicGameState;
/**
* @author Matthew D. Hicks
*/
-public class JMEDesktopState extends GameState {
+public class JMEDesktopState extends BasicGameState {
private static final Logger logger = Logger.getLogger(JMEDesktopState.class
.getName());
@@ -58,7 +58,6 @@
private Node guiNode;
private InputHandler guiInput;
private JMEDesktop desktop;
- private Node rootNode;
private int width;
private int height;
@@ -68,12 +67,14 @@
}
public JMEDesktopState(int width, int height) {
+ super("jMEDesktop");
this.width = width;
this.height = height;
init();
}
public JMEDesktopState(boolean variableDesktopSize) {
+ super("jMEDesktop");
this.variableDesktopSize = variableDesktopSize;
init();
}
@@ -124,24 +125,18 @@
public void update(float tpf) {
guiInput.update(tpf);
-
guiNode.updateGeometricState(tpf, true);
- rootNode.updateGeometricState(tpf, true);
+ super.update(tpf);
}
public void render(float tpf) {
DisplaySystem.getDisplaySystem().getRenderer().draw(guiNode);
- DisplaySystem.getDisplaySystem().getRenderer().draw(rootNode);
}
public void cleanup() {
desktop.dispose();
}
- public Node getRootNode() {
- return rootNode;
- }
-
public Node getGUINode() {
return guiNode;
}
I would absolutely appreciate that being changed.
Is there anybody out there (like, um, darkfrog, maybe? ;)) who can tell if this is going to be patched or not so I can decide whether to write an Adapter for JMEDesktopState or just to wait?
Thanks.
Um, there's one more thing… Is that right that I cannot attach BasicGameStateNodes to BasicGameStateNodes, because BasicGameStateNode !instanceof BasicGameState. That seems to be some kind of the same problem. Buiding tree structures with the BaiscGameState{Node}-Implementations is still rather impossible…
I've went a bit more into it and suspect that BasicGameStateNode should not parametrize GameStateNode with BasicGameState when extending.
In the comments describing BasicGameStateNode it says:
BasicGameStateNode this is identical to BasicGameState except it allows you to add additional GameStates as children beneath it.
But unfortunately this is not true. It does not allow you to have GameState children. It restricts you to BasicGameStates.
But there is a weakness in the class structure: while GameStateNode extends GameState, BasicGameStateNode does
not extend BasicGameState. So one cannot build trees using BasicGameStateNode.
I think that BasicGameStateNode sould set the Parameter to GameState (since it does not depend on the getRootNode()-implementation) or not set it at all...
Patches would be
I) for leaving GameStateNode unparametrized (probably the better solution)
Index: src/com/jmex/game/state/BasicGameStateNode.java
===================================================================
RCS file: /cvs/jme/src/com/jmex/game/state/BasicGameStateNode.java,v
retrieving revision 1.3
diff -u -r1.3 BasicGameStateNode.java
--- src/com/jmex/game/state/BasicGameStateNode.java 20 Sep 2007 15:15:01 -0000 1.3
+++ src/com/jmex/game/state/BasicGameStateNode.java 7 Apr 2008 20:08:33 -0000
@@ -41,7 +41,7 @@
*
* @author Matthew D. Hicks
*/
-public class BasicGameStateNode extends GameStateNode<BasicGameState> {
+public class BasicGameStateNode<G extends GameState> extends GameStateNode<G> {
/** The root of this GameStates scenegraph. */
protected Node rootNode;
II) for parametrizing with GameState
Index: src/com/jmex/game/state/BasicGameStateNode.java
===================================================================
RCS file: /cvs/jme/src/com/jmex/game/state/BasicGameStateNode.java,v
retrieving revision 1.3
diff -u -r1.3 BasicGameStateNode.java
--- src/com/jmex/game/state/BasicGameStateNode.java 20 Sep 2007 15:15:01 -0000 1.3
+++ src/com/jmex/game/state/BasicGameStateNode.java 7 Apr 2008 20:05:16 -0000
@@ -41,7 +41,7 @@
*
* @author Matthew D. Hicks
*/
-public class BasicGameStateNode extends GameStateNode<BasicGameState> {
+public class BasicGameStateNode extends GameStateNode<GameState> {
/** The root of this GameStates scenegraph. */
protected Node rootNode;
Anyone know any reason why these changes shouldn't be applied?
Just to summarize: It is not possible to build trees with the depth greater 1 with BasicGameStateNodes, because BasicGameStateNodes may not be attached to BasicGameStateNodes.
Here is the class hierarchy of the GameStates and Basic*-implementations.
The key point here is that the extends-relationship “GameStateNode extends GameState” is not mirrored with BasicGameState and BasicGameStateNode. Therefore BasicGameStateNode is-not-a BasicGameState.
A tree structure like the following one may not be modelled because a BasicGameStateNode may not be attached to a BasicGameStateNode (highlighted):
I would actually suggest leaving the Parameter unbound when extending GameStateNode (possibility “I” from above).
darkfrog said:
Anyone know any reason why these changes shouldn't be applied?
From what that tell me, Darkfrog dont know UML or can no answer problem
Or hasn't taken the time to look through the code for himself…
So one interesting question remains: Will you patch it? 
I'll see if I can't get a look at it tonight…