• 0

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


Question

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");
		}
	}
}

16 answers to this question

Recommended Posts

  • 0

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.
  • 0

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.

  • 0
  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.

  • 0

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.
 

post-438852-0-67093800-1428949795.png

post-438852-0-43131100-1428949853.png

post-438852-0-48379000-1428949880.png

  • 0
  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.

  • 0
  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.

  • 0
  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.

  • 0
  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.  

  • 0
  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. 

  • 0
  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.

  • 0

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

  • 0
  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.

  • 0
  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...?

  • 0

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. :)

  • 0
  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

This topic is now closed to further replies.
  • Posts

    • I see you subscribe to the Linux/macOS approach to winning over converts...
    • As we get closer to October 2025 and the end of Windows 10 support, Windows 11 will pick up its pace and soon become the most popular desktop operating system worldwide. That's an assertion. It's also quite possible that the only growth will come from attrition as people and companies buy new machines that only come with Win11. It's also possible that we'll get a literal repeat of Win XP and Win 7, where a large number of users just waited until Microsoft gave in and fixed the core problems that the consumers were complaining about in Win Vista and Win 8 when they released Win 7 and Win 10.
    • It's significant growth for Linux considering the market share, so it could have had that effect I described.
    • Microsoft and Crowdstrike announce partnership on threat actor naming by Pradeep Viswanathan Whenever a cyberattack is discovered, companies disclose it to the public and assign it a unique name based on their internal procedures. Unfortunately, this leads to inconsistencies, as each company has its own naming conventions. As a result, the same threat actor behind a cyberattack may end up with multiple names, causing delays and confusion in response efforts. For example, a threat actor that Microsoft refers to as Midnight Blizzard might be known as Cozy Bear, APT29, or UNC2452 by other security vendors. To address this issue, Microsoft and CrowdStrike are teaming up. These companies will align their individual threat actor taxonomies to help security professionals respond to cyberattacks with greater clarity and confidence. It’s important to note that Microsoft and CrowdStrike are not attempting to create a single naming standard. Instead, they are releasing a mapping that lists common threat actors tracked by both companies, matched according to their respective taxonomies. The mapping also includes corresponding aliases from each group’s naming system. You can view the joint threat actor mapping by Microsoft and CrowdStrike here. Although this threat actor taxonomy mapping is a joint effort between Microsoft and CrowdStrike, Google/Mandiant and Palo Alto Networks' Unit 42 are expected to contribute to this initiative in the future. Vasu Jakkal, Corporate Vice President of Microsoft Security, wrote the following about this collaboration with CrowdStrike: As more organizations join this initiative, the collective defense against cyber threats will undoubtedly be improved.
  • Recent Achievements

    • First Post
      chriskinney317 earned a badge
      First Post
    • Week One Done
      Nullun earned a badge
      Week One Done
    • First Post
      sultangris earned a badge
      First Post
    • Reacting Well
      sultangris earned a badge
      Reacting Well
    • First Post
      ClarkB earned a badge
      First Post
  • Popular Contributors

    1. 1
      +primortal
      172
    2. 2
      ATLien_0
      125
    3. 3
      snowy owl
      122
    4. 4
      Xenon
      116
    5. 5
      +Edouard
      93
  • Tell a friend

    Love Neowin? Tell a friend!