Search Unity

  1. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  2. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

better code for this?

Discussion in 'Scripting' started by Mike, Jul 21, 2007.

  1. Mike

    Mike

    Joined:
    Dec 1, 2006
    Posts:
    118
    Hi!

    Please help me with this... how could I write that better/cleaner:

    Code (csharp):
    1. function FixedUpdate () {
    2.  
    3.  
    4. var hit : RaycastHit;
    5.  
    6.  
    7. if (Input.GetKey ("e")  Physics.Raycast(Camera.main.ViewportPointToRay(Vector3(0.5, 0.5, 0.0)), hit,5) ){
    8. if (hit.rigidbody  hit.rigidbody.CompareTag ("door")) {
    9.  
    10. hit.rigidbody.AddRelativeForce(-5,0,0);
    11.  
    12. }
    13. }
    14. }
    15.  
    16.  
    The user can press the "e" key to open doors.
    Can I somehow improve the second "if" statement? I need that to check if the rigidbody that got hit by the raycast is tagged with "door".

    Thanks a lot,
    Mike
     
  2. Daniel_Brauer

    Daniel_Brauer

    Unity Technologies

    Joined:
    Aug 11, 2006
    Posts:
    3,355
    Looks pretty good to me. The only things I'd do are:

    Put it in Update() rather than FixedUpdate()
    FixedUpdate() tends to be called more often, and unless you're interested in applying forces every frame, Update() is better for input.

    Make the "Activate" key a virtual one instead of a fixed one
    This will allow players to change the key, which is always nice. Just go to Edit>Project Settings>Input and add "Activate". Look at other virtual key configurations for examples of what your settings should look like.

    Code (csharp):
    1. function Update() {
    2.  
    3.     var hit : RaycastHit;
    4.  
    5.     if (Input.GetButtonDown("Activate")  Physics.Raycast(Camera.main.ViewportPointToRay(Vector3(0.5, 0.5, 0.0)), hit,5))
    6.         if (hit.rigidbody  hit.rigidbody.CompareTag("door"))
    7.             hit.rigidbody.AddRelativeForce(-5,0,0);
    8. }
     
  3. Eric5h5

    Eric5h5

    Volunteer Moderator Moderator

    Joined:
    Jul 19, 2006
    Posts:
    32,401
    I would also suggest

    Code (csharp):
    1.  
    2.      if (Input.GetButtonDown("Activate") ) {
    3.           if (Physics.Raycast( // etc.
    4.  
    That way, you're only evaluating whether the key was pressed every frame, and only doing the raycast if so. Doing them both in the same statement means both have to be evaluated every frame. Not that it would make a huge difference in this case, but it's the principle....

    --Eric
     
  4. seon

    seon

    Joined:
    Jan 10, 2007
    Posts:
    1,441
    Id also only put the

    Code (csharp):
    1. var hit : RaycastHit;
    inside the first if statement, so it only needs to be declared IF the key is pressed, unless you plan on using the variable elsewhere in this script.
     
  5. Daniel_Brauer

    Daniel_Brauer

    Unity Technologies

    Joined:
    Aug 11, 2006
    Posts:
    3,355
    Most compilers (including Unity's) do not evaluate the left-hand side of if the right is false.
     
  6. Eric5h5

    Eric5h5

    Volunteer Moderator Moderator

    Joined:
    Jul 19, 2006
    Posts:
    32,401
    That's good to know, but if that's the case, shouldn't the button check and the raycast then be reversed?

    --Eric
     
  7. Daniel_Brauer

    Daniel_Brauer

    Unity Technologies

    Joined:
    Aug 11, 2006
    Posts:
    3,355
    Ugh, got my sides mixed up. Right does not get evaluated if left is false.
     
  8. freyr

    freyr

    Joined:
    Apr 7, 2005
    Posts:
    1,148
    That's not a good idea. Any physics manipulations should go into FixedUpdate and never in Update.
     
  9. Mike

    Mike

    Joined:
    Dec 1, 2006
    Posts:
    118
    Wow, thanks for all the answers !

    I'm always scared that someone who sees my code will finally find out that all the programming lessons at university were in vain :D
    And the one who WILL see that code is my JS+JAVA teacher cause he'll evaluate my project... damn !

    so, apart from the virtual key ... can I leave this as is?
     
  10. seon

    seon

    Joined:
    Jan 10, 2007
    Posts:
    1,441
    Code (csharp):
    1. function FixedUpdate () {
    2.  
    3.    if (Input.GetButtonDown("Activate") ) {
    4.         var hit : RaycastHit;
    5.         if (Physics.Raycast(Camera.main.ViewportPointToRay(Vector3(0.5, 0.5, 0.0)), hit,5) ){
    6.             if (hit.rigidbody  hit.rigidbody.CompareTag ("door")) {
    7.                 hit.rigidbody.AddRelativeForce(-5,0,0);
    8.             }
    9.         }
    10.     }
    11. }
     
  11. Daniel_Brauer

    Daniel_Brauer

    Unity Technologies

    Joined:
    Aug 11, 2006
    Posts:
    3,355
    This is news to me. Even impulses?
     
  12. pete

    pete

    Joined:
    Jul 21, 2005
    Posts:
    1,647
    never is a strong word but typically :D

    update is called every frame which makes it framerate dependent. fixed update uses the physics timestep setting. so it's time dependent and will run the same on all end user machines.
     
  13. Daniel_Brauer

    Daniel_Brauer

    Unity Technologies

    Joined:
    Aug 11, 2006
    Posts:
    3,355
    That's why I made the disclaimer about applying forces every frame, though. If it's just a single impulse, does it matter when it's applied? As I understand it, applying an impulse just changes velocity, not position, so you're not going to get undetected collisions, or anything.
     
  14. pete

    pete

    Joined:
    Jul 21, 2005
    Posts:
    1,647
    never is a strong word :D

    that reads backwards. update does exactly that - it would apply force every frame. in mike's case, using update would make velocity change at different rates on different machines because it would apply the force every frame. you wouldn't want the door to fly open on one machine and slowly open on another. adding the force in fixed update will make the door open at the same rate on all machines because the force would be applied based on time increments not every frame.

    a one shot impulse may or may not matter as much. the impulse might get called earlier on a faster machine but i guess it really depends on what you're doing. if update works, ship it!
     
  15. Daniel_Brauer

    Daniel_Brauer

    Unity Technologies

    Joined:
    Aug 11, 2006
    Posts:
    3,355
    You're right, and what I meant was a constant force. I overlooked the fact that Mike's code uses GetKey rather than GetKeyDown. I was operating on the assumption that the force application was a one-off.

    So, if you're still listening, Mike:

    Code (csharp):
    1. function Update() {
    2.     if (Input.GetButton("Activate")) {
    3.         var hit : RaycastHit;
    4.         if (Physics.Raycast(Camera.main.ViewportPointToRay(Vector3(0.5, 0.5, 0.0)), hit,5))
    5.             if (hit.rigidbody  hit.rigidbody.CompareTag("door"))
    6.                 hit.rigidbody.AddRelativeForce(-5,0,0);
    7.     }
    8. }
     
  16. Mike

    Mike

    Joined:
    Dec 1, 2006
    Posts:
    118
    Hey, I read every new post :D

    I have to admit that I changed the GetKeyDown/GetButtonDown from the previous post with what I needed... :wink:
     
  17. Daniel_Brauer

    Daniel_Brauer

    Unity Technologies

    Joined:
    Aug 11, 2006
    Posts:
    3,355
    Good. What's important to take away from this, though, is the Update/FixedUpdate difference. If you're doing something as a one-off (even physics), especially related to input, Update is fine. It's when you're applying continuous force that you need to use FixedUpdate to keep them steady across varying framerates.