Optimize and Simplify Raycasting Code in Unity for Store Clerk Interaction?

Question

I plan to use a raycast for interacting with all things in my games but Im afraid itll be too laggy how can I shorten this code up?

using System.Collections;
using System.Collections.Generic;
using UnityEngine;
public class StoreClerkOpen : MonoBehaviour
{
 // Start is called before the first frame update
 public GameObject StoreClerk;
 public GameObject Store;
 public GameObject Player;
 public GameObject SCText;
 // Whatever is your max distance (remove if not needed). However, it is nice to 
have a max distance o which your monster can see the player.
 float maxDistance = 10;
 public bool LookingStoreClerk;
 public bool StoreClerkOPEN;
 void FixedUpdate()
 {
 // Will contain the information of which object the raycast hit
 RaycastHit hit;
 // if raycast hits, it checks if it hit an object with the tag Player
 if(Physics.Raycast(transform.position, transform.forward, out hit, 
maxDistance) &&
 hit.collider.gameObject.CompareTag("StoreClerk"))
 {
 SCText.SetActive(true);
 LookingStoreClerk = true;
 }
 if (Input.GetMouseButtonDown(0) && LookingStoreClerk)
 {
 StoreClerkOPEN = true;
 SCText.SetActive(false);
 Store.SetActive(true);
 Player.GetComponent<SC_FPSController>().enabled = false;
 Cursor.lockState = CursorLockMode.None;
 Cursor.visible = true;
 }
 if (StoreClerkOPEN) 
 {
 SCText.SetActive(false);
 } 
 }
}

A short and easy code to detect if the player is looking at the store clerk and if they are a text pops up and they can click to interact. Problem is the code got slightly complicated and the text and raycast tend to glitch.

• pacmaninbw: Is this the complete class? It would be helpful to see the definitions of MonoBehaviour and GameObject.

Answer 1

 SCText.SetActive(true);
 LookingStoreClerk = true;
 ...
 SCText.SetActive(false);

You never set LookingStoreClerk to false.

the text and raycast tend to glitch.

Not exactly sure what that means, but perhaps a player who is near the maxDistance radius will register "hit" and "no hit" in rapid succession. Consider wrapping some hysteresis around it.

More generally, protect the .Raycast() call with a guard.

too laggy

maxDistance is conveniently small. If we know we're outside a bounding box, or Manhattan distance, or Euclidean distance, then a cheap boolean test will often be able to save us an expensive raycast probe.

Additionally, when SCText is active there's no point in making any further raycast probes, since nothing new will happen even if we do get a hit.

comments lie!

 // if raycast hits, it checks if it hit an object with the tag Player
 if(Physics.Raycast( ... ) &&
 hit.collider.gameObject.CompareTag("StoreClerk"))

The comment suggests one thing, while the code says another.

dead code

 StoreClerkOPEN = true;
 SCText.SetActive(false);
 ...
 if (StoreClerkOPEN) 
 {
 SCText.SetActive(false);
 } 

I don't understand why we have that last line at all. There's only one way to become OPEN, and it called .SetActive(), so that final call to .SetActive() appears to be a no-op.

Answer 2

You could simplify the code by reducing redundant checks and combining conditions.

using UnityEngine;
public class StoreClerkOpen : MonoBehaviour
{
 public GameObject StoreClerk, Store, Player, SCText;
 float maxDistance = 10;
 bool LookingStoreClerk, StoreClerkOPEN;
 void FixedUpdate()
 {
 RaycastHit hit;
 if (Physics.Raycast(transform.position, transform.forward, out hit, maxDistance) && hit.collider.CompareTag("StoreClerk"))
 {
 HandleRaycastHit();
 }
 }
 void HandleRaycastHit()
 {
 SCText.SetActive(!StoreClerkOPEN);
 LookingStoreClerk = true;
 if (Input.GetMouseButtonDown(0))
 {
 StoreClerkOPEN = true;
 Store.SetActive(true);
 Player.GetComponent<SC_FPSController>().enabled = false;
 Cursor.lockState = CursorLockMode.None;
 Cursor.visible = true;
 }
 }
}

This example eliminates some redundancy and groups related actions into a separate method (HandleRaycastHit). It should be easier to manage and less prone to glitches.