Bug: NiftyJmeDisplay is not ”reentrant”

In the process of upgrading to a recent jME version, I found out that the behavoir of NiftyJmeDisplay changed with respect to input handling: now on cleanup() (triggered by detaching this processor from the viewport) it removes it’s RawInputListener from InputManager which is great.



But when I attach my instance of NiftyJmeDisplay again it does not recieve any inputs because it only adds as RawInputListener once in the constructor but IMHO it should do that in initialize() to allow attach/detach without inconsistencies - is this a bug and if yes, could you fix that please?

@cmur2 said:
In the process of upgrading to a recent jME version, I found out that the behavoir of NiftyJmeDisplay changed with respect to input handling: now on cleanup() (triggered by detaching this processor from the viewport) it removes it's RawInputListener from InputManager which is great.

But when I attach my instance of NiftyJmeDisplay again it does not recieve any inputs because it only adds as RawInputListener once in the constructor but IMHO it should do that in initialize() to allow attach/detach without inconsistencies - is this a bug and if yes, could you fix that please?


I would call it a bug. I agree with the solution. I do not have time to fix it personally but hopefully the one who broke it can swing back and fix it.

In the mean time, you could maybe just create a new NiftyJmeDisplay, I guess.

Thank you, would it speed up the process if I submit a patch?



I worked around it by subclassing the NiftyJmeDisplay and implementing the fix there:



[java]

public class ReentrantNiftyJmeDisplay extends NiftyJmeDisplay {



// avoid double addition of the listener after constructor call

private boolean rawInputListenerAdded;



public ReentrantNiftyJmeDisplay(AssetManager assetManager,

InputManager inputManager,

AudioRenderer audioRenderer,

ViewPort vp)

{

super(assetManager, inputManager, audioRenderer, vp);



rawInputListenerAdded = true;

}



@Override

public void initialize(RenderManager rm, ViewPort vp) {

// the fix

if(inputManager != null && !rawInputListenerAdded) {

inputManager.addRawInputListener(inputSys);

}



super.initialize(rm, vp);

}



@Override

public void cleanup() {

super.cleanup();



rawInputListenerAdded = false;

}

}

[/java]

Bump

Why does it need the boolean flag? initialize should only be called once anyway… ?



Edit: Oh I see, you need it because you are subclassing. The proper fix would be to just move it in the NJD class.

However I just looked at the code and I don’t see where cleanup removes the rawInputListener.

[java]



public void cleanup() {

inited = false;

inputSys.reset();

// nifty.exit();

}

[/java]

Jepp, I would appreciate it very much if it’s fixed soon :slight_smile:

Öhm, in NiftyJmeDisplay.java from 17.11.2012 (SVN):



[java]

public void cleanup() {

inited = false;

inputSys.reset();

if (inputManager != null) {

inputManager.removeRawInputListener(inputSys);

}

// nifty.exit();

}

[/java]



Or has this changed meanwhile?

Yeah, my bad. Nov 10th that change was made by @momoko_fan to fix issue 460 but I didn’t have the change in my local.



Should be easy enough to fix I expect but I don’t want to touch it when someone has been working on it so recently.

Ok, momoko_fan has committed a fix to SVN. Should be fine in tonight’s nightly.

No problem, thanks to the availability of a not-so-hacky-workaround it’s not an urgent bug for me… thanks for your investigation



Edit: Oh great!