how can i shorten this code? [Solved]

i am new to scripting i know that there is a better way to write this here is the script :


public GameObject GameobjectName;
public string GameobjectTag;


public GameObject Item1;
public GameObject Item2;
public GameObject Item3;


public int WoodenPlanks = 0;
public int Log = 0;
public int HalfLog = 0;


void Update()
{
if (Input.GetKeyDown(KeyCode.E))
{
if (GameobjectTag == "WoodenPlank")
{
GameobjectName.gameObject.SetActive(false);
WoodenPlanks = WoodenPlanks + 1;
}
}
if (Input.GetKeyDown(KeyCode.E))
{
if (GameobjectTag == "Log")
{
GameobjectName.gameObject.SetActive(false);
Log = Log + 1;
}
}
if (Input.GetKeyDown(KeyCode.E))
{
if (GameobjectTag == "HalfLog")
{
GameobjectName.gameObject.SetActive(false);
HalfLog = HalfLog + 1;
}
}


// that's the code is there a smarter way to put the code? instead of having to do if statement for each GameobjectTag?
thank you!

 

http://forum.unity3d.com/threads/using-code-tags-properly.143875/


you might also want to consider http://www.dofactory.com/reference/csharp-coding-standards

 

you can compact things into a single "if input e" and I'd probably go with a switch statement

Code (csharp):

1.  
2. if (Input.GetKeyDown(KeyCode.E))
3. {
4.     switch (GameobjectTag )
5.     {
6.         case "myTag1":
7.                 // stuff
8.                 break;
9.         case "myTag2":
10.                 // other stuff
11.                 break;
12.         default:
13.                 // stuff if it's totally the wrong tag!
14.                 break;
15.     }
16. }
17.  

 

Thank you so much LeftyRighty ! i needed the switch statement in my life!

 

You may also consider putting the ItemX into an array, instead of individual variables

Code (csharp):

1. public GameObject Item1;
2. public GameObject Item2;
3. public GameObject Item3;

becomes

Code (csharp):

1. public GameObject Items[] = new GameObjects[3]

If you even need it at all. I don't see the ItemX's getting referenced anywhere

 

i am going to use them soon thanks for the tip Vedrit

 

I would use a collection. A dictionary seems well suited to the problem

Code (csharp):

1. public GameObject theGameObject;
2. public string theTag;
3.  
4. Dictionary <string, int> items = new Dictionary <string, int>();
5.  
6. void Update() {
7.     if (Input.GetKeyDown(KeyCode.E)) {
8.           theGameObject.SetActive(false);
9.           if (items.ContainsKey(theTag){
10.               items[theTag] = items[theTag] + 1;
11.           } else {
12.               items.Add(theTag, 1);
13.           }
14.     }
15. }

That said your way of handling the GameObject and string at the top of your script is pretty odd. Its likely to be prone to bugs down the track. I would suggest implementing raycasting inside of your if statement to figure out what the GameObject should be.

 

i do have raycast that's how i get the gameobject and gameobjecttag
thanks for the info i will check out the "dictionary" and see how it works out!

 

Yes, I agree, a dictionary is the way to go. I would also add, you should put this into a function with a good name that describes what you are doing.

Example:

Code (CSharp):

1.  
2.  
3.      public const UnityEngine.KeyCode KEYBIND_GET_ITEM = KeyCode.E;
4.      Dictionary<string, int> Inventory = new Dictionary<string, int> {
5.        { "Wooden Planks", 0 },
6.        { "Log", 0 },
7.        { "Half Log", 0 }
8.      };
9.  
10.      void Update()
11.      {
12.        if (Input.GetKeyDown(KEYBIND_GET_ITEM))
13.        {
14.          AddToInventory(gameObject);
15.        }
16.      }
17.  
18.      void AddToInventory(GameObject go)
19.      {
20.        if (Inventory.ContainsKey(go.tag))
21.        {
22.          // go is a gameObject that has a tag of 'Wooden Planks', 'Log', or 'Half Log'.
23.          Inventory[go.tag] += 1;
24.          go.SetActive(false);
25.        }
26.      }
27.  

There is now a new function, 'AddToInventory', whose name more clearly conveys what is happening.

In your original example, you have a variable called 'WoodenPlanks', and an object tag 'WoodenPlank'. This spelling difference will bite you in the future. You can avoid providing redundant information, by setting it up like above. This provides the pleasing benefit of being able to use spaces in the item name now, and automatic tag checking, which prevent non-inventory tags from being processed.

 

thank you sooo much i am still new to all of this that's why i don't have a good naming system
i can honestly say that i love unity and c# !!