C#•2y ago

❔ Trying to get better at writting code. any advice?

        moving = true;
        bool canSprint;
        if (sprinting)
        {
            canSprint = true;
            // TODO: sprinting system like in SCP: CB
        }
        else
        {
            canSprint = false;
        }

        Vector3 movement = Vector3.zero;
        float tempSpeed = canSprint ? sprintSpeed : walkSpeed;
        moving = false;
        if (speed.x != 0)
        {
            moving = true;
            movement += speed.x * Time.deltaTime * tempSpeed * Vector3.left;
        }
        if (speed.y != 0)
        {
            moving = true;
            movement += speed.y * Time.deltaTime * tempSpeed * Vector3.back;
        }
        if (movement != Vector3.zero)
        {
            transform.Translate(movement);
            StepClimb(movement);
        }

        moving = true;
        bool canSprint;
        if (sprinting)
        {
            canSprint = true;
            // TODO: sprinting system like in SCP: CB
        }
        else
        {
            canSprint = false;
        }

        Vector3 movement = Vector3.zero;
        float tempSpeed = canSprint ? sprintSpeed : walkSpeed;
        moving = false;
        if (speed.x != 0)
        {
            moving = true;
            movement += speed.x * Time.deltaTime * tempSpeed * Vector3.left;
        }
        if (speed.y != 0)
        {
            moving = true;
            movement += speed.y * Time.deltaTime * tempSpeed * Vector3.back;
        }
        if (movement != Vector3.zero)
        {
            transform.Translate(movement);
            StepClimb(movement);
        }

Any changes you think can be done in the code above? (do note speed variable is a method argument. the method is called either by "GameManager" that manages player input, or by an AI)

49 Replies

TheBoxyBear•2y ago

Lost of semi duplicate instructions that have the same idea with different values. You can condense it using variables and methods movement += speed.y * Time.deltaTime * tempSpeed * Vector3.back This is a good start, if it's 0 the multiplication will resolve to 0 anyway Though it could be optimized by caching the result of Time.deldaTime * tempSpeed to a variable moving = movement > 0 logical operators already return bool so it's simply assigning it directly to the variable Right, you can't directly compare a Vector3 to an int, so then it should be moving = movement.x > 0 || movement.y > 0 or compare to that though it will have a wrong result if z > 0

Costin•2y ago

I wouldn't remove canSprint since I will add a sprinting mechanism and if I make it all depend on sprinting, some other code may malfunction. there isn't any abs

        float tempSpeed = walkSpeed;
        if (sprinting)
        {
            tempSpeed = sprintSpeed;
            // TODO: sprinting system like in SCP: CB
        }
        Vector3 movement = Vector3.zero;
        float resSpeed = Time.deltaTime * tempSpeed;
        movement += speed.x * resSpeed * Vector3.left;
        movement += speed.y * resSpeed * Vector3.back;

        moving = false;
        if (movement != Vector3.zero)
        {
            moving = true;
            transform.Translate(movement);
            StepClimb(movement);
        }

        float tempSpeed = walkSpeed;
        if (sprinting)
        {
            tempSpeed = sprintSpeed;
            // TODO: sprinting system like in SCP: CB
        }
        Vector3 movement = Vector3.zero;
        float resSpeed = Time.deltaTime * tempSpeed;
        movement += speed.x * resSpeed * Vector3.left;
        movement += speed.y * resSpeed * Vector3.back;

        moving = false;
        if (movement != Vector3.zero)
        {
            moving = true;
            transform.Translate(movement);
            StepClimb(movement);
        }

this is my new code. any changes now?

TheBoxyBear•2y ago

Seems good except for the two assignements of moving You could also set that to movement != Vector3.Zero and put moving in the if

Costin•2y ago

wdym also: from what I've learned from you guys: 1. try to use as few variables as possible strategically 2. try to make as few branches as possible, math is more optimized than that, and 0 multiplication is always 0 (logically)

TheBoxyBear•2y ago

Conditions are themselves bool values

Costin•2y ago

yeah but StepClimb and transform.translate should be called only when the player moves and the player moves when moving would be set to true and moving would be set to true when movement isn't zero and there's no abs in vector3, but there's Vector3.zero which is (0,0,0)

TheBoxyBear•2y ago

So do the things when moving, which is directly defined from the variable, which is itself defined from a condition Unless the moving variable is used elsewhere to avoid checking the vector3 too many times, you can get rid of it entirely

Costin•2y ago

the moving variable would be used for animation and sounds and stuff

TheBoxyBear•2y ago

If you do need the variable, it's always set to true when not zero and false when zero, so you can condense that logic as a single bool assignement moving = movement != Vector3.Zero

Costin•2y ago

also that is in a move method so it's

    public override void Move(Vector3 speed)
    {
        float tempSpeed = walkSpeed;
        if (sprinting)
        {
            tempSpeed = sprintSpeed;
            // TODO: sprinting system like in SCP: CB
        }
        Vector3 movement = Vector3.zero;
        float resSpeed = Time.deltaTime * tempSpeed;
        movement += speed.x * resSpeed * Vector3.left;
        movement += speed.y * resSpeed * Vector3.back;

        moving = false;
        if (movement != Vector3.zero)
        {
            moving = true;
            transform.Translate(movement);
            StepClimb(movement);
        }
    }

    public override void Move(Vector3 speed)
    {
        float tempSpeed = walkSpeed;
        if (sprinting)
        {
            tempSpeed = sprintSpeed;
            // TODO: sprinting system like in SCP: CB
        }
        Vector3 movement = Vector3.zero;
        float resSpeed = Time.deltaTime * tempSpeed;
        movement += speed.x * resSpeed * Vector3.left;
        movement += speed.y * resSpeed * Vector3.back;

        moving = false;
        if (movement != Vector3.zero)
        {
            moving = true;
            transform.Translate(movement);
            StepClimb(movement);
        }
    }

and Move is called either by an AI when he wants, or when player inputs

TheBoxyBear•2y ago

Since the vector was already compared once and the result stored in moving, you can use that as the condition in the if if (moving) {} bools and conditions are interchangable, more like they're both bool

Costin•2y ago

the speed variable also shouldn't really be a speed, it should rather be -1 to +1 (as AD and WS, but also controller support) so I can do if (movement != Vector3.zero) instead of that bool? oh

TheBoxyBear•2y ago

The opposite Set moving to the condition and check moving

Costin•2y ago

moving = movement.magnitude == 0 ? false : true; is magnitude good idk what it does but I've seen it's smth like x*x + y*y..

TheBoxyBear•2y ago

If you have controller support, make sure the values can be decimal to allow for analog movement

Costin•2y ago

the input supports controllers, but the game won't have other devices support other than pc for a long time anyway

TheBoxyBear•2y ago

Most pc games also have controller support

Costin•2y ago

but what do I do if the speed is more or less than -1 and +1? does magnitude work?

TheBoxyBear•2y ago

It wouldn't. It would just be a decimal number in the range -1, 1 where -1 or 1 is the max speed in the matching direction

Costin•2y ago

no like moving = movement.magnitude == 0 ? false : true;

TheBoxyBear•2y ago

Is it a top down where moving up isn't jumping?

Costin•2y ago

it's a 3d first person

TheBoxyBear•2y ago

Then magnitude would be > 0 whenever airborne

Costin•2y ago

but I did not implement anything about jumping rn, and idk if I should add it move method will not be called when gravity it will be called either by AI or by player input

TheBoxyBear•2y ago

Magnitude is if you drew a box with the dimensions of the vector and made a 3d arrow going from one corner to the opposite corner. The magnitude is the length of the line/arrow And if I recall, unity uses Y for the vertical axis, so moving and strafing should use x and z

Costin•2y ago

so if speed is 0,0,0, then the magnitude would be 0, right?

TheBoxyBear•2y ago

yes

Costin•2y ago

so I can use magnitude or Vector3.zero

TheBoxyBear•2y ago

The vector is obtained from the input right?

Costin•2y ago

    public override void Move(Vector3 direction)
    {
        float tempSpeed = walkSpeed;
        if (sprinting)
        {
            tempSpeed = sprintSpeed;
            // TODO: sprinting system like in SCP: CB
        }
        Vector3 movement = Vector3.zero;
        float resSpeed = Time.deltaTime * tempSpeed;
        movement += Mathf.Clamp01(direction.x) * resSpeed * Vector3.left;
        movement += Mathf.Clamp01(direction.y) * resSpeed * Vector3.back;

        moving = movement == Vector3.zero ? false : true;

        if (moving)
        {
            transform.Translate(movement);
            StepClimb(movement);
        }
    }

    public override void Move(Vector3 direction)
    {
        float tempSpeed = walkSpeed;
        if (sprinting)
        {
            tempSpeed = sprintSpeed;
            // TODO: sprinting system like in SCP: CB
        }
        Vector3 movement = Vector3.zero;
        float resSpeed = Time.deltaTime * tempSpeed;
        movement += Mathf.Clamp01(direction.x) * resSpeed * Vector3.left;
        movement += Mathf.Clamp01(direction.y) * resSpeed * Vector3.back;

        moving = movement == Vector3.zero ? false : true;

        if (moving)
        {
            transform.Translate(movement);
            StepClimb(movement);
        }
    }

is this better?

TheBoxyBear•2y ago

Where does direction come from?

Costin•2y ago

input or AI basically this is input:

    public void FixedUpdate()
    {
        if (player != null && controllablePlayer)
        {
            // Camera Rotation
            float inputX = Input.GetAxisRaw("Mouse X") * Time.deltaTime * mouseSensitivity;
            float inputY = Input.GetAxisRaw("Mouse Y") * Time.deltaTime * mouseSensitivity;

            xRotation -= inputY;
            xRotation = Mathf.Clamp(xRotation, -90, 90);

            playerCam.localRotation = Quaternion.Euler(xRotation, 180f, 0f);
            player.transform.Rotate(player.transform.up * inputX);

            // Player Movement
            inputX = Input.GetAxis("Horizontal");
            inputY = Input.GetAxis("Vertical");
            switch (player)
            {
                case HumanEntity human:
                    human.sprinting = Input.GetKey(KeyCode.LeftShift);
                    break;
                default:
                    break;
            }
            player.Move(new Vector3(inputX, inputY, 0));
        }
    }

    public void FixedUpdate()
    {
        if (player != null && controllablePlayer)
        {
            // Camera Rotation
            float inputX = Input.GetAxisRaw("Mouse X") * Time.deltaTime * mouseSensitivity;
            float inputY = Input.GetAxisRaw("Mouse Y") * Time.deltaTime * mouseSensitivity;

            xRotation -= inputY;
            xRotation = Mathf.Clamp(xRotation, -90, 90);

            playerCam.localRotation = Quaternion.Euler(xRotation, 180f, 0f);
            player.transform.Rotate(player.transform.up * inputX);

            // Player Movement
            inputX = Input.GetAxis("Horizontal");
            inputY = Input.GetAxis("Vertical");
            switch (player)
            {
                case HumanEntity human:
                    human.sprinting = Input.GetKey(KeyCode.LeftShift);
                    break;
                default:
                    break;
            }
            player.Move(new Vector3(inputX, inputY, 0));
        }
    }

also yes I know I can do player != null && controllablePlayer but I might have added something else (I think I'll just wrap the two if statements for now)

TheBoxyBear•2y ago

Well if you only care about it moving in any direction, you can use magnitude Though depending on how it's implemented in Vector3, that may be expensive to do

Costin•2y ago

https://docs.unity3d.com/ScriptReference/Vector3-magnitude.html

Costin•2y ago

I'd move it in x and z, there's no jumping mechanisms yet buy in case I add, I plan checking if direction.y is > 0

TheBoxyBear•2y ago

It's suggesting using square magnitude if it's only for comparison so safe to say it gets calculated on the fly Then it's faster to just use Vector3.Zero

Costin•2y ago

ah lol didn't see that notice

TheBoxyBear•2y ago

What I've been suggesting earlier Then if (moving) {}

Costin•2y ago

    public override void Move(Vector3 direction)
    {
        float tempSpeed = walkSpeed;
        if (sprinting)
        {
            tempSpeed = sprintSpeed;
            // TODO: sprinting system like in SCP: CB
        }
        Vector3 movement = Vector3.zero;
        float resSpeed = Time.deltaTime * tempSpeed;
        movement += Mathf.Clamp01(direction.x) * resSpeed * Vector3.left;
        movement += Mathf.Clamp01(direction.y) * resSpeed * Vector3.back;

        moving = movement != Vector3.zero;

        if (moving)
        {
            transform.Translate(movement);
            StepClimb(movement);
        }
    }

    public override void Move(Vector3 direction)
    {
        float tempSpeed = walkSpeed;
        if (sprinting)
        {
            tempSpeed = sprintSpeed;
            // TODO: sprinting system like in SCP: CB
        }
        Vector3 movement = Vector3.zero;
        float resSpeed = Time.deltaTime * tempSpeed;
        movement += Mathf.Clamp01(direction.x) * resSpeed * Vector3.left;
        movement += Mathf.Clamp01(direction.y) * resSpeed * Vector3.back;

        moving = movement != Vector3.zero;

        if (moving)
        {
            transform.Translate(movement);
            StepClimb(movement);
        }
    }

    public void FixedUpdate()
    {
        if (player != null && controllablePlayer) // might unwrap these 2 conditions someday
        {
            // Camera Rotation
            float inputX = Input.GetAxisRaw("Mouse X") * Time.deltaTime * mouseSensitivity;
            float inputY = Input.GetAxisRaw("Mouse Y") * Time.deltaTime * mouseSensitivity;

            xRotation -= inputY;
            xRotation = Mathf.Clamp(xRotation, -90, 90);

            playerCam.localRotation = Quaternion.Euler(xRotation, 180f, 0f);
            player.transform.Rotate(player.transform.up * inputX);

            // Player Movement
            inputX = Input.GetAxis("Horizontal");
            inputY = Input.GetAxis("Vertical");
            switch (player)
            {
                case HumanEntity human:
                    human.sprinting = Input.GetKey(KeyCode.LeftShift);
                    break;
                default:
                    break;
            }
            player.Move(new Vector3(inputX, inputY, 0));
        }
    }

    public void FixedUpdate()
    {
        if (player != null && controllablePlayer) // might unwrap these 2 conditions someday
        {
            // Camera Rotation
            float inputX = Input.GetAxisRaw("Mouse X") * Time.deltaTime * mouseSensitivity;
            float inputY = Input.GetAxisRaw("Mouse Y") * Time.deltaTime * mouseSensitivity;

            xRotation -= inputY;
            xRotation = Mathf.Clamp(xRotation, -90, 90);

            playerCam.localRotation = Quaternion.Euler(xRotation, 180f, 0f);
            player.transform.Rotate(player.transform.up * inputX);

            // Player Movement
            inputX = Input.GetAxis("Horizontal");
            inputY = Input.GetAxis("Vertical");
            switch (player)
            {
                case HumanEntity human:
                    human.sprinting = Input.GetKey(KeyCode.LeftShift);
                    break;
                default:
                    break;
            }
            player.Move(new Vector3(inputX, inputY, 0));
        }
    }

so is this neat? ok and should I revert that if I add non-player code in FixedUpdate?

Unknown User•2y ago

Message Not Public

Costin•2y ago

why var? does it do anything apart from shortening Vector3?

    public override void Move(Vector3 direction)
    {
        var tempSpeed = walkSpeed;
        if (Sprinting)
        {
            tempSpeed = sprintSpeed;
            // TODO: sprinting system like in SCP: CB
        }
        var movement = Vector3.zero;
        var resSpeed = Time.deltaTime * tempSpeed;
        movement += Mathf.Clamp01(direction.x) * resSpeed * Vector3.left;
        movement += Mathf.Clamp01(direction.y) * resSpeed * Vector3.back;

        Moving = movement != Vector3.zero;

        if (Moving)
        {
            transform.Translate(movement);
            StepClimb(movement);
        }
    }

    public override void Move(Vector3 direction)
    {
        var tempSpeed = walkSpeed;
        if (Sprinting)
        {
            tempSpeed = sprintSpeed;
            // TODO: sprinting system like in SCP: CB
        }
        var movement = Vector3.zero;
        var resSpeed = Time.deltaTime * tempSpeed;
        movement += Mathf.Clamp01(direction.x) * resSpeed * Vector3.left;
        movement += Mathf.Clamp01(direction.y) * resSpeed * Vector3.back;

        Moving = movement != Vector3.zero;

        if (Moving)
        {
            transform.Translate(movement);
            StepClimb(movement);
        }
    }

so like this so it just shortens the code ok then next time I create variables, I'll always use var local* next time I make local variables, I will use var wdym subjective I never listen to my teacher to know what that means ?

blinkbat•2y ago

var is good, it's redundant to declare inferred types. intellisense can give you the type info.

Costin•2y ago

I do get js flashbacks and I really hate js for not having types

blinkbat•2y ago

js has types, it just does a lot of coercion. it's not statically typed, though.

Costin•2y ago

I prefer ts

blinkbat•2y ago

ts IS static. it's much better.

Costin•2y ago

anyway back to my code

Unknown User•2y ago

Message Not Public

Accord•2y ago

Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.

Gaming

Programming

❔ Trying to get better at writting code. any advice?