Is this code too unoptimized?

[sinful crest]

I am kinda new to making my own code, so I don't exactly know what makes a good function and what makes it optimized right. If you have the time, could someone help me out and explain what I could change to make this more optimized and work more fluidly?

Overall, what I really just want help with is understanding what does or doesn't cause something to be optimized. And how I can better optimize my scripts :D
Currently the code works fine, but I assume as soon as I begin adding more and more types of nectars to have in the game it will begin to tax performance.

Thanks a bunch!

[sturdy minnow]

I wouldn't worry too much about optimization unless we're talking maybe a million amount of checks but even then for a one time operation ;p

Anyway, alternatively if you dislike seeing if statements you can do a switch statement:

switch (collision.name)
{
    case "Flower1":
        redNectar += 1;
        break;

    case "Flower2":
        blueNectar += 1;
        break;

    case "Flower3":
        greenNectar += 1;
        break;
}```

[sinful crest]

Oh sweet! Thanks a bunch man! I just hear a lot of things about optimization online so I thought it was a thing I had to worry about a lot haha :)

[sturdy minnow]

switch may be faster actually assuming c# hashes it all

otherwise you'd do a dictionary where you have a key to a value but I doubt you'll need it here

[broken bronze]

Yeah you can always go back and change the code after you've noticed & profiled the slow parts. It's much more important to focus on other things at this point

Speaking of which, it's not a good idea to use object names as data. They should only be used for labeling things in the editor, not for any functionality. You could add a component to the flowers:

class FlowerNectar : MonoBehaviour
{
    public string color;
}

and then get the color from the component:

if (TryGetComponent<FlowerNectar>(out FlowerNectar flowerNectar))
{
    if(flowerNectar.color == "red")
    {
        ...
    }
    else if(...)
}

The next improvement to that would be to use enums instead of strings

Also !code not screenshots please

[fleet prairie]

Posting code

📃 Large Code Blocks

Use links to services like:
https://paste.mod.gg/, https://hastebin.skyra.pw/, https://paste.ofcode.org/, https://paste.myst.rs/

📃 Inline Code

Surround code with three backquotes. Not quotation marks.
To format as C#, add cs to the first line:
```cs
// Your code here
```
Add a comment with a line number if there is an error message.

[broken bronze]

Also the nectar collecting speed is now tied to the fixed time step so it should be something like redNectar += Time.deltaTime * nectarCollectingSpeed and use floats for the variables

[spice crane]

I agree with Nitku. Super architecture improvement -- along with using an enum for it.

Few additional notes:

• Calling Object.name is extremely expensive in general. Not important here but useful to know.
• Use CompareTag instead when comparing tags.
• AVOID using tags in general. For collisions you get Physics Layers.
• AVOID using strings in general -- unless you feel they're needed.

A general rule of thumbs to get better at programming would be to focus on writing AS FEW lines of code as possible to produce the desired behaviour.

[sinful crest]

Thank you all so so much! This helps out majorly! I’ll definitely be taking your tips and suggestions into account when writing code from now on haha :)

[sinful crest]

Also, sorry. I didn't know :)

[broken bronze]

no worries

[vagrant locust]

there are specific condition for switches to be turned into jump table e.g : massive switch case

I don't think 4 cases or less will take that path

Object.name will always allocate new string each time you call it this is due to Unity's quirk with the interop

most same string operations in c# are interned when access repeatedly normally you should not be too concerned about this

[sharp pelican]

Just a heads up here, c# can be cute with generics where if it can learn what type your trying to use via indirect context clues you can skip explicitly declaring it, an example of this is this code

if (TryGetComponent<FlowerNectar>(out FlowerNectar flowerNectar))
{

}

can just be reduced to

if (TryGetComponent(out FlowerNectar flowerNectar))
{

}

since it can figure out the type via the explicit out value deceleration 😄

big nitpick just something i find looks a lot cleaner