Package Dependencies

archive development-discussion-jme2
1

I was using revjava to explore the package structure of jME, and noticed a lot of cyclic dependencies between the packages. I’m not anal about cyclic dependencies (e.g. Particle and ParticleManager), but I get the feeling that nobody’s looked at the bird’s eye view of jME in a little while:


Package [Package com.jme.curve] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.renderer] ]

Package [Package com.jme.effects] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.renderer] ]

Package [Package com.jme.image] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.renderer] ]

Package [Package com.jme.input] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.renderer], [Package com.jme.input.action], [Package com.jme.input.lwjgl], [Package com.jme.widget.input.mouse] ]

Package [Package com.jme.input.action] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.input] ]

Package [Package com.jme.input.lwjgl] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.input] ]

Package [Package com.jme.light] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.scene.state] ]

Package [Package com.jme.math] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.util] ]

Package [Package com.jme.renderer] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.util], [Package com.jme.scene.state], [Package com.jme.scene], [Package com.jme.effects], [Package com.jme.curve], [Package com.jme.input], [Package com.jme.widget], [Package com.jme.image] ]

Package [Package com.jme.scene] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.renderer], [Package com.jme.bounding], [Package com.jme.util], [Package com.jme.scene.state], [Package com.jme.scene.shape] ]

Package [Package com.jme.scene.shape] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.scene] ]

Package [Package com.jme.scene.state] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.renderer], [Package com.jme.light], [Package com.jme.scene] ]

Package [Package com.jme.sound] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.sound.scene], [Package com.jme.sound.joal], [Package com.jme.sound.lwjgl] ]

Package [Package com.jme.sound.joal] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.sound] ]

Package [Package com.jme.sound.lwjgl] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.sound] ]

Package [Package com.jme.sound.lwjgl] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.sound] ]

Package [Package com.jme.system] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.system.lwjgl], [Package com.jme.widget.font], [Package com.jme.util] ]

Package [Package com.jme.system.lwjgl] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.system] ]

Package [Package com.jme.util] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.system], [Package com.jme.math], [Package com.jme.scene], [Package com.jme.renderer], [Package com.jme.util.lwjgl] ]

Package [Package com.jme.util.lwjgl] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.util] ]

Package [Package com.jme.widget] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.widget.border], [Package com.jme.renderer], [Package com.jme.widget.layout] ]

Package [Package com.jme.widget.border] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.widget] ]

Package [Package com.jme.widget.font] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.system] ]

Package [Package com.jme.widget.impl.lwjgl] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.widget.renderer] ]

Package [Package com.jme.widget.input.mouse] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.input] ]

Package [Package com.jme.widget.layout] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.widget] ]

Package [Package com.jme.widget.panel] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.widget.renderer], [Package com.jme.widget.scroller] ]

Package [Package com.jme.widget.renderer] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.widget.impl.lwjgl], [Package com.jme.widget.panel], [Package com.jme.widget.text], [Package com.jme.widget.scroller], [Package com.jme.widget.slider] ]

Package [Package com.jme.widget.scroller] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.widget.renderer], [Package com.jme.widget.panel] ]

Package [Package com.jme.widget.slider] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.widget.renderer] ]

Package [Package com.jme.widget.text] may be involved in cyclic dependencies. It knows of and is known by the following packages: [ [Package com.jme.widget.renderer] ]

2

Well, I’m not quite sure what to say to this. I’m not sure if this is an issue or not. Most of the dependancies are within Renderer. This is how the design is, when a node is drawn it is given a Renderer to display to. Render properties are initialized, then the node requests that the renderer draw it.



So, yes, nodes import Renderer and Renderer knows about different nodes.

3

It’s not an issue in and of itself. When I first looked at my fledgling LWJGL-based game’s code this way it was in a similar state, and a lot of it was easily rectified by moving a few methods and classes.



From my perspective, the issue is just one of unit testing - if the code has enough cyclic dependencies, it’s hard to instantiate classes in isolation. I’ll be more specific as I get into jME more.



On the subject or Renderer and Nodes, I think the cycle stems from the fact that the scene graph is partly library-specific, and partly generic. The Nodes are generic, but the RenderStates they contain are specific. I haven’t read Eberly’s book - is this hybrid approach deliberate or accidental?



Both approaches have their appeal. A library-specific scene graph can render itself (e.g. LWJGLTextureState renders itself, which seems like a cleaner approach than the many draw() methods in LWJGLRenderer).



On the other hand, a totally generic scene graph can be instantiated, tested, serialized and otherwise manipulated without having to initialize any particular display system. This approach seems more flexible - imagine a jME-based tool that loads a scene graph then annotates it with light maps and BSP information (possibly by outputting replacement nodes and textures) - it would be pretty silly if that only worked with an OpenGL window open!



As a more immediate example, a fully generic scene graph would make it easy to test whether model loaders are working correctly without making the test dependent on a specific renderer implementation.



I hope I’m not coming across as too audacious. “Hi! I’m new… here’s how you should completely rewrite your code!”

4

I can see how this might make unit testing somewhat difficult. But this design is quite deliberate. It’s due to the fact that the scene tree must be parsed, each node is required to set up it’s state and call draw on itself. The draw method of the Renderer must then render the data. This allows the Renderer method to contain all the library specific code, so it will be easy to swap out renderers.



However, maybe something like TriMesh (LWJGLTriMesh which contains it’s own draw method) would be possible. But that will be a major can of worms when we have to deal with passing cameras around, creating a concrete class implicitly, etc. But I definately will look at it.


I hope I'm not coming across as too audacious. "Hi! I'm new.. here's how you should completely rewrite your code!"


Not at all. This is how it will become better.
5

Well, for now, perhaps the simplest way to get unit testing started would be to create a TestDisplaySystem and a TestRenderer, whose factory methods return plain RenderState objects (e.g. TextureState). I’ll do this if that’s acceptable to you. That might be all that’s necessary to get windowless tests running.



By the way, would it be more appropriate to name factory methods as createSomething or newSomething rather than just getSomething? They just look like accessors, which confused me on a couple of occasions. DisplaySystem.getRenderer(String) does a lot more than just retrieve something

6

While perhaps not the best naming, it is not without precedent in Java. For example, Image.getGraphics or Document.getDOMImplementation. That said, "create" would be my choice if we did change it. :slight_smile:

7

I suppose getStuff does make [some] sense in context. You’re getting a specific implementation of a component, such as a LWJGLDisplaySystem. Also “get” is a bit faster to type - something everyone can appreciate. Though, should we eventually break out the ol’ refactoring tools, I prefer “create”.

8

Let’s focus on functionality at the moment… there is way too much to do right now to worry about symantics. There is a whole release target that will take care of this kind of thing in the future. And it’s not something that we want to just change on a whim, because it is a public interface. Personally, I don’t have a problem at all with get, and see no benefit with create, other than the joy of extra typing.