AppState interface usage poll

The sdk will need java 13 or higher to fix some things. Oracle really screwed everything up with the decisions they made.

Forcing everyone to switch to jdk13 will kill a lot of us.

EDIT: Unless it’s just for the SDK in which case it’s no big deal

Just for the SDK because of netbeans, etc…

Me included. :slight_smile:

1 Like

I’m curiously interested in knowing why. The Oracle mess-up started with Java 9, I think… So upgrading to Java 8 should be fine and reccommended, as of 2019…

1 Like

I thought it was an “android is jdk 7” issue. To which you will get folks listing the 8 different forms of gymnastics you can perform that can make it work with 8… but still in the end android is jdk 7.

I had the understandin that android was only java 7 compatible but i found this code for gradle, i dont know if all the features of java 8 cant be used but it had help me a little

compileOptions {
    sourceCompatibility 1.8
    targetCompatibility 1.8
}

Edit: here is some info of whats compatible

Hey guys seems this post is slowly going out of topic :upside_down_face:

3 Likes

Note: part 1 of this refactoring has been completed on master and will thus be in any future 3.3 releases. That is AppState now has a getId() method. This is a breaking change for anyone implementing AppState directly.

Though as stated in the commit message, for those who are doing that, it’s worth a relook at extending AbstractAppState to insulate from future changes… and perhaps discover some places where the app state contract is being broken. For example, in making these changes, I found out that BulletAppState is breaking the isInitialized() contract and creating a potential for weird inconsistencies in what is reported and what AppStateManager thinks.

Anyway, next piece will be the AppStateManager changes to allow using this ID.

4 Likes

AppStateManager now automatically indexes AppStates with a non-null getId() value. New methods have been added for getting a state by ID.

I also added a few test cases. (As a side effect, I also added the ability to add groovy unit tests because writing Java unit tests makes little baby kittens cry.)

8 Likes

Thanks @pspeed

Regarding the stateForId method

Curious why you are not using the failOnMiss boolean approach instead?

To keep it consistent with this one.

Because the forX() (like Class.forName()) is more along the lines of what Java would do.

Technically, getState(Class) should have been findFirstState(Class) or similar. We’re deep into misnamed methods there.

In the case of ID, there will only ever be one state with that ID, or none. I decided to name the methods differently to better indicate this difference… since getState(Class) is already misnamed.

stateForId() is the method most callers should use. getState(id, class) should only be used in some specific conditional cases where an extra hasState(id) could be avoided. It’s provided for completeness but the number of use-cases where it should be used is really small.

For examples…

“My code depends on this other app state”:

stateManager.stateForId("someState").callAMethodOnIt();

“My code wants to do something with this other app state only if it exists”:

if( stateManager.hasState("someState") ) {
    stateManager.stateForId("someState").callAMethodOnIt();
}

A case where getState(id) might make sense:

AppState myState = stateManager.getState(id, AppState.class);
if( myState != null ) {
    stateManager.detach(myState);
}

But you do remind me that I should add a methods to BaseAppState for looking states up by ID.

Edit: and note, I could be persuaded to keep going down the bad naming path if the community feels really strongly about it. After all, JME naming is consistently bad in lots of places.

2 Likes

I see, thanks for the explanation. :slightly_smiling_face:

Personally I don’t see any great reasons to keep with current naming patterns - if anything, I wonder if it’s worth renaming poorly named methods now when there’s already a breaking change. As long as functionality is the same refactoring for renamed methods is easy to do.

Could we rename getId() and setId() to getName() and setName() please?

When a String is used to identify instances of a class, the getter is usually called getName().

For example:

  • com.jme3.scene.Spatial
  • com.jme3.renderer.ViewPort
  • com.jme3.texture.Texture
  • com.jme3.animation.Animation
  • com.jme3.animation.Bone
  • com.jme3.light.Light
  • com.jme3.material.MatParam
  • com.jme3.input.Joystick
  • com.jme3.input.controls.Trigger
  • com.jme3.anim.AnimClip
  • com.jme3.anim.Joint

That would indicate that it’s just merely a name and not an ID though. Names may not be unique, IDs should be by contract. If that is the case that contract should be enforced, too, so it does indeed act like an ID that can not contain duplicates.

2 Likes

In all of your examples, the name is just informational.

In this case, the string is a unique identifier and not a name. Thus I used the getId() naming.

Edit: well, in at least most of your examples. I think name in AnimClip is probably really “id”.

1 Like