Problem with elevator script (C#)

I'm trying to make a script that allows the player to open doors and chests and activating elevators, however I've bumped into a few problems, for some reason it can't find my player character. The other problem is that I've tried make the defined thingie for my player public and draged the player prefab there from the inspector while in-game and then tried activating the elevator to see if it works but nothing happends.

Also since I might use this script on quite a few of my objects in-game (for chests etc) I'm not sure if it's smart having it reading if the object is selected in every frame in case it needs to perform the interactive actions, does anyone know another way I could set the interactive part up?

using UnityEngine;
using System.Collections;

public class Button : MonoBehaviour {
    public GameObject target;
    public bool allowReset = false;

    public bool elevator = false;
    public Transform elevPointA;
    public Transform elevPointB;
    public bool door = false;
    public bool loot = false;
    public int maxDistance;
    public int Speed;

    private bool _selected = false;     //is the player marking this object with the mouse
    private Transform _player;
    private Transform _myPosition;
    private Transform _targetTransform;

    /// <summary>
    /// State 0 = Elevator PointA, Door closed, Loot closed
    /// State 1 = Elevator PointB, Door open, Loot looted
    /// State 2 = Elevator moving
    /// </summary>
    private int _state = 0;

    void Awake() {
        _targetTransform = target.transform;
        _myPosition = transform;                                            //this is this objects transform
        elevPointA = transform;
        elevPointB = transform;
    }
    void Start() {
        GameObject fp = GameObject.FindGameObjectWithTag("Player");     //find the gameObject tagged Player
        _player = fp.transform;                                         //read gameObject Player as a transform
    }

    void OnMouseEnter() {
        _selected = true;
        renderer.material.SetColor ("_OutlineColor", Color.blue);
    }

    void OnMouseExit() {
        _selected = false;
        renderer.material.SetColor ("_OutlineColor", Color.black);
    }

    void Update() {
        if(Input.GetMouseButtonUp(0)) {
            if(_selected == true) {
                if(Vector3.Distance(_player.position, _myPosition.position) > maxDistance) {
                    _state = 1;
                    if(elevator == true)
                        Elevator();
                    else if(door == true)
                        Door();
                    else if(loot == true)
                        Loot();
                }
            }
        }
// stop the elevator and reset the button if the elevator has reached CheckPoint A
        if(Vector3.Distance(_targetTransform.position, elevPointA.position) == 0) {
            if(_state == 0) {
                return;
            }
            else {
                Speed = 0;
                _state = 0;
            }
        }
// stop the elevator and reset the button if the elevator has reached CheckPoint B
        if(Vector3.Distance(_targetTransform.position, elevPointB.position) == 0) {
            if(_state == 1) {
                return;
            }
            else {
                Speed = 0;
                _state = 0;
            }
        }
    }

    void Elevator() {

        if(_state == 0) {
            _targetTransform.Translate(Vector3.up * Speed * Time.deltaTime);
            _state = 2;
        }
        else if(_state == 1) {
            _targetTransform.Translate(Vector3.up * Speed *Time.deltaTime);
            _state = 2;
        }

    }

    void Door() {

    }

    void Loot() {

    }
}

(I'm usually just doing graphics but have begun coding just recently so please excuse me if my script is a mess.)

The code is actually nicely formatted. The logic is very broken though.

In your Awake, you assign various transforms. None of them are needed and the very presence of some of those assignments breaks your functionality:

  • Why are you assigning both `elevPointA` and `elevPointB` to be the current `transform`? by doing this, you are storing 3 separate references to the current transform and when you write code like `Vector3.Distance(_targetTransform.position, elevPointA.position) == 0` and then later `Vector3.Distance(_targetTransform.position, elevPointB.position) == 0`, both will be true and false at the same time because they are the same transform.
  • If you are only ever using the `Transform` of `target`, then why not just make that `public` and drop the `GameObject` altogether.
  • `transform` is `public`, you don't need to store it for what you're doing with it

In your Elevator(), you only ever go up. Is this intentional? Also, speed is only ever set to 0 here. Because you are moving up by some floating point value which you can't be sure divides evenly into the distance between target's transform.position and elvePointA.position or elevPointB.position, it is likely that the distance between then will never be 0 and your if statements (which essentially both do the same thing and should be combined) will never be true.

Your functions and variables are poorly named and this makes the code harder to understand. What does it mean for elevator to be true? Does it mean that the button controls an elevator? If it can only control one type of thing at a time, then you should only need to store one state variable to represent this. You should consider naming your functions verbs or events and your variables what they actually mean (like controlsDoor or something that can actually be said to be true or false, unlike simply door which does not indicate something that is true or false). For complex state variables, you might consider using enumerators to improve readability.

If you're worried about efficiency, you should rearrange your logical comparisons to do the cheaper operations first and to do computation less often by merging your statements. Also, using squared distance is cheaper than distance because it requires one less square root and square roots are fairly expensive. You should also combine your logical comparisons into the same clause if appropriate using AND (&&) and OR (||), because while it's not all that expensive, it's a matter of readability and ease of following the logic.

Should a button really store what it is a button for? Should it do all the controls? If that's the case then you should have specialized buttons for each thing, rather than going for a generalized button. If you want a generic button, it's better to put that logic with the object being controlled so that if you wanted your buttons to control other things, then all you'd have to do is write the appropriate script for your other thing and you wouldn't have to worry about the button.

The following is simply an example of how to break up the code:

Button.cs (Attached to buttons)

using UnityEngine;

public class Button : MonoBehaviour {
    public GameObject target;
    public float maxSqrDistance = 900.0f;
    public bool pushed = false;

    private Transform player;

    void Start() {
        player = GameObject.FindGameObjectWithTag("Player").transform;
    }

    void OnMouseEnter() {
        renderer.material.SetColor("_OutlineColor", Color.blue);
    }

    void OnMouseExit() {
        renderer.material.SetColor("_OutlineColor", Color.black);
    }

    void OnMouseUp() {
        if((player.position-transform.position).sqrMagnitude < maxSqrDistance) {
            pushed = true;
            target.SendMessage("ButtonPushed", transform);
        }
    }

    void Done() {
        pushed = false;
    }
}

Elevator.cs (Attached to elevators)

using UnityEngine;
using System.Collections.Generic;

public class Elevator : MonoBehaviour {
    public Transform[] targetPositions;
    public float minTransitionTime = 2.0f;
    public float maxSpeed = 5.0f;
    public float timeToWait = 3.0f;
    private Queue<Transform> buttonRequests = new Queue<Transform>();
    private KeyValuePair<Transform,Transform> current = 
        new KeyValuePair<Transform,Transform>(null,null);
    private float yVelocity = 0.0f;
    private float waitTime = 0.0f;
    private bool waiting = true;

    void ButtonPushed(Transform button) { //Queue up the request
        if(!buttonRequests.Contains(button)) buttonRequests.Enqueue(button);
    }

    void Update() {
        if(targetPositions.Length == 0) return;
        if(waitTime > 0.0f) waitTime -= Time.deltaTime;
        if(waiting && waitTime <= 0.0f) { //Done waiting
            //Clear destination
            if(current.Key != null) {
                //Do stuff here like sending the elevator to another position
                //This depends on your elevator setup
                current = new KeyValuePair<Transform,Transform>(null,null);
            }
            //Get a new destination
            if(buttonRequests.Count == 0) return;
            Transform nextButton = buttonRequests.Dequeue();

            //Find the closest position to the button
            Transform nextPosition = transform;
            float minDistance = Mathf.Infinity;
            float currentDistance = 0.0f;
            foreach(Transform targetPosition in targetPositions) {
                currentDistance = Mathf.Abs(targetPosition.position.y
                                  -nextButton.position.y);
                if(currentDistance < minDistance) {
                    minDistance = currentDistance;
                    nextPosition = targetPosition;
                }
            }
            //Set to move to the position nearest the button
            current = new KeyValuePair<Transform,Transform>(nextPosition,
                                                            nextButton);
            waiting = false;
        }
        if(!waiting) {
            //Move towards our destination
            float newPosition = Mathf.SmoothDamp(transform.position.y,
                                                 current.Key.position.y,
                                                 ref yVelocity,
                                                 minTransitionTime,
                                                 maxSpeed);
            transform.position = new Vector3(transform.position.x,
                                             newPosition,
                                             transform.position.z);
            //We've reached our destination
            if(Mathf.Abs(transform.position.y-current.Key.position.y)<0.03f) {
                //Do stuff here like opening doors, etc.
                waiting = true;
                waitTime = timeToWait;
                current.Value.gameObject.SendMessage("Done");
            }
        }
    }
}

Lootable.cs (Attached to lootable objects)

using UnityEngine;

public class Lootable: MonoBehaviour {
    void ButtonPushed(Transform button) {
        //Whatever you do when you loot something
    }
}

Door.cs (Attached to doors)

using UnityEngine;

public class Door: MonoBehaviour {
    void ButtonPushed(Transform button) {
        //Open/Close the door
    }
}