Question Team dependent material

Hi. I can't figure out if this code is wrong or not. I'm new, so I'd really appreciate replies:

Code (CSharp):

1.  
2. public class TeamBasedMaterial : MonoBehaviour
3. {
4.     public TeamBasedMaterial()
5.     {
6.     }
7.  
8.     public void Awake()
9.     {
10.         base.GetComponent<SkinnedMeshRenderer>().material = this.BlueMaterial;
11.         base.GetComponent<SkinnedMeshRenderer>().material = this.RedMaterial;
12.     }
13.  
14.     public void SetTeam(Team p_team)
15.     {
16.         if (this.SkinnedMesRenderer == null)
17.         {
18.             Debug.LogWarning("couldn't find any skinned mesh renderer for TeamBasedMaterial.");
19.             return;
20.         }
21.         if (p_team == Team.MFA)
22.         {
23.             this.SkinnedMesRenderer.sharedMaterial = this.BlueMaterial;
24.             return;
25.         }
26.         this.SkinnedMesRenderer.sharedMaterial = this.RedMaterial;
27.     }
28.  
29.     public Material RedMaterial;
30.  
31.     public Material BlueMaterial;
32.  
33.     public SkinnedMeshRenderer SkinnedMesRenderer;
34. }
35.  

I wanted it to change the material depending on the team

 

So are you asking simple if the code works or are you asking for a code review? If the former, well the easiest way would be to run it and see.

If you want a review though I'll gladly give one

1) It's tradition to declare your variables above the code that uses them. Technically not that important but it makes things confusing to seasoned people that have been seeing code organized this way for decades.

2) The BlueMaterial and RedMaterial are both being set in the Awake method and also publicly accessible and serializable. Pick one or the other. There is more risk of confusing yourself and not understanding why it doesn't work the way you'd expect when it's like this. If you want to make the materials automatically assigned yet readable from other code then use properties with a public get and a private set and then continue to assign them as you are in Awake().

3) The use of 'base' in the awake method is really weird. Typically you only use it when you've specifically overridden a method or property on a more derived class yet for some reason you specifically need to access the base version. That isn't the case here so it's much better to use 'this' or simply use nothing at all since most people familiar with Unity will understand that it is derived from MonoBehaviour.

4) You have a constructor that is empty. No need for it if it's not doing anything. Also, constructors are a bit weird in Unity. Contrary to popular belief you can use constructors for Unity objects but you also need to be aware that any serialized values will be reset after the constructor has been called. It may also be possible for Unity to call the constructor multiple times during the creation of the script object.

5) The null check inside of SetTeam is probably not needed. Personally I just let null references throw exceptions in cases like this so that I know immediately when they happen during testing and development. In a release build you would ideally want to have tested enough by this point to know that it will *never* be null or if null is a distinct possibility then you'd want a much more robust system for handling the problem rather than printing a debug message that no one will ever see. Perhaps the best alternative is to use Assert.IsNotNull() from UnityEngine.Assertions. This will display a debug message for you that can also be seen in dev builds but the code gets conditionally compiled out for the final build so that you don't pay the cost of the null check or the extra logic that will never matter anyway.

Hopefully some of that will help you as you learn and grow. Keep it up!

 

Wow, thank u for such detailed reply. This code sometimes works, but sometimes it doesn't work. It's just not assigning the correct material and there are no errors in the log. Is there any way to determine what's causing this issue based on that code? It's assigned to the gameobject and skinned mesh renderer, I've tried a lot of things and ended up posting here

 

So I'm not entirely sure what I was thinking on point 2 in my previous post. But what I said makes no sense. I'll blame it on the twelve hour shift I'd just return home from Anyway, what I *really* should have said is that you don't assign the skinned mesh rendere variable anywhere.

Which brings me to your last question. Your issue is likely due to the fact that you never assign that value. The best place would likely be in Awake(). As for how to debug code more easily, take a look for how to debug in Unity https://docs.unity3d.com/Manual/ManagedCodeDebugging.html. Using the debugger will allow you to step through code line by line as it runs and inspect the state of your data as you follow the logic.