[C#] Door animation script in Unity. IF statement issue.

[DaveGreen93]

Hi guys,

 

Hoping someone here can help me with C# and Unity.

 

I'm working on a project and trying to animate a door I created in 3DS max. I've attached a script to a switch in the scene and press it. It isn't acting as I would expect.

 

Instead of playing the open animation, it just plays the close animation every time I interact with the switch.
Adding the Debug lines, shows that "First" is passed twice for every interaction and "Closed" and "Opened" are passed once each. I'm struggling to see why this is and hoping a fresh pair of eyes could spot the issue and suggest a solution.

using UnityEngine;
using System.Collections;

public class DoorSwitch : MonoBehaviour 
{
	public GameObject door;
	public GameObject button;
	public bool isDoorOpen = false;

	private GameObject player;

	// Use this for initialization
	void Awake () 
	{
		player = GameObject.FindGameObjectWithTag(Tags.player);
	}

	void OnTriggerStay (Collider other) 
	{
		if(other.gameObject == player)
			if(Input.GetButtonDown("Switch"))
				DoorToggle();
	}

	void DoorToggle()
	{
		Debug.Log("First");
		if(isDoorOpen == true)
		{
			isDoorOpen = false;
			door.animation.Play("Closing");
			button.renderer.material.color = Color.red;
			Debug.Log("Closed");
		}
		else if(isDoorOpen == false)
		{
			isDoorOpen = true;
			door.animation.Play("Opening");
			button.renderer.material.color = Color.green;
			Debug.Log("Opened");
		}
	}
}

[firey]

Looks fine to me.  The only thing I can think of is that the switch is being triggered twice.  You could add in a timeout that if the switch is hit within 3 seconds (or something) of the last hit you ignore it.

[Seabizkit]

yeah looks fine but my guess is that your creating a new DoorSwitch each time... i would need to understand how this is consumed to ensure that there is only one instance and the same instance of the DoorSwitch

[firey]

  On 13/04/2015 at 18:22, Seabizkit said:

yeah looks fine but my guess is that your creating a new DoorSwitch each time... i would need to understand how this is consumed to ensure that there is only one instance and the same instance of the DoorSwitch

Yea, was thinking that after too.  That maybe you are binding two cases in case of on/off but without seeing how you bind, it's hard to say for sure.

[DaveGreen93]

I've added these three screenshots.

 

The first shows the switch and the inspector, I've looked at all items in the hierarchy to make sure I haven't added the script to another object.

The second is the imported animations, not sure if there's any settings there that I should tweak. The same goes for the third screenshot of the door model in the hierarchy.
 

[DaveGreen93]

  On 13/04/2015 at 18:19, firey said:

Looks fine to me.  The only thing I can think of is that the switch is being triggered twice.  You could add in a timeout that if the switch is hit within 3 seconds (or something) of the last hit you ignore it.

 

Still finding my way around C# and Unity, any pointers to how I could make a timeout for the switch? I've tried reading up about Coroutines and IEnumerator, but I'm not sure how to implement it within the script.

[firey]

  On 13/04/2015 at 19:06, DaveGreen93 said:

Still finding my way around C# and Unity, any pointers to how I could make a timeout for the switch? I've tried reading up about Coroutines and IEnumerator, but I'm not sure how to implement it within the script.

I'm not entirely sure how unity works.  You could do it a couple ways, the simplest approach is to do something like this:

 

bool ignoreSwitch = false;
timer tmrIgnore;

protected void initialize() 
{
   tmrIgnore = new timer();
   tmrIgnore.Interval = 3000;
   tmrIgnore.Tick += new tmrIgnore_tick();
   tmrIgnore.Enabled = false;
}

protected void tmrIgnore_tick() 
{
    ignoreSwitch = false;
    tmrIgnore.Enabled = false;
}

protected void toggleDoor()
{
   if (!ignoreSwitch) {
      //run your code
      ignoreSwitch = true;
      tmrIgnore.Enabled = true;
   }
}

It's a really rough concept and by no means the best.  I mean ideally unity would give you ticks that you could just keep track of (ticks since last call).  However it should give you an idea.

[DaveGreen93]

  On 13/04/2015 at 19:29, firey said:

I'm not entirely sure how unity works.  You could do it a couple ways, the simplest approach is to do something like this:

 

bool ignoreSwitch = false;
timer tmrIgnore;

protected void initialize() 
{
   tmrIgnore = new timer();
   tmrIgnore.Interval = 3000;
   tmrIgnore.Tick += new tmrIgnore_tick();
   tmrIgnore.Enabled = false;
}

protected void tmrIgnore_tick() 
{
    ignoreSwitch = false;
    tmrIgnore.Enabled = false;
}

protected void toggleDoor()
{
   if (!ignoreSwitch) {
      //run your code
      ignoreSwitch = true;
      tmrIgnore.Enabled = true;
   }
}

It's a really rough concept and by no means the best.  I mean ideally unity would give you ticks that you could just keep track of (ticks since last call).  However it should give you an idea.

 

Hmmm, timer isn't included in Unity. Guess I'll have to research Coroutines more. Hopefully my lecturer can get back to me asap.

 

Thank you for your time and ideas firey.

[firey]

  On 13/04/2015 at 19:54, DaveGreen93 said:

Hmmm, timer isn't included in Unity. Guess I'll have to research Coroutines more. Hopefully my lecturer can get back to me asap.

 

Thank you for your time and ideas firey.

No problem, sorry I couldn't offer more assistance.  

[joddie191]

  On 13/04/2015 at 19:06, DaveGreen93 said:

Still finding my way around C# and Unity, any pointers to how I could make a timeout for the switch? I've tried reading up about Coroutines and IEnumerator, but I'm not sure how to implement it within the script.

 

I think a hint to the solution is in the documentation of your event:

 

  Quote

 

OnTriggerStay is called once per frame for every Collider other that is touching the trigger

 

keyword: Once per frame. You could try to add yield after the Call to your DoorToggle(), but I have not read up on how they work With events.  Or add a timestamp that cancels the Logic if the event is triggered within x milliseconds. 

[Eric Veteran]

If you just want to use a last used timer you only need a few lines:

 

        static readonly TimeSpan DoorDelay = TimeSpan.FromSeconds(3);
        DateTime lastDoorToggle = DateTime.MinValue;

        public void DoorToggle()
        {
            DateTime now = DateTime.Now;
            if (now.Subtract(lastDoorToggle) < DoorDelay)
                return;

            lastDoorToggle = now;

            Debug.Log("Door animation runs");
            // rest of door logic
        }

This way you have no timers running that you don't need.

[DaveGreen93]

  On 14/04/2015 at 14:58, Eric said:

If you just want to use a last used timer you only need a few lines:

 

        static readonly TimeSpan DoorDelay = TimeSpan.FromSeconds(3);
        DateTime lastDoorToggle = DateTime.MinValue;

        public void DoorToggle()
        {
            DateTime now = DateTime.Now;
            if (now.Subtract(lastDoorToggle) < DoorDelay)
                return;

            lastDoorToggle = now;

            Debug.Log("Door animation runs");
            // rest of door logic
        }

This way you have no timers running that you don't need.

 

 

Excellent, thank you Eric! After adding using System; it worked flawlessly. My lecturer was useless in helping.

 

Also, thank you everyone else for your suggestions.

[Seabizkit]

Hi DaveGreen93

 

Not that I care about unity but i do care about my understanding of c# logic.

 

Could you explain why this works and your original does not, as to me its in either one of two states..."open" or "closed".

 

If there is an instance of DoorSwitch per a door then i don't understand why the original logic doesn't work.

 

Could you explain how a timer fixes the issue.

 

Thanks

[Eric Veteran]

  On 14/04/2015 at 17:09, Seabizkit said:

Hi DaveGreen93

 

Not that I care about unity but i do care about my understanding of c# logic.

 

Could you explain why this works and your original does not, as to me its in either one of two states..."open" or "closed".

 

If there is an instance of DoorSwitch

per a door then i don't understand why the original logic doesn't work.

 

Could you explain how a timer fixes the issue.

 

Thanks

Behavior scripts in Unity are trigged for as long as the input is active so even a quick click on a switch could result in the event firing multiple times. A time delay ensures that the door routine doesn't toggle open/closed repeatedly for one click. I used a static 3s delay in my example but I suppose you could probably retrieve the animation time from the engine and use that instead.

[Seabizkit]

  On 14/04/2015 at 23:00, Eric said:

Behavior scripts in Unity are trigged for as long as the input is active so even a quick click on a switch could result in the event firing multiple times. A time delay ensures that the door routine doesn't toggle open/closed repeatedly for one click. I used a static 3s delay in my example but I suppose you could probably retrieve the animation time from the engine and use that instead.

 

Thank you!!! now it kinda makes sense its odd tho,

 

Is all game programming like this?

 

Wouldn't this consume large amounts of pointless CPU cycles?

 

Is it considered normal to have events fire like this?

 

how long is your animation for the door?

 

have you scripted it to that as its opening and its half way (assume 3 secs) and you click it again, does it start closing from the correct animation point.

 

I would assume you would need to code extra code for this... so that its starts the animation from the correct point?

 

PS door looks cool

 

PS re-read it. OK i see the click itself can be fired multiple time while holding it down... now i see why the delay was introduced its not for the animation but for time between allowed clicks...?

[Eric Veteran]

Well, it's DaveGreen's code, but yes to most of that. The "switch" just looks like a switch. In reality the engine is just detecting the player is colliding with the switch and generating a signal while they're touching. Using the time delay I posted will still cause it to generate the door event as long as you're touching the switch it just doesn't respond to it except every three seconds. It's similar to keyboard "debouncing."

Games do waste CPU cycles. The trick is to get it to a minimum so it runs smoothly. That's why I suggested the timestamp instead of a timer. The timer runs in the background.

[Seabizkit]

  On 15/04/2015 at 13:43, Eric said:

Well, it's DaveGreen's code, but yes to most of that. The "switch" just looks like a switch. In reality the engine is just detecting the player is colliding with the switch and generating a signal while they're touching. Using the time delay I posted will still cause it to generate the door event as long as you're touching the switch it just doesn't respond to it except every three seconds. It's similar to keyboard "debouncing."

Games do waste CPU cycles. The trick is to get it to a minimum so it runs smoothly. That's why I suggested the timestamp instead of a timer. The timer runs in the background.

 

Thanks Eric