I create BDialogBox manually with YES_NO options. In DialogListener I only process YES option. When I choose NO as answer, I get enum crash and it looks like bug to me, but I might be constructing object wrongly. If I do, it would be nice to crash during construction then
Oh, and one more thing (almost forgot). When packing dialog using pack() or its overload, I get nullpointer exc?
Construction code:
BDialogBox dialogBox = new BDialogBox(
"dialogbox",
new BTitleBar("titlebar", "Huh?", TitleOptions.NONE),
new BDialogMessage("dialogmessage", "Do you really wanna quit, wimpy?", IconOptions.QUESTION, DisplayStyleOptions.WINDOWS),
DialogOptions.YES_NO,
TcgUI.getGameStylesheet());
dialogBox.setSize(300,200); // <-- if I pack here, it crashes with nullpointer
dialogBox.center();
dialogBox.setDialogListener(new DialogListener() {
public void responseAvailable(UserResponse userResponse, BComponent bComponent) {
if (userResponse.equals(UserResponse.YES))
TcgGame.finishGame();
}
});
BuiSystem.getRootNode().addWindow(dialogBox);
Error:
SEVERE: Exception in game loop
java.lang.IllegalArgumentException: No enum const class com.jmex.bui.UserResponse.No
at java.lang.Enum.valueOf(Enum.java:192)
at com.jmex.bui.UserResponse.valueOf(UserResponse.java:8)
at com.jmex.bui.BDialogBox.fireResponse(BDialogBox.java:59)
at com.jmex.bui.BDialogBox.access$000(BDialogBox.java:30)
at com.jmex.bui.BDialogBox$1.actionPerformed(BDialogBox.java:52)
at com.jmex.bui.event.ActionEvent.dispatch(ActionEvent.java:47)
at com.jmex.bui.BComponent.dispatchEvent(BComponent.java:718)
at com.jmex.bui.BButton.dispatchEvent(BButton.java:169)
at com.jmex.bui.BRootNode.dispatchEvent(BRootNode.java:459)
at com.jmex.bui.BComponent.emitEvent(BComponent.java:972)
at com.jmex.bui.BButton.fireAction(BButton.java:210)
at com.jmex.bui.BButton.dispatchEvent(BButton.java:150)
at com.jmex.bui.BRootNode.dispatchEvent(BRootNode.java:459)
at com.jmex.bui.EventRootNode$2.dispatchEvent(EventRootNode.java:322)
at com.jmex.bui.EventRootNode$2.onButton(EventRootNode.java:276)
at com.jme.input.lwjgl.LWJGLMouseInput.update(Unknown Source)
at com.jme.input.InputSystem.update(Unknown Source)
at com.jme.app.FixedFramerateGame.start(Unknown Source)
at com.funcom.tcg.client.TcgJme.start(TcgJme.java:65)
at com.funcom.tcg.client.TcgJme.main(TcgJme.java:76)
at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
at java.lang.reflect.Method.invoke(Method.java:597)
at com.intellij.rt.execution.application.AppMain.main(AppMain.java:90)
I just spent some interesting time stepping through the code, and think I might have actually found a bug in Java but I'll leave that call to others with more experience. When I ran your code, I received the exception whether I pressed the Yes or No buttons. It looks like the enum.valueOf() method isn't returning the proper value according to the JDK documentation.
The current UserResponse.java code:
package com.jmex.bui;
/**
The possible user responses for the standard dialog.
*
@author Lucian Cristian Beskid
*/
public enum UserResponse {
NONE("None"),
YES("Yes"),
NO("No"),
OK("OK"),
CANCEL("Cancel");
private String label;
private UserResponse(String label) {
this.label = label;
}
@Override
public String toString() {
return label;
}
}
The call to UserResponse.valueOf("Yes") should return UserResponse.YES. However, it's not, and we're getting the IllegalArgumentException. If you change all the string values to match the casing of the enumerated values, as follows:
package com.jmex.bui;
/**
The possible user responses for the standard dialog.
*
@author Lucian Cristian Beskid
*/
public enum UserResponse {
NONE("NONE"),
YES("YES"),
NO("NO"),
OK("OK"),
CANCEL("CANCEL");
private String label;
private UserResponse(String label) {
this.label = label;
}
@Override
public String toString() {
return label;
}
}
then your code should work as expected. Note that the AllDialogsTest.java appears to work merely as a coincidence as it uses only the "OK" button, which already matched the casing. If you change the DialogBoxUtil.java to use the YES_NO option instead of OK, you'll see the same error. So I think it's an error in Java, but you can work around it for now by modifying the GBUI source. Hopefully you're working from the SVN rather than the pre-compiled jars anyway. If not, and you're using Eclipse, check out the tutorial I just added in the wiki (under the jME2 section) for getting up and running.
If you put the call to pack() AFTER the addWindow call (the last line of the code you included in your post), it should work as expected. I know the call fails before then because the TextFactory is null when trying to calculate the size required for the text, but I can't remember the root cause of that: I got that far tracking down a different error, and didn't need to dig any further.
Hm, correct me if I'm wrong, but you might misunderstood the JDK documentation.
For a start, beside one ugly hack (which is documented) they have in Swing rendering code, I doubt that JDK has bugs.
Enum.valueOf(String) returns constant instance only if you type exactly the name of constant language artifact. The constant constructor parameter in parenthesis is optional and a part of custom enum code, and if you decide that your constants do not carry any extra data, valueOf() method wouldn't work. So, in original enum, calling valueOf("Yes") actually should throw IllegalArgumentException.
That's actually the bug in gbui code (asking for constant by its toString() value),
so you actually did discover it It should be fixed by replacing valueOf calls with exact constant names. Although, imho is that this bug discovers that might not be the best approach.
From JDK perspective, relying on user enum data entry and toString() method to ensure vital enum static functions work - would be quite huge code smell :).
@Sfera or @standtrooper, how about just keeping string constant in the BDialogBox? If you're using valueOf to access enum from an ActionEvent which I'm guessing you are, you could instead use the constant to configure action itself so you wouldn't need to sync code in enum and action fire code.
@mjsimpson, again, if I misunderstood what are you saying, please do correct me.
@deus_ex_machina – that's a good catch… and, BTW, the JDK does have bugs… not many, but they are there:) However, in this case I believe you are correct. There should have been an error thrown on the valueOf("Yes").
I'll make that patch today… I've now got three or four things going up later today for GBUI.
I've given this some thought. Let me know what you guys think, but I think that the better way to go about the fix for this is to remove the String label… a valueOf is going to give us the Enum Object… and a toString() will give us the value of that object… the only thing left would be to add some static util methods to the Enum… like a toDisplay(UserResponse) that would then toString() and turn UserResponse.YES into "Yes" for Display… that also means that OK would now be Ok.
Does anyone have any objections or do you have any suggestions for another way that you'd like to see this done?
Well, I offered alternative way in my previous post
But, my imho is just that enums as type constants, although sometimes cool, are C construct and I've experienced that they often smell up code rather than improve the design of it. I try not to use them and either use static finals of most primitive type I can use in the class or some arbiter class.
It is way too easy to turn enum into a class-like entity, so I'd rather have a class instead.
I'll kind of agree with Deux: I like the use of enums as defining all the valid values (rather than static finals), but I've never liked the use of them as anything more: I view methods added to an enum as a sign that something has gone awry. In my book, data with methods is a class, not an enum or struct.
In the end though, I don't really have a preference for the implementation, as long as it works, and the interface continues to hide it.
I tend to agree with both of you, and in this instance I was trying to do the wrong thing by providing a quick solution to a problem rather than actually address the problem.
I usually use Enums to give me a static list of options that don't change. I have and do use Enums with values different from the UpperCase value. I could call these Classes, with you mj, and if you're going to give the value an int value different than an enumerated constant or String value different than the String name then it might make sense, otherwise there are methods to get those values.
I'll refrain from making these changes and confer with Sfera and go through the code one more time when I'm not so tired, as I was last night and right now.
afaik vauelOf() is intended to return the enum member if the string matches the enum member name. that label was there with an entirely other purpose (like displaying the text on the button). so if valueOf("No") fails, that's imho entirely correct(vauleOf.("NO") should have worked nevertheless). please correct me if i'm wrong.
