How to shorten my code?

Hi, I'm new to coding, and I would like to know how I make it so I don't have to type the same code again and again to do something. This is what I want to "Shorten" :

Code (csharp):

1. void OnTriggerEnter (Collider other)
2.     {
3.         if (other.gameObject.CompareTag ("Green Rupee"))
4.         {
5.             other.gameObject.SetActive (false);
6.         }
7.  
8.         if (other.gameObject.CompareTag ("Blue Rupee"))
9.         {
10.             other.gameObject.SetActive (false);
11.         }
12.  
13.         if (other.gameObject.CompareTag ("Red Rupee"))
14.         {
15.             other.gameObject.SetActive (false);
16.         }
17.  
18.         if (other.gameObject.CompareTag ("Purple Rupee"))
19.         {
20.             other.gameObject.SetActive (false);
21.         }
22.  
23.         if (other.gameObject.CompareTag ("Silver Rupee"))
24.         {
25.             other.gameObject.SetActive (false);
26.         }
27.  
28.         if (other.gameObject.CompareTag ("Yellow Rupee"))
29.         {
30.             other.gameObject.SetActive (false);
31.         }
32.     }
33.  

Any help is greatly appreciated!

 

Use code tags, please.

 

I've never used a form before, what do you mean?

 

https://forum.unity.com/threads/using-code-tags-properly.143875/

As far as your question goes, you could add a script to your rupee that you can get the value of that rupee from vs checking for all the possible color options.

 

I will try that, and thank you for the link about code tags.

 

Code (CSharp):

1. ISet<string> Rupees = new HashSet<string>
2. {
3.     "Blue Rupee",
4.     "Red Rupee",
5.     "Silver Rupee",
6.     "Purple Rupee",
7.     "Yellow Rupee"
8. };
9.  
10. //...
11.  
12. if (Rupees.Any(gameObject.CompareTag))
13. {
14.     gameObject.SetActive(false);
15. }

 

I'm assuming he wants a value of the Rupee which is why OP had an if statement to compare which one was picked up.

 

In that case, he can just replace 'Any' with 'FirstOrDefault' then

 

Put the OnTriggerEnter method on the rupee instead. Assuming the player is the one triggering.

Code (csharp):

1.  
2. void OnTriggerEnter(Collider other)
3. {
4.     if (other.gameObject.CompareTag("Player"))
5.         gameObject.SetActive(false);
6. }
7.  

 

I agree with this the most.

Why not create a "collectable" component script and attach that to anything you want to be collectable instead?

 

Yep, there are many ways to do this, but some of it depends on how much more you plan to do with this. Simply turning off the object isn't a big deal, but I'm assuming you'll have various values, maybe other pickups, etc that you'll need to have additional stuff to deal with.

A pickup script on the object itself is one of the better ways to do this.

 

I'd agree on @KelsoMRK and just do it the other way around. This could be applied to anything that needs the same behaviour without caring about lots of different tags, it's easily extensible and maintainable since you do not need to adjust any tag collection, and you cannot forget to 'tag' a prefab / a go in general as this single comparison covers it all.

However, if you need more behaviour in any way, like triggering certain behaviour for different kinds of 'rupees' or other items, you should follow the suggestion about the component anyway, which can eliminate the need for the tag in many situations completely.

Also, if you feel like that's not what you want, you could start searching for patterns, i.e. (in it's simplest form) using the 'Contains', 'StartsWith', 'EndsWith' methods of strings. But honestly, this wouldn't be as reliable as the recommendation already given in this thread, and it's just as bad in regards to reliability, maintainability and extensibility as the other suggestions.

 

Thank you everyone for your suggestions and help.