#would i have to change much with my code
1 messages Β· Page 1 of 1 (latest)
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();
}
}
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.
using UnityEngine.Events;
Oop thx
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?
ok uh so i'd need to add this in a private void Update() https://gdl.space/soposodazi.cs
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
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.
All of the stuff regarding materials can be removed from the SelectionManager, as buttons will manage their materials on their own now (which also allows you use different materials for different buttons, if you desire!)
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()
Cannot cast Transform to Interactable?
on line 35 the selection is underlined red
Getting closer π
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
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
Yeah that's looking good π. There are a few other issues that will need addressing - but it's pretty much what I described
ok nice
You'll just need to delete the other var selection = hit.transform; from right above of the tag-check conditional - C# doesn't like that there's two local variables being declared with the same name there
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
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
ok i think i got this in
noice
how do i fix that?
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
so i just need to add this somewhere in my void update? _selection = selection
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 π
it seems like it makes some more sense now yah
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 π
Looks good to me π
fire away
for the Interactable script that u showed me earlier
i put that on each button right?
Correct, yep!
and for the SelectionManager script i put that on a empty game object still?
Yeah that can be wherever, since it's just casting from the camera remotely
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.
oh oops
it has the "renderer" in private Renderer renderer; underlined in green
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
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?
The private Renderer renderer;
does it matter what i rename it to?
nope - whatever you feel like. Just be sure to update wherever it's used in the code as well
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
Excellent π
it's a nice time saver!
also for the script that goes on button
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
oh ok that's cool
well thank you so much for the help!!
i really appreciate it a ton!
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 youGetComponent<Interactable>(), to avoid performing that look-up unless it's absolutely necessary. -
At some point you might consider refactoring
Interactableto 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 bothInteractableButtonandInteractableAnimationscripts which do wildly different things without adding all those fields for that varied functionality toInteractable, yetSelectionManagercould 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 classInteractabledeclares. -
Alternately or in addition to the abstract class, you could also expose
UnityEvents forOnFocus()andOnBlur(), so you could hook up things like swapping the material without hard-coding them intoInteractable