Why is my lift not working (Sort of)

Unity Engine
1

Hi guys,

just making a 2d lift for a player to hop onto
i actually have it working, but its working too far to the right from its initial position.
so its going down, but automatically moves about 6 cm to the right of where i placed it.

here is my code:

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

public class Lift1Controller : MonoBehaviour
{
    float currentQuantity;
    float desiredQuantity;
    float MovementPerSecond = 2f;
    Transform transform;

    private void Start()
    {
        currentQuantity = -1.5f;
        desiredQuantity = currentQuantity;
    }

    private void Update()
    {
        MoveLift();
    }

    void MoveLift()
    {
        desiredQuantity = -10.316f;
        currentQuantity = Mathf.MoveTowards(
            currentQuantity,
            desiredQuantity,
            MovementPerSecond * Time.deltaTime);

        transform = GetComponent<Transform>();

        transform.position = (Vector2.up * currentQuantity);
    }
}
2

First, you don’t need this… you’re replicating the transform Component that already exists as that property in a standard MonoBehaviour!

You also don’t need line 31, as that is part of this unnecessary stuff…

Now, once you’ve removed that cruft, start printing values.

Is your lift prefab perhaps constructed wonky, with the lift offset by a bit?

3

This line fixed it

transform.position = new Vector2(-17.19f, currentQuantity);

4

Every object has a transform, so there is no reason to have to reference it?

5

Every GameObject has a Transform component, and Component object (which ALL of your MonoBehaviours also derive from) gives you a shortcut to that Transform since it is simply so broadly useful.

https://docs.unity3d.com/ScriptReference/Component-transform.html

In other news, you really should try to get away from magic numbers:

https://en.wikipedia.org/wiki/Magic_number_(programming)

6

yes, i know that is wrong. Will amend the code, was really just for testing :slight_smile: thanks!

7

This is much better:

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

public class Lift1Controller : MonoBehaviour
{
    float currentQuantity;
    float desiredQuantity;
    float MovementPerSecond = 0.6f;
    private bool liftGoingUp = true;
    public AudioSource liftSound;
    private float liftAtTop = -1.4f;
    private float liftAtBottom = -10.316f;
    private float xPosition = -17.50f;

    private void Start()
    {
        if (liftGoingUp)
        {
            currentQuantity = liftAtBottom;
        } else
        {
            currentQuantity = liftAtTop;;
        }
        desiredQuantity = currentQuantity;
        StartCoroutine(ChangeLiftDirection());
     
    }

    private void Update()
    {
        CheckIfMoving();
        if (liftGoingUp)
        {
            MoveLiftUp();
        } else
        {
            MoveLiftDown();
        }
    }

    void MoveLiftUp()
    {
        desiredQuantity = liftAtTop;
        currentQuantity = Mathf.MoveTowards(
            currentQuantity,
            desiredQuantity,
            MovementPerSecond * Time.deltaTime);

        transform.position = new Vector3(xPosition, currentQuantity,100);
    }

    void MoveLiftDown()
    {
        desiredQuantity = liftAtBottom;
        currentQuantity = Mathf.MoveTowards(
            currentQuantity,
            desiredQuantity,
            MovementPerSecond * Time.deltaTime);

        transform.position = new Vector3(xPosition, currentQuantity, 100);
    }

    private IEnumerator ChangeLiftDirection()
    {
        while (true)
        {
            yield return new WaitForSeconds(25);
            liftGoingUp = !liftGoingUp;
        }
    }

    private void CheckIfMoving()
    {
        if (transform.position.y == liftAtTop || transform.position.y == liftAtBottom)
        {
            liftSound.volume = 0;
        } else
        {
            liftSound.volume = 0.05f;
        }
    }
}
8

You could do a LOT better!

  • remove MoveLiftUp() and MoveLiftDown() and just make it a MoveLift() method.

  • in the MoveLift() method, leave the MoveTowards() and the transform set, but DO NOT ever set desiredQuantity (first line - remove it)

Second, leave the CheckIfMoving() call in update, but replace ALL the rest of it with:

        if (liftGoingUp)
        {
            desiredQuantity = liftAtTop;
        }
        else
        {
            desiredQuantity = liftAtBottom;
        }
        MoveLift();

Far less code to maintain. No two copies of MoveTowards(). No scattering around the setting of the flag vs the position far from each other.

While you’re at it might as well name the moving quantities nicely, something like:

currentLiftPosition```

That makes it pretty unambiguous. Be kind to your future brain. "ZOMG, what is this 'Quantity' thing in here FFS!?"
9

Will redo this when i get a chance, thanks Kurt.