Need help figuring out the source of my Physics.Raycast bug

Unity Engine
1

http://makeagif.com/VAAbvB

There is the code and a GIF illustrating what is happening.

The code in concern is the HighlightTiles method. It is simply supposed to highlight the tile while the raycast is hitting it and then unhighlight the tile once the raycast is no longer hitting it. From the GIF you can see that it works perfectly fine as long as I am moving the mouse slowly over the tiles. Once I speed up the movement it won’t unhighlight tiles. I’m not quite sure why but I was thinking it is because the update is being call before it can complete the HighlightTiles method?? I’ve only been programming for a few weeks so I’m not really sure how to troubleshoot these kind of problems yet. Do you think my theory is correct? If so, what would you think would fix it? I don’t really want anyone to code anything for me, just looking for a nudge in the right direction. :slight_smile: cheers!

2

Replace “previousTile = null;” at line 154 with “previousTile = highlightedTile” then remove the first conditional at 146 completely. Put “previousTile.GetComponent().material.color= originalColor;” in an additional If conditional for “if previousTile != null”.

There’s also no reason for this to be in a “while” loop, since it never loops- just change it to an If statement instead, and change the bottom conditional “if(highlightedTile !=null)” to just “else”.

The end result will look something like this:

void HighlightTiles()
   {
       Ray hRay = Camera.main.ScreenPointToRay(Input.mousePosition);
       RaycastHit hHitInfo;
       int ground_LayerMask = 1 << 9;

       if(Physics.Raycast(hRay, out hHitInfo, Mathf.Infinity, ground_LayerMask))
       {
            highlightedTile = hHitInfo.collider.gameObject;

           if(highlightedTile != previousTile)
           {
                if(previousTile != null)
                    previousTile.GetComponent<MeshRenderer>().material.color = originalColor;
                previousTile = highlightedTile;
           }

           if(originalColor == blankColor)
           {
                originalColor = highlightedTile.GetComponent<MeshRenderer>().material.color;
           }

            highlightedTile.GetComponent<MeshRenderer>().material.color = highlightColor;
       }
       else
       {
            highlightedTile.GetComponent<MeshRenderer>().material.color = originalColor;
            highlightedTile = null;
            previousTile = null;
       }
   }

or, alternatively and if you don’t actually need “previousTile” for anything else, you can simplify this quite a bit:

   void HighlightTiles()
   {
       Ray hRay = Camera.main.ScreenPointToRay(Input.mousePosition);
       RaycastHit hHitInfo;
       int ground_LayerMask = 1 << 9;

       if(Physics.Raycast(hRay, out hHitInfo, Mathf.Infinity, ground_LayerMask))
       {

           if(highlightedTile == hHitInfo.collider.gameObject)
               return;

           if(highlightedTile != null)
           {
               highlightedTile.GetComponent<MeshRenderer>().material.color = originalColor;
           }

           highlightedTile = hHitInfo.collider.gameObject;

           originalColor = highlightedTile.GetComponent<MeshRenderer>().material.color;
           highlightedTile.GetComponent<MeshRenderer>().material.color = highlightColor;
       }
       else
       {
           if(highlightedTile != null)
           {
               highlightedTile.GetComponent<MeshRenderer>().material.color = originalColor;
               highlightedTile = null;
           }
       }
   }
1 Like
3

Awesome! With a slight tweak, yours worked perfectly. The else statement wasn’t needed. Plus, with it in it gave a Nullreference error since highlightedTile was defined yet. Thanks for the help!! :slight_smile:

Also, could you explain how you came about to changing the code that way? Now that I see it, it makes perfect sense but I’m curious as to how I was going about it the wrong way… if that makes sense lol

4

EDIT: The else statement is actually needed because otherwise the “last highlighted” will stay highlighted when you move the cursor off of all tiles, until you move it back onto the tiles again. I actually edited it just before you posted back to remove a few more unnecessary things, and add the conditional, though I suppose I did only add it to the simplified version… Look at the “else” in the simplified version for how this should work.

As for how I came up with the changes, it’s just a simple logic chain. You eliminate the simplest conditions first- in my “simplified” version for instance, if the highlighted object is the same as the previously highlighted object, there’s no need to do anything, so just exit the whole function. This also saves on unnecessary processing and cuts down on mistakes that can happen. Second off, take into account nulls right away, what if the previous was null? The current can’t be null since we’re in a raycast conditional, so now we can operate under certain assumptions: if you get to this point, you’re not null, and if you get to this point, you’re a different tile than the previous one.

From there, the changes you need to make are easy. Always keep the most complicated option for last, and make sure you eliminate any other (simpler) options before you get there.

5

Ah yes, my mistake. I didn’t even notice that when I tested it. And thank you again, it was pretty insightful! Seems like I was on the right track but just need more practice. :slight_smile: