#would i have to change much with my code

1 messages Β· Page 1 of 1 (latest)

wicked swallow
#

So re-reading your code - I think you'd want to create a separate script for buttons/interactables - possibly even named something like "Interactable", or even "Selectable". I'd give it a few simple methods along the lines of

public class Interactable : MonoBehaviour {
  [SerializeField] private Material highlightMaterial;
  [SerializeField] private Material defaultMaterial;
  public UnityEvent OnInteract;
  private Renderer renderer;

  void Start() {
    renderer = GetComponent<Renderer>();
  }

  public void Focus() {
    renderer.material = highlightMaterial;
  }

  public void Blur() {
    renderer.material = defaultMaterial;
  }

  public void Interact() {
    OnInteract?.Invoke();
  }
}
stoic igloo
#

ok

#

i put the code in a script but i'm getting an error and a warning Assets\Scripts\Interactable.cs(9,12): error CS0246: The type or namespace name 'UnityEvent' could not be found (are you missing a using directive or an assembly reference?)

#

Assets\Scripts\Interactable.cs(10,22): warning CS0108: 'Interactable.renderer' hides inherited member 'Component.renderer'. Use the new keyword if hiding was intended.

wicked swallow
#
using UnityEngine.Events;
stoic igloo
#

Oop thx

wicked swallow
#

I'd set up the raycaster thing to see if what the raycast hit has your "Selectable" tag, and if so, get a reference to it's Interactable component, and store it in private Interactable _selection (instead of storing the Transform). If _selection is already set at that time and is not the same as the one from the raycast hit - or when the raycast does not hit a "Selectable" - call _selection.Blur() to swap the currently highlighted thing's material back to default before you update _selection to the new Interactable or null

Then call _selection.Focus() on the new selection to highlight it. If the player hits E, call _selection.Interact().

That was a pretty large word dump - did it all make sense?

stoic igloo
#

but remove the part where it sets the material to highlight?

#

sorry this is kinda confusing to me cause im just getting started learning how to code

wicked swallow
#

No worries - that was sort of a lot to throw at you all at once! I'm fairly new to Unity myself and got kind of excited by the problem πŸ˜…

It would be a little more involved than that - the idea is to move everything specific to the buttons/interactables themselves to the new class, and create "an interface" between the two such that the code performing the raycasts isn't directly altering anything on the interactable game objects - it more or less just tells them what happened - Focus() (Player is looking at you), Blur() (Player stopped looking at you), and Interact() (Player is interacting with you). What the button does with that information becomes the button's business alone.

Changing the private Transform _selection; field into private Interactable _selection; is a bit in the interests of "separation of concerns" - since the raycasting code no longer should be directly altering buttons or their data, it no longer needs a reference to the button's Transform. But it does still need to know what the last button the player was looking at was, so it can tell that button to Blur() if the player stops looking at it - so it makes sense to store that Interactable instance as _selection.

wicked swallow
#

In it's place, you'll tell the button what happened - where you'd set the button material to it's highlighted material, now you'd instead just the tell the button it's being looked at (_selection.Focus()). Where you'd reset the material to it's default, you'd now tell the button the player is no longer looking at it (_selection.Blur())

#

And Debug.Log("INTERACTED"); // check if interacted would just be replaced with _selection.Interact()

stoic igloo
#

ok

#

so far i got this

#

im getting an error on this though

wicked swallow
#

Cannot cast Transform to Interactable?

stoic igloo
#

on line 35 the selection is underlined red

wicked swallow
#

Getting closer 😁

stoic igloo
#

yes

#

it is

wicked swallow
#

So instead of looking up the Renderer component, you'll want to look up the Interactable component now... You might remove the variable for the transform altogether now, since it's only needed a couple times:

if(hit.transform.CompareTag(selectableTag)) {
  var selection = hit.transform.GetComponent<Interactable>();

and selectionRenderer itself can also be removed

stoic igloo
#

ok

#

this is what i got so far

#

i wasn't sure what to put in place for selectionRenderer so i put in selection not sure if thats correct though

#

im also getting 1 error called: Assets\Scripts\SelectionManager.cs(29,21): error CS0136: A local or parameter named 'selection' cannot be declared in this scope because that name is used in an enclosing local scope to define a local or parameter

wicked swallow
#

Yeah that's looking good πŸ‘Œ. There are a few other issues that will need addressing - but it's pretty much what I described

stoic igloo
#

ok nice

wicked swallow
stoic igloo
#

oh ok

#

fixed it

wicked swallow
#
        if (_selection != null) // reset to default material when looking away from object
        {
            _selection.Blur();
        }

So the effect of this being right inside Update() is that the button currently getting looked at will swap it's material back to the default every frame. We only want that to happen once, when the player stops looking at the button

#

You can change it to an else if (_selection != null) and throw it after the if(Physics.Raycast( //... conditional, such that it will only blur the currently looked at button if the raycast misses. You'll also still want to reset _selection to null in that case, to indicate that the player is no longer looking at a button

stoic igloo
#

ok this what i got now

wicked swallow
#

But that should also take effect if the raycast hits something that is not a button. Luckily, when you have two if() statements nested with no other logic directly within the outer if() apart from the nested if(), you can just combine them, if there's no other reason to keep them separated. So you might just change

        if (Physics.Raycast(ray, out hit, interactDistance))
        {
            if (hit.transform.CompareTag(selectableTag)) // check if tag is selectable
            {
              // ...
            }
        }
        else if (_selection != null) {
          // ...
        }

into

        if (Physics.Raycast(ray, out hit, interactDistance) && hit.transform.CompareTag(selectableTag)) // check if tag is selectable
          // ...
        }
        else if (_selection != null) {
          // ...
        }
#

I think one of the last things to address might be the condition that the raycast hits a different button from the current _selection, like if two buttons were right next to eachother and the player moved from looking at one to the other, in which case the previous one would need to be blurred before you update _selection

wicked swallow
#

Well in the code the local variable selection refers to the new button, and the class field _selection refers to the old button (if any) - so you can check them against each other to see whether or not they're the same, and if not, blur the old button before you loose your reference to the old button when you _selection = selection

stoic igloo
#

so i just need to add this somewhere in my void update? _selection = selection

wicked swallow
#

A little more than that - you already have that in your code. I would change

        if (Physics.Raycast(ray, out hit, interactDistance) && hit.transform.CompareTag(selectableTag))
        {
            var selection = hit.transform.GetComponent<Interactable>();
            if (selection != null)
            {
                _selection.Focus();
            }

            _selection = selection;

            if (selection != null && Input.GetKeyDown(KeyCode.E))
            {
                _selection.Interact();
            }
        }

to something more like

        if (Physics.Raycast(ray, out hit, interactDistance) && hit.transform.CompareTag(selectableTag))
        {
            var selection = hit.transform.GetComponent<Interactable>();
            if (selection != _selection) // If the button the player's looking at changed...
            {
                if (_selection != null) // If the player was previously looking at a button, blur it.
                    _selection.Blur();

                if (selection != null) // If the player's currently looking at a button, focus it.
                    selection.Focus();

                _selection = selection; // Update _selection to refer to the new button (or null if there was no Interactable component on the thing the ray hit)
            }

            if (_selection != null && Input.GetKeyDown(KeyCode.E))
            {
                _selection.Interact();
            }
        }
#

By putting all of the blur/focus logic there into a check to see if the button changed, it also ensures that the buttons will only swap materials once, during the frame that the player changed whatever they're looking at, rather than continuously every frame that the ray hits a button

#

And I think that's everything πŸ€”

It was a much larger refactor than I originally thought - but I feel like it's made everything a bit more simple? Maybe it doesn't seem like it at the moment, for the sort of rapid refactor 😁

stoic igloo
#

ok this is what i got now

stoic igloo
wicked swallow
#

You lost the

        else if (_selection != null) // reset to default material when looking away from object
        {
            _selection.Blur();
            _selection = null;
        }

but otherwise I think it should be good to go πŸ‘Œ

stoic igloo
#

is this good?

wicked swallow
#

Looks good to me πŸ‘

stoic igloo
#

ok so

#

i have a few quick questions

wicked swallow
#

fire away

stoic igloo
#

for the Interactable script that u showed me earlier

#

i put that on each button right?

wicked swallow
#

Correct, yep!

stoic igloo
#

and for the SelectionManager script i put that on a empty game object still?

wicked swallow
#

Yeah that can be wherever, since it's just casting from the camera remotely

stoic igloo
#

also from my interactable script im getting this warning: Assets\Scripts\Interactable.cs(11,22): warning CS0108: 'Interactable.renderer' hides inherited member 'Component.renderer'. Use the new keyword if hiding was intended.

wicked swallow
#

oh oops

stoic igloo
#

it has the "renderer" in private Renderer renderer; underlined in green

wicked swallow
#

I didn't realize that was a member on Component - renderer in the Interactable class should be renamed to something else

#

We could do what the error mentions and use the new keyword, but I feel it's generally better to avoid the name collision unless you have a good thought out reason to do otherwise - it would be unfortunate for something to try to access renderer on that script and expect whatever Component.renderer is, but get our field instead

stoic igloo
#

wait im a little confused

#

do i need to rename renderer in the component.renderer to something else or renderer in the private Renderer renderer;

#

or leave it like that?

wicked swallow
#

The private Renderer renderer;

stoic igloo
#

does it matter what i rename it to?

wicked swallow
#

nope - whatever you feel like. Just be sure to update wherever it's used in the code as well

stoic igloo
#

ok

#

i changed it to render and updated it everywhere else

wicked swallow
#

If you're using Visual Studio, you can place your cursor on the variable name, and hit Ctrl + R and then Ctrl + R again - whatever you type as the new variable name will update wherever it's used

stoic igloo
#

oh cool

#

cool*

wicked swallow
stoic igloo
#

i didn't know that

#

noice

wicked swallow
#

it's a nice time saver!

stoic igloo
#

also for the script that goes on button

#

in the inspector window what do i have to do for the On Interact () part

wicked swallow
# stoic igloo in the inspector window what do i have to do for the On Interact () part

Click the + button to add a new callback, then drag a gameobject from the scene which has a script attached that exposes a public method which does whatever you want it to do. So say you want to open a door when the button's pressed, and your door gameobject has a DoorController script on it that has a public void OpenDoor() method. You'd drag the door game object to the GameObject field for the UnityEvent callback, then select DoorController > OpenDoor() from the Function drop-down

stoic igloo
#

oh ok that's cool

#

well thank you so much for the help!!

#

i really appreciate it a ton!

wicked swallow
#

Yeah no problem! Thanks for a good exercise 😁

#

As a final few thoughts on all of this - maybe some room for future improvement:

  • GetComponent() can be a little expensive, you might want to avoid calling it every frame. It's not a big concern presently, but you could get around that by storing the hit thing's transform after all (I hadn't thought that through all the way, originally), and comparing that to the new transform before you GetComponent<Interactable>(), to avoid performing that look-up unless it's absolutely necessary.

  • At some point you might consider refactoring Interactable to be an "abstract class" that just defines the interface or "contract" for what kind of functionality all things that you can interact with all have in common. Like maybe at some point you want to make a creature play a funny animation when you look at it, but not swap it's materials or anything. So you could have both InteractableButton and InteractableAnimation scripts which do wildly different things without adding all those fields for that varied functionality to Interactable, yet SelectionManager could handle and trigger them both without a single change to it's code, because it only cares that they have the methods which their base class Interactable declares.

  • Alternately or in addition to the abstract class, you could also expose UnityEvents for OnFocus() and OnBlur(), so you could hook up things like swapping the material without hard-coding them into Interactable