Is my code efficient?

Hello there! I'm new to Unity and coding in general ( started learning 4 days before this posting )

I've written a script ( took about a day of headaches and troubleshooting to know why it wasn't working properly )

Anyway in the end I got it working, it's a simple

If player is in a trigger set InRange to true
If player presses E and IF he is InRange and IF IsActive is false then do this OTHERWISE IsActive must be true so do this.

It turns on some particle effects and lights etc but boy am I proud of it.

I'd like to know if the code i've written is as sleek as it can be and if not why and then how to go about making it work better in the future.​

Code (csharp):

1.  
2. using System.Collections;
3. using System.Collections.Generic;
4. using UnityEngine;
5.  
6. public class Interact : MonoBehaviour {
7.  
8.     public GameObject Text;
9.     public GameObject Object; // Object in this case is a particle system.
10.     public bool IsActive; // Is the object active?
11.     public bool InRange; // These are public purely so I could see in the inspector if it was triggering.
12.  
13.     private void OnTriggerEnter(Collider Enter)
14.     {
15.         InRange = true;
16.         Text.SetActive(true);
17.     }
18.  
19.     private void OnTriggerExit(Collider Exit)
20.     {
21.         InRange = false;
22.         Text.SetActive(false);
23.     }
24.  
25.  
26.     void Start ()
27.     {
28.         InRange = false;
29.         IsActive = false;
30.         Object.SetActive(false);
31.         Text.SetActive(false);
32.     }
33.  
34.  
35.     void Update()
36.     {
37.         if(Input.GetKeyDown(KeyCode.E))
38.         {
39.             if(InRange==true)
40.             {
41.                 if(IsActive==false)
42.                 {
43.                     IsActive = true;
44.                     Object.SetActive(true);  // Setting Object to true.
45.                 }
46.                 else if (IsActive==true)
47.                 {
48.                     IsActive = false;
49.                     Object.SetActive(false); // Setting Object to false.
50.                 }
51.             }
52.         }
53.     }
54. }
55.  

 

Seems okay to me.

Please refer to this page for how to post code on the forums: https://forum.unity.com/threads/using-code-tags-properly.143875/

Couple of small notes that aren't hugely important, but nice to know..
Bools can be compared like so (no difference, just an option):

Code (csharp):

1. if(IsActive)   // instead of if(IsActive==true)
2. // or
3. if(!IsActive) // instead of if(IsActive == false)
4.  

Your code could be slightly shorter, if you wrote:

Code (csharp):

1.  
2. f(Input.GetKeyDown(KeyCode.E))
3. {
4.    if(InRange)
5.    {
6.        IsActive = !IsActive;  // give the opposite of whatever it is.
7.        Object.SetActive(IsActive); // Setting Object active/inactive based on variable
8.    }
9.  }
10.  

I only wrote that to show it could be shortened a little, however I believe readability and comfort are more important than length. You should write what you're comfortable with and what (for you) is easier to understand

 

I will comment that variable names such as Text and Object are horrible choices for names. Use something meaningful.

That being said...if you want people to look at your code, use code tags https://forum.unity.com/threads/using-code-tags-properly.143875/

 

Hi,
well it depends on what you want to effect. So what type of game do you want to do. Is it something complex like multiplayer or RTS style or just Singleplayer with some interactions.

I strongly recommend not to use the Unity way of doing scripting. Maybe for beginners its ok. But if your project will start to grow (and it will for sure), it wont be easy to maintain it anymore.

I can recomand using an ECS approach...
Oh what wonder I have one
https://forum.unity.com/threads/ecs-rebuild-of-unitys-upcoming-entity-component-system-free.503239/


Ok, but why?
Its easy to maintain and flexible enough to add, remove features on the fly without breaking the hole project
And one more reason... Its super fast!

BUT:
Its a hole different programming approach! Its datadriven. You dont use OOP pattern anymore.

So what is ECS exactly:
ECS stands for Entity Component System. Basicly you have entities. An Entity will be defined by its components, which hold the data. And this is importent! Components ONLY holding Data, no functions at all!
The functionallity comes with the different systems, which will act on the data.
And each system sould do only one thing. E.g. update Positions, handle Damages, and so on
Systems always acts over all entities with the desired component on it (e.g. all Entities with the position component.)

By the fact that systems handles the behaviour, you can just add a new system or remove one to add or remove a feature. The same for components. Your player character shouldn't be run anymore because of something happend in the game. Remove a isMoveable component from the player and catch it in the PlayerInputSystem. Done

Something like that.

If you like to know more about ecs and some other approaches, I can recommend the following:

• http://www.sebaslab.com/the-truth-behind-inversion-of-control-part-i-dependency-injection/
• http://www.sebaslab.com/the-truth-behind-inversion-of-control-part-ii-inversion-of-control/
• http://www.sebaslab.com/the-truth-behind-inversion-of-control-part-iii-entity-component-systems/
• http://www.sebaslab.com/the-truth-b...ntrol-part-iv-dependency-inversion-principle/

And for completion:
Dont use ECS for UI stuff
For UI it's better to use MVC pattern (Model View Controller)

 

Please don't plug your thing to beginners with basic getting started scripting questions. It will confuse them and your code is going to be nowhere near as well supported or documented as Unity's when they inevitably ask for more help.

 

Sorry, I was going to add about Text and Object, also, but deleted it.
I would stay away from naming variables the same as classes.

And wow, @Spy-Shifty , it was only a simple question about the 1 script lol

 

I used to feel uncomfortable with projects when they got bigger. I would have been glad for such a tip. That's why I posted this. Maybe it doesn't help him for now. But maybe later on or someone else.

 

Come on guys, he just started programming. Making things actually work is what you do at this stage. You don't worry about syntax or patterns just yet

 

He did ask if his code was "sleek" I agree introducing some new system that he may have trouble getting help on is a bit over kill, but basic stuff is good to introduce so bad habits don't get developed. Bad habits can lead to bugs that leads to frustration and a person giving up.

 

Tip 1: Use code tags, otherwise people won't read your code.

Tip 2: Don't center your post what the frigg
​

Actually useful:
Tip 3: As people said, don't name your things "Object", and then put a comment next to it saying what it is. Name your particle system GameObject something like "particles". Also don't name your things the same as built-in types, like "Object".

Tip 4: Get rid of the isActive flag. Objects have an .activeSelf property which is exactly the same thing. Having two variables that contain the same data that have to be in sync will cause bugs.

Tip 5: Ignore the advice of @Spy-Shifty. You're a novice at this, and introducing a high-concept framework is the worst thing you could do right now.

 

Wow lots of replys! Thank you for the suggestions everyone, glad to see people are willing to help instead of criticize.

I'll remember the code tags if I have any future questions!

Baste I used the IsActive flag as I didn't know how to include if the game object was activated or not without making it a bool.

And god why did I center the post, looking at it now it does indeed look like arse.