Question How to correctly change sprite on a prefab clone?

Hello Forum,
I have made a script so that when the Linecast between the mouse cursor and the player hits a gameobject ("Int_Molecule") it changes its sprite into a new one.

Code (CSharp):

1. public class CursorXhair : MonoBehaviour
2. {
3.     public Player Player;
4.     public AttachmentController Molecule;
5.     //public SpriteRenderer spriteRenderer;
6.     public Sprite newSprite;
7.     public Sprite originalSprite;
8.  
9.     // Update is called once per frame
10.     void Update()
11.     {
12.         Vector2 playerPos = Player.transform.position;
13.         Vector2 mousePos = Camera.main.ScreenToWorldPoint(Input.mousePosition);
14.         Vector2 direction = mousePos - playerPos;
15.  
16.         transform.position = mousePos;
17.  
18.         Debug.DrawRay(playerPos, direction);
19.         RaycastHit2D mouse2Player = Physics2D.Linecast(mousePos, playerPos);
20.  
21.         if (mouse2Player.collider != null && mouse2Player.collider.tag == "Int_Molecule")
22.         {
23.             Debug.Log("mouse2Player is colling");
24.             Molecule.spriteRenderer.sprite = newSprite;
25.         }
26.         else
27.         {
28.             Molecule.spriteRenderer.sprite = originalSprite;
29.             Debug.Log("mouse2Player is NOT colling");
30.         }
31.     }
32. }

Since I need multiple clones instantiation of this same object I create this script and assign it to a LevelManager. So I create a prefab as the original object to instantiate. The Linecast works as expected as you can see in the two images, however the clones do not update the sprite as expected. Interestingly the prefab in the prefab folder does update (see the last image), but the update is not reflected in the running game and the clones instantiations.

Code (CSharp):

1. public class MoleculeInstantiator : MonoBehaviour
2. {
3.     public GameObject molecule;
4.     //public Transform moleculeLocation;
5.    
6.     // Start is called before the first frame update
7.     void Start()
8.     {
9.         Instantiate(molecule, new Vector2(-1, 6), Quaternion.identity);
10.         Instantiate(molecule, new Vector2(-11, 2), Quaternion.identity);
11.         Instantiate(molecule, new Vector2(9, -6), Quaternion.identity);
12.         Instantiate(molecule, new Vector2(14, 3), Quaternion.identity);
13.         Instantiate(molecule, new Vector2(-14, -7), Quaternion.identity);
14.         Instantiate(molecule, new Vector2(-17, 6), Quaternion.identity);
15.         Instantiate(molecule, new Vector2(17, -9), Quaternion.identity);
16.     }
17.  
18. }

Is there something I am doing wrong or in the wrong order?

I am thinking that the best way would be to have the two scripts above as one attached to the prefab (and therefore the clones), but I preferred to keep the two scripts separated for better tidiness.

Would that solve it or am I missing the point?

 

That means you are manipulating the Prefab itself, not the cloned instance.

Usually one finds the thing you hit by doing a GetComponent<T>() on the collider returned by the hit, if it hits stuff.

This means you need another way to set it back when you DON'T hit.

One approach is to make a "contact manager" that basically knows all the molecules in play, and iterates them all, asking if you hit them, and then setting either yes or no.

Alternately you can have a molecule assume if it hasn't heard that is hit this frame, it reverts to the other shape, but then you need to make sure things execute in the correct order. Without seeing more of your setup I'm not sure which would be better.

 

Thanks for your answer Kurt,
I haven't really used GetComponent with collider before because I am not confident with the syntax for it and in one occasion I have been discouraged to use it. How would you write that script?

Anyway, this is my simple setup:

The Player (white square) can:

• Move with inputs
• Stick to molecules by Raycast from "detectors" and moving them to "holders" (and a few other things)
• Eject molecules by by going through a list (and a few other things)
• Change molecules tags

You can see the cursor (orange dot) script in the post above (CursorXhair)
Same for the LevelManager, which is just an empty object with the MoleculeInstantiator script you can read above.

I doubt the Enemy and the Membrane (a door) have anything to do with this issue...

Finally the Int_Molecule (green square) (I should rename it just "molecule") is the prefab that I clone, so it is not usually in the scene hierarchy. It can, with a similar method to the player, stick to other molecules by Raycast from detectors and moving them to holders. It also posses a SpriteRenderer for the purpose of changing its sprite (which is the problem I am trying to fix).

Does this give you enough info to tell me which one would be the best approach?

 

I finally solved (with tsome external help):

- First I use Linq to query a List of SpriteRenderers (collidingObjects)
- I added a mask to exclude the player from the methods,

- Instead of just a RaycastHit2D I am using a local "emumerable" variable hitObjects that I can go through later,
Now I combine various Linq features in logical order, so:
I select all of the hit objects SpriteRenderer, Where there is a SpriteRenderer and Where the tag is "Int_Molecule"
- I also get the Spriterenderer of the first hit object to use later

As an insurance I loop through the spriteRenderers in the hitObjects and if I find the colldingObjects don't have a spriteRenderer I add one.

- I make a new enumerable variable noLongerHitObjects which is collidingObjects except hitObjects
I loop through these objects "i" times to select the "i"th object, then its spriteRenderer goes back to the original and I make sure it gets removed from the collidingObject.
I do this as long as there are colldingObjects.

I order the colldingObjects based on their distance from the mouse and put the closer first, that's the closestObject, I set its sprite to the new one.

Finally I loop trough the collindObjects that are not closestObjects for spriteRenderers, and I make sure that one remains the same.

Code (CSharp):

1. using System;
2. using System.Collections.Generic;
3. using System.Linq;
4. using UnityEngine;
5.  
6. public class CursorXhair : MonoBehaviour
7. {
8.     public Player Player;
9.     public AttachmentController Molecule;
10.     public Sprite newSprite;
11.     public Sprite originalSprite;
12.     List<SpriteRenderer> collidingObjects = new List<SpriteRenderer>();
13.     private LayerMask mask;
14.    
15.     private void Awake()
16.     {
17.         mask = ~(1 << LayerMask.NameToLayer("Player"));
18.     }
19.  
20.     void Update()
21.     {
22.         Vector2 playerPos = Player.transform.position;
23.         Vector2 mousePos = Camera.main.ScreenToWorldPoint(Input.mousePosition);
24.         Vector2 debugDirection = mousePos - playerPos;
25.  
26.         transform.position = mousePos;
27.  
28.         Debug.DrawRay(playerPos, debugDirection);
29.    
30.         var hitObjects = Physics2D.LinecastAll(mousePos, playerPos, mask)
31.             .Select(_ => _.collider.GetComponent<SpriteRenderer>())
32.             .Where(_ => _ != null)
33.             .Where(_ => _.CompareTag("Int_Molecule"));
34.  
35.         var hitObject = Physics2D.Linecast(mousePos, playerPos, mask);
36.  
37.         if (hitObject.collider != null)
38.         {
39.             hitObject.collider.GetComponent<SpriteRenderer>();
40.         }
41.        
42.         foreach (var spriteRenderer in hitObjects)
43.         {
44.             if (!collidingObjects.Contains(spriteRenderer))
45.             {
46.                 collidingObjects.Add(spriteRenderer);
47.             }
48.         }
49.  
50.         var notLongetHitObjects = collidingObjects.Except(hitObjects).ToList();
51.         for (var i = 0; i < notLongetHitObjects.Count; i++)
52.         {
53.             notLongetHitObjects[i].sprite = originalSprite;
54.             collidingObjects.Remove(notLongetHitObjects[i]);
55.         }
56.  
57.         if (collidingObjects.Count == 0)
58.         {
59.             return;
60.         }
61.  
62.         var closestObject = collidingObjects.OrderBy(_ => (_.transform.position - (Vector3)mousePos).sqrMagnitude).First();
63.         closestObject.sprite = newSprite;
64.  
65.         foreach (var spriteRenderer in collidingObjects.Where(_ => _ != closestObject))
66.         {
67.             spriteRenderer.sprite = originalSprite;
68.         }
69.     }
70. }

How does it look? It seems to me that the code is very tight, maybe too much since now I wanted to add other features, like ejecting the highlighted new sprite objects.

 

This is the problem with Linq. Generally it results in extremely brittle unchangeable code.

You're not fooling the computer either. It's going to do everything one step at a time. By chaining Linq statements you don't unlock some crazy new CPU instructions or compiler optimizations. That's not a thing.

In fact, by combining chained statements, the only thing you have done is prohibited yourself from debugging intermediate stages when you have a bug, and made it less likely you can understand your code two months from now when you have a bug in it.

But hey, some people just love Linq. I'm not one of them. I don't see the value proposition for simple stuff like this, and for complicated stuff, it results in code that essentially prevents reasoning about what is actually happening.

Your mileage may vary.

 

I have started to rewrite the script and I even included some Linq

Code (CSharp):

1. using System;
2. using System.Collections.Generic;
3. using System.Linq;
4. using UnityEngine;
5.  
6. public class CursorXhair : MonoBehaviour
7. {
8.     public Player Player;
9.     public AttachmentController Molecule;
10.     public Sprite newSprite;
11.     public Sprite originalSprite;
12.     List<SpriteRenderer> collidingObjects = new List<SpriteRenderer>();
13.  
14.     void Update()
15.     {
16.         Vector2 playerPos = Player.transform.position;
17.         Vector2 mousePos = Camera.main.ScreenToWorldPoint(Input.mousePosition);
18.         Vector2 direction = mousePos - playerPos;
19.  
20.         transform.position = mousePos;
21.  
22.         Debug.DrawRay(playerPos, direction);
23.         var hitObjects = Physics2D.Linecast(mousePos, playerPos);
24.  
25.         if (hitObjects.collider != null && hitObjects.collider.tag == "Int_Molecule")
26.         {
27.             Debug.Log("hitObjects is colliding");
28.             hitObjects.collider.GetComponent<SpriteRenderer>().sprite = newSprite;
29.             collidingObjects.Add(hitObjects.collider.GetComponent<SpriteRenderer>());
30.  
31.         }
32.         else if (collidingObjects.Count >= 1 && hitObjects.collider.tag == "Player")
33.         {
34.             Debug.Log("hitObjects is not colliding");
35.             collidingObjects.Last().GetComponent<SpriteRenderer>().sprite = originalSprite;
36.             collidingObjects.RemoveAt(collidingObjects.Count - 1);
37.         }
38.  
39.     }
40. }

The script works fine, it selects and unselect, However it should only select one molecule at a time (the first one it hits) but I am not sure how to make this so.

I thought maybe there is a limit to the max number of objects in a list, but that's not really a thing .

The cleanest thing to do would be to select only the hit molecule which is closer to the cursor, but I am not sure how to write this.

The guy who helped me made a mirror list of objects that are no longer hit, however I am not sure that is the best solution, nor how to do it.