0
\$\begingroup\$

I'm making Space shooter game in Unity, in which I want to place boss. I have script in which I wrote how boss should move and it works but... I know i can do better. Any ideas how can I improve that code?

void FixedUpdate () {

     time -= Time.deltaTime;

     if(time < 0 && time > -2.7)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = -1 * this.gameObject.transform.up * speed;
     }
     else if(time < -2.7 && time > -5)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = -1 * this.gameObject.transform.up * 0;
     }
     else if(time < -5 && time > -7.7)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = -1 * this.gameObject.transform.right * speed;
     }
     else if (time < -7.7 && time > -9.7)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = -1 * this.gameObject.transform.up * 0;
     }
     else if(time < -9.7 && time > -11)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = this.gameObject.transform.up * 1/speed;
     }
     else if (time < -11 && time > -11.5)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = -1 * this.gameObject.transform.up * speed * 10;
     }
     else if (time < -11.5 && time > -16.15)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = this.gameObject.transform.up * speed;
     }
     else if (time < -16.15 && time > -19)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = -1 * this.gameObject.transform.up * 0;
     }
     else if (time < -19 && time > -24.5)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = this.gameObject.transform.right * speed;
     }
     else if (time < -24.5 && time > -26.5)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = this.gameObject.transform.up * 0;
     }
     else if (time < -26.5 && time > -27.8)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = this.gameObject.transform.up * 1 / speed;
     }
     else if (time < -27.8 && time > -28.3)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = -1 * this.gameObject.transform.up * speed * 10;
     }
     else if (time < -28.3 && time > -33)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = this.gameObject.transform.up * speed;
     }
     else if (time < -33 && time > -36)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = -1 * this.gameObject.transform.up * 0;
     }
     else if (time < -36 && time > -38.8)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = -1 * this.gameObject.transform.right * speed;
     }
     else if (time < -38.8 && time > -41)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = this.gameObject.transform.up * 0;
     }
     else if (time < -41 && time > -42.3)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = -1 * this.gameObject.transform.up * speed;
     }
     else if (time < -42.3 && time > -70)
     {
         this.gameObject.GetComponent<Rigidbody2D>().velocity = this.gameObject.transform.up * 0;
         Laser();
     }
     else if (time < -70 && time > -71.2)
     {
         laserSpawnDelay = 0;
         this.gameObject.GetComponent<Rigidbody2D>().velocity = this.gameObject.transform.up * speed;
     }
     else if (time < -71.2)
     {
         time = -2.7f;
     }

     GunShoot();
     RocketShoot();
     //Laser();

 }
\$\endgroup\$
  • 2
    \$\begingroup\$ It would really help if you explain the logic behind the code. For example, why between -2.7 and -5 the velocity is 0 ? What is the boss's expected behaviour and why ? \$\endgroup\$ – TomTsagk Apr 5 '18 at 13:34
  • 1
    \$\begingroup\$ What is the reason you don't use animation for moving a character? \$\endgroup\$ – Nick Apr 5 '18 at 13:34
  • 2
    \$\begingroup\$ By the way, we have a stackexchange site specialized in improving code: codereview.stackexchange.com \$\endgroup\$ – Philipp Apr 5 '18 at 13:49
  • \$\begingroup\$ couple of things to research: Switch/Case, statemachine pattern (might be overkill for something like this) and using polymorphism instead of conditionals also the Strategy pattern can help it is related to the State Machine. \$\endgroup\$ – Slagmoth Apr 5 '18 at 14:25
2
\$\begingroup\$

The main problem here seems to be a code solution for a data problem. You basically want an array of {time,speed, direction} triplets. "Speed" in this context would be just the multiplier. So the table would look like this:

{ 0, 1, up }

{ 2.7, 0, up }

{ 5.0, 1, right }

BTW, the 1/speed makes no sense; the dimension is just wrong. Whether you express speed in pixels/second or tiles/tick, you can't mix speed and 1/speed because you'd be mixing distance per time and time per distance.

\$\endgroup\$
0
\$\begingroup\$

A few things:

You probably don't need to set the velocity every frame. It looks like you may want to just make sure it's set to be kinematic (in the inspector if possible), and then set the velocity once on each change.

This implementation is prone to decaying accuracy. Since you are setting the velocity rather than the position, and relying on an amount of time to pass before changing it, and fixedDeltaTime may not be set to an exact value where you trigger changes at the exact right physics step, the body won't necessarily end up in the same place each time you go through your movement loop. This would compound over time, causing gradual drift.

As mentioned by @MSalters, this does seem to be a problem where you are trying to represent data as code. You could expose some parameters in the inspector and use those. Here is an example of that put into code. This uses a coroutine to loop through the steps, and only sets the velocity on each change.

[Serializable]
public class AnimationStep {
    float Duration;
    Vector2 Velocity;
}
public AnimationStep[] Steps;

void Start () {
     StartCoroutine(RunBehavior());
}

IEnumerator RunBehavior() {
     Rigidbody2D body = GetComponent<Rigidbody2D>();
     var wait = new WaitForFixedUpdate();
     yield return wait;
     while (true) {
         foreach (var step in Steps) {
             body.velocity = step.Velocity * speed; //You could add more fields to AnimationStep to make this easier to use.
             float endTime = Time.fixedTime + step.Duration;
             while (Time.fixedTime < endTime)
                 yield return wait;
         }
     }
 }

void FixedUpdate() {
     GunShoot();
     RocketShoot();
     //Laser();
}

Note that this still suffers from the potential decaying accuracy problem. It uses a loop yield returning every fixed update to wait for the next change, rather than using WaitForSeconds, which could cause even worse drift.

A better solution would be to specify a set of target positions to move between, rather than velocities (really, just make it an animation).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.