- Home /
Why is an action in my "if" block executing in the "else" conditions?
I'm having a weird issue, that I'm sure is user error... I have a tree of if statements to determine how a prompt GUI should be worded, and it also determines what happens if you push a button. Most of my conditions seem to work totally fine, but for some reason one of them is acting strangely. I have, nested in an "else" block, an "if" and "else" block. But for some reason, when I create the scenario that would present me with the "else" statement's conditions, and I press the D Key, it executes my code in both the "if" and "else" blocks. Here's the code:
else{
if(!emptyPlanet){
Debug.Log("Introduce to new friend GUI activated");
GUI.Label(Rect(0,LTT, Screen.width, LTH),"Press D to introduce your cargo to a new friend.\n\nPress Spacebar to take him/her elsewhere.");
if(Input.GetKeyDown(KeyCode.D)){
emptyPlanet = false; //is this redundant once isActive = false?
carryOver.SadCatman.SlotB = Cargo;
carryOver.SadCatman.isActive = false;
carryOver.currentCargo = "empty";
//load "new home" avatars for both aliens
Debug.Log("You shouldn't be reading this if the planet was empty before the delivery!");
}
}
else{
Debug.Log("Make this alien's new home GUI activated");
GUI.Label(Rect(0,LTT, Screen.width, LTH),"Press D to make this planet your cargo’s new home.\n\nPress Spacebar to take him/her elsewhere.");
if(Input.GetKeyDown(KeyCode.D)){
carryOver.SadCatman.SlotA = Cargo;
carryOver.SadCatman.isActive = false;
//load "new home" avatar for alien in Slot A
carryOver.currentCargo = "empty";
emptyPlanet = false; //is this redundant once isActive = false?
}
}
}
Let me know if you need to see the whole "if" tree block. I've already tried swapping the contents of the if and else blocks and made the "if" read if(emptyPlanet) instead of looking for the inverse to be true, but it still produces the dual results.
What am I not catching?
Answer by Bunny83 · Oct 07, 2014 at 03:08 AM
The reason is pretty simple ;) You have this code in OnGUI. OnGUI is called for multiple reasons:
layouting
repainting
input events
The layout and the repaint event are fired once every frame. Input events such as MouseDown, KeyDown, KeyUp, .... are of course only fired once. Your problem is that you use the Input class within OnGUI. The Input class is frame based, so during one frame the input has a certain state.
So if you press the "D" key, Input.GetKeyDown(KeyCode.D) will return true until the next frame. Since OnGUI is called at least two times per frame you process your code two times. Since you change your if condition when you process your code for the first time, the second time it will execute the else block, all within the same frame.
So you simply shouldn't use the Input class in OnGUI or if you do, make sure you only process it in one of the events. However in OnGUI you usually use the Event class to process Mouse or keyboard events:
// I guess you use UnityScript?
var e = Event.current;
if(e.type == EventType.KeyDown && e.keyCode == KeyCode.D){
Note: the Event system handles key input differently. It wraps the systems keyboard input. That means if you hold down a key, the key is repeated at the systems repeatrate.
If you want to use Input.GetKeyDown, do something like this:
var e = Event.current;
if(e.type == EventType.Repaint && Input.GetKeyDown(KeyCode.D)){
This ensures that you only process the input during the repaint event.
You were right on the money, Bunny. Thank you so much!
I tried to incorporate your solution directly but got a couple errors, including asking me to declare the e variable in either Start() or Awake(), so I declared:
var e : Event;
at the top of the script, then:
e = Event.current;
in Awake(), and finally used your revised if() statement where $$anonymous$$e was in OnGUI(). It indeed cleared up my initial problem that I posted about, and now the only error message I'm still getting is
NullReferenceException: Object reference not set to an instance of an object
which points me to the line of code with the revised "if" statement on it. I'm assu$$anonymous$$g it's referring to the "e" object, but I can't figure out why exactly. It only gives me the error after the button push event occurs, not before.
Thoughts?
Never, ever use else in OnGUI.
If at all possible shift your logic to update and send data back and forwards with variables.
Edit: As indicated in the further comments this is not very good advice. Better advice would be to use caution with else in OnGUI and consider moving logic out of OnGUI. Leaving the bad advice up so the rest of the comments still make sense.
@slampants: Since your code contains GUI elements you can only execute it from OnGUI. You can't use GUI stuff in Update. The Event class is also only valid within OnGUI. So i might not have beed clear enough here. $$anonymous$$y solution of course also needs to be placed within OnGUI.
If you want to split your logic from your GUI, you can do the input stuff in update and your GUI stuff in OnGUI. $$anonymous$$y solution was to keep everything inside OnGUI.
@Bored$$anonymous$$ormon: That's a strange advice to not use "else" in OnGUI. Yeah, input or other frame based stuff is usually better handled in Update but it's still possible in OnGUI. OnGUI is actually much more versatile than Update. You just have to understand how closely related it's with the Event class. They always come in pairs :)
@Bunny83 yes, sorry, I understood that you meant for the code to be in OnGUI. But when I initially had everything existing solely in OnGUI, I got an error that the declaration of the "Event" had to happen in Start() or Awake(), hence me changing things up a bit. But the if(e.type == EventType.Repaint && Input.Get$$anonymous$$eyDown($$anonymous$$eyCode.D)) bit is definitely in OnGUI.
Any idea why I might be getting the NullReferenceException?
----UPDATE----
I don't know why it didn't work the first time, but I reverted back to your original answer telling me to declare the "e" variable in OnGUI, and it's working fine. Thanks again, @Bunny83!
@Bored$$anonymous$$ormon: It simply depends on your condition. So if you for example switch between two different GUIs or even when you simply handle all other events it's perfectly fine to use else. The behaviour isn't undefined as it is clear when the else block is executed.
Answer by shadbags · Oct 07, 2014 at 02:51 AM
Definitely handy to see the whole tree, probably post it to pastebin.
Are you absolutely sure that it's running these blocks? IE: A breakpoint in each of them gets hit in the same turn? Because what I suspect is that this is actually happening is it's doing both, but not in the way you think it is.
Notice how the else block has the final line emptyplanet = false
?
If this is in update, what is happening is that the first time this if block fires, and emptyplanet is true, the ELSE block runs. The last line sets emptyplanet to false, and then 16ms later (at 60fps) the D key is likely still held down, so it runs through the same if block, but now does the other half of the if statement.
Few possible solutions:
Add a flag for the keypress so it can only fire once.
Force a delay between possible key events.
Your answer
Follow this Question
Related Questions
error CS0103 How do you fix this? 0 Answers
I'm wondering with if function working 2 Answers
MouseLook half disabling 1 Answer
Power-Ups scripting error 1 Answer