Selection and automatic deselection problem

user_18163126 0
Viewed 46

as I stated above, I need help with my c# code which I'm using in unity to make a 3D game. When I click on an object it should change color and if there was previously another object selected, I need it to be deselected automatically. I already wrote a code which is supposed to at least help me select(change a material color) but it isn't working. I am checking if the left mouse button is pressed down and using raycast to check if there is something under a cursor. Also wrote a Selection class with 2 methods(Select/deselect) that are supposed to change color of an object. I'll be thankful if you can help me. Here's the code with mouse click:

public class ClickSelect : MonoBehaviour
{

    private void Update()
    {
        Click();

    }
    public void Click()
    {
        if (Input.GetMouseButtonDown(0))
        {
            RaycastHit hit;
            Ray ray = Camera.main.ScreenPointToRay(Input.mousePosition);
            if (Physics.Raycast(ray, out hit))
            {
                Selectable selectable = hit.collider.gameObject.GetComponent<Selectable>();
                if (selectable)
                {
                    hit.collider.gameObject.GetComponent<Selectable>().Select();
                }

            }
        }
    }
    

}

And this is code of a selection class:

public class Selectable : MonoBehaviour
{
    

    public void Select()
    { 
        GetComponent<Renderer>().material.color = Color.yellow;
    }

    public void Deselect()
    {
        GetComponent<Renderer>().material.color = Color.gray;
    }
}
unity3d c#
2 Answers

First you should check if there is a Collider on the Selectable. If not add one. Because you are asking for a Ray that hits a collider and if there's no Collider it will return always null

then this

Selectable selectable = hit.collider.gameObject.GetComponent<Selectable>();
            if (selectable)
            {
                hit.collider.gameObject.GetComponent<Selectable>().Select();
            }

is complete Nonsense why you get the Selectable in First Place and then the Selectable again? And you don't need a Collider on it to detect the Transform so you can remove the hit.collider to hit.gameObject or better is hit.transform (Because u don't need the whole GameObject)

hit.collider.gameObject.TryGetComponent(out Selectable selectable);
//hit.transform.TryGetComponent(out Selectable selectable)
if(selectable != null)
{
 selectable.Select();
}

Then instead of

Ray ray = Camera.main.ScreenPointToRay(Input.mousePosition);

Try

Ray ray = Camera.main.ViewportPointToRay(Input.mousePosition);

hope it helps you, if so, feel free to mark it as solved and correct Answer.

user_181649840

To expand a bit on this answer, once you get the raycast itself to work properly, you want to store the current selection so you can fire deselect where necessary.

I would keep this information in the Selectable class itself like e.g.

public class Selectable : MonoBehaviour
{
    // Stores the current selection and can only be modified by this class
    private static Selectable _currentSelection;
    // Public read-only accessor if needed
    public static Selectable CurrentSelection => _currentSelection;

    // Deselcts if current selection and resets the selection
    public static void ClearSelection()
    {
        // if there is a current selection -> deselect it
        if(_currentSelection) _currentSelection.Deselect();

        // "forget" the reference
        _currentSelection = null;
    }

    // In general rather cache reused references
    [SerializeField] private Renderer _renderer;

    // Optional accessor
    public bool IsSelected => _currentSelection == this;

    private void Awake ()
    {
        // if not referenced yet get it ONCE on runtime
        if(!_renderer) _renderer = GetComponent<Renderer>();

        // I would also ensure the default state initially
        Deselect();
    }

    public void Select()
    { 
        // if this is already the selected object -> nothing to do
        if(_currentSelection == this) return;

        // otherwise first clear any existing selection
        ClearSelection();

        // set your color
        _renderer.material.color = Color.yellow;

        // and store yourself as the current selection
        _currentSelection = this;
    }

    public void Deselect()
    {
        // set your color
        _renderer.material.color = Color.gray;

        // if this is the current selection forget the reference
        // usually this should always be the case anyway
        if(_currentSelection == this)
        {
            _currentSelection = null;
        }
    }
}

and then

public class ClickSelect : MonoBehaviour
{
    // in general Camera.main is expensive -> rather store he reference and reuse it
    // Already reference this via the Inspector if possible
    [SerializeField] private Camera _mainCamera;

    private void Awake ()
    {
        // as fallback get it ONCE on runtime
        if(!_mainCamera) _mainCamera = Camera.main;
    }

    private void Update()
    {
        if (Input.GetMouseButtonDown(0))
        {
            var ray = _mainCamera.ScreenPointToRay(Input.mousePosition);
            if (Physics.Raycast(ray, out var hit))
            {
                // Is what you clicked on a selectable?
                if (hit.transform.TryGetComponent<Selectable>(out var selectable))
                {
                    // Internally already handles deselection of current selected and skips if already selected this
                    selectable.Select();
                }
                // Optional: Deselect the current selection if you click on something else
                else
                {
                    Selectable.ClearSelection();
                }
            }
            // Optional: Deselect the current selection if you click on nothing
            else
            {
                Selectable.ClearSelection();
            }
        }
    }
}
user_71115610
Related