3
\$\begingroup\$

hi I have made and AI for my enemies which works perfectly but a particular section of code is repetitive and I feel that it can be optimised

void ChangeDirection()
    {
        if (movement == Vector3.left)
            transformModel.rotation = Quaternion.Euler(0, 270, 0);
        else if (movement == Vector3.right)
            transformModel.rotation = Quaternion.Euler(0, 90, 0);
        else if (movement == Vector3.back)
            transformModel.rotation = Quaternion.Euler(0, 180, 0);
        else if (movement == Vector3.forward)
            transformModel.rotation = Quaternion.Euler(0, 0, 0);
    }

the enemy will be in a tight corridor where only the following movements are possible i.e enemy can only move the vector3. right, left, back and forward. thus the code works without any issues, however, the code is repetitive and I want the enemy to move in whichever its movement vector will be.

here is the rest my code

using System.Collections;
using System.Collections.Generic;
using UnityEngine;

public class EnemyMovement2 : MonoBehaviour
{
    public float speed;
    public Vector3 movement;
    public Animator animator;
    private GameObject gameManager;

    void Start()
    {

        movement = Vector3.back;
        gameManager = GameObject.FindWithTag("GameController");


    }
    void OnCollisionEnter(Collision collision)
    {
        if (collision.gameObject.tag == "Wall")
        {

            CorrectPosition(new Vector3(1, 1, 1));
            movement = Vector3.zero;

        }
    }

    public Transform transformModel;
    void ChangeDirection()
    {
        if (movement == Vector3.left)
            transformModel.rotation = Quaternion.Euler(0, 270, 0);
        else if (movement == Vector3.right)
            transformModel.rotation = Quaternion.Euler(0, 90, 0);
        else if (movement == Vector3.back)
            transformModel.rotation = Quaternion.Euler(0, 180, 0);
        else if (movement == Vector3.forward)
            transformModel.rotation = Quaternion.Euler(0, 0, 0);
    }
    void CorrectPosition(Vector3 CorrectPositionOF)
    {
        Vector3 posNow = transform.position;
        if (CorrectPositionOF.x == 1)
            posNow.x = Mathf.RoundToInt(gameObject.transform.position.x);
        if (CorrectPositionOF.y == 1)
            posNow.y = Mathf.RoundToInt(gameObject.transform.position.y);
        if (CorrectPositionOF.z == 1)
            posNow.z = Mathf.RoundToInt(gameObject.transform.position.z);
        transform.position = posNow;
    }
    public LayerMask whatIsWall;
    private Vector3[] direction = { Vector3.left, Vector3.forward, Vector3.back, Vector3.right };
    void FixedUpdate()
    {
        GetComponent<Rigidbody>().velocity = movement * speed;
        animator.SetFloat("runningSpeed", GetComponent<Rigidbody>().velocity.magnitude);
        for (int i = 0; i < direction.Length; i++)
        {
            RaycastHit hit;
            if (Physics.Raycast(transform.position, direction[i], out hit, 18, 0 << 11 | 1 << 8 | 1 << 9)
                && hit.collider.gameObject.tag == "Player"
                && movement == Vector3.zero)
            {
                movement = direction[i];
                ChangeDirection();
                return;
            }
        }


    }
}
```
\$\endgroup\$
4
\$\begingroup\$

One downside to your approach is that if you later add more movement directions such as diagonal directions, you will have to add additional branches in ChangeDirection

Another approach that would work with arbitrarily many possible movement directions is to use Quaternion.LookRotation.

Quaternion.LookRotation creates a Quaternion that:

  1. Rotates from Vector3.forward to a target vector
  2. Rotates Vector3.up as close as possible to a target vector that defaults to Vector3.up

Because what you are doing is calculating a Quaternion that rotates from Vector3.forward to movement while not rotating Vector3.up, using Quaternion.LookRotation makes for a very simple one-liner that explains itself to anyone who is familiar with the Unity API:

void ChangeDirection()
{
    if(movement!=Vector3.zero)
        transformModel.rotation = Quaternion.LookRotation(movement);
}
\$\endgroup\$
  • 1
    \$\begingroup\$ You might get some votes if 1) made one or more observations about the posters code. 2) explained why the alternate solution might be better 3) explained what the alternate solution is because sometimes links get broken. Please see code review guidelines at codereview.stackexchange.com/help/how-to-answer. \$\endgroup\$ – pacmaninbw Jul 26 at 19:32
  • \$\begingroup\$ @pacmaninbw thanks for the feedback. I added some more information. Hope that helps. \$\endgroup\$ – Ruzihm Jul 26 at 19:40
  • \$\begingroup\$ Thank you this works perfectly although I had to account for an exceptional case when the movement is vector.zero by using void ChangeDirection() { if(movement!=Vector3.zero) transformModel.rotation = Quaternion.LookRotation(movement); } \$\endgroup\$ – FlamesWillBurst Jul 26 at 21:57
  • \$\begingroup\$ @FlamesWillBurst Thanks! I went ahead and included that in my answer. \$\endgroup\$ – Ruzihm Jul 26 at 22:01
6
\$\begingroup\$

If you want DRY the KISS way, I suggest substituting the if-chain..

if (movement == Vector3.left)
    transformModel.rotation = Quaternion.Euler(0, 270, 0);
else if (movement == Vector3.right)
    transformModel.rotation = Quaternion.Euler(0, 90, 0);
else if (movement == Vector3.back)
    transformModel.rotation = Quaternion.Euler(0, 180, 0);
else if (movement == Vector3.forward)
    transformModel.rotation = Quaternion.Euler(0, 0, 0);

..with a lookup map:

var rotationsEulerY = new Dictionary<Vector3, int>
{
    {Vector3.left, 270}, {Vector3.right, 90}, {Vector3.back, 180}, {Vector3.forward, 0}
};

void ChangeDirection()
{
    transformModel.rotation = Quaternion.Euler(0, rotationsEulerY[movement], 0);
}

You could do something similar for CorrectPosition using a predefined map of guard conditions and assignor actions.

\$\endgroup\$
  • \$\begingroup\$ Wow, thanks, though I think I don't comprehend it fully. from my understanding whenever ChangeDirection() is called the rotation on the y-axis is set to rotationEulerY[movment] which takes the current moment and checks it with the Vector3 defined in the dictionary and if a match is found the int value next to it is pushed out. I will use this code right now it. Though you see I plan to add a scenario where the enemy will not be confined and directly move towards the player so its movement vector won't be as it has been defined. so I want something that would work even then. \$\endgroup\$ – FlamesWillBurst Jul 26 at 6:35
  • \$\begingroup\$ That would be a follow-up question :) We can't know how you will extend your API upfront. \$\endgroup\$ – dfhwze Jul 26 at 6:37
  • 1
    \$\begingroup\$ I guess you are right. though I did have to learn about how to use Dictionaries today. I am quite new to programming so I learn something new practically every day. one of the problems that I had was the way the need to be implemented I had to write var rotationsEulerY = new Dictionary<Vector3, int> as Dictionary<Vector3, int> rotationsEulerY = new Dictionary<Vector3, int> \$\endgroup\$ – FlamesWillBurst Jul 26 at 7:46
  • \$\begingroup\$ Which version of .NET Framework you use? \$\endgroup\$ – dfhwze Jul 26 at 8:03
  • \$\begingroup\$ well in my Unity player setting it is marked as .NET 4.x equivalent but it should be 4.6 \$\endgroup\$ – FlamesWillBurst Jul 26 at 8:06

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.