Question Help with optimizing this update function

Much of this code is very similar as you can see, I was wondering if there was a way to shorten this? All of these timers represent something similar to a statemachine.

Code (CSharp):

1.     void ProcessCooldowns(){
2.         if(comboTimer > 0){
3.             comboTimer-=Time.deltaTime;
4.             if(comboTimer <= 0){
5.                 ToggleBasedOnCooldownType(CooldownType.Combo,true);
6.             }
7.         }
8.         if(moveTimer > 0){
9.             moveTimer-=Time.deltaTime;
10.             if(moveTimer <= 0){
11.                 ToggleBasedOnCooldownType(CooldownType.Move,true);
12.             }
13.         }
14.         if(stunTimer > 0){
15.             stunTimer-=Time.deltaTime;
16.             if(stunTimer <= 0){
17.                 ToggleBasedOnCooldownType(CooldownType.Stun,true);
18.             }
19.         }
20.     }

 

There's ALWAYS a way to shorten it:

- put all the code on one line.
- make all the variables one letter (s for stun, c for combo, m for move)

Now do you see the problem with asking a question like "how to shorten things?"

A more helpful way to think about the problem is:

- identify a pain point in your code ("it takes me too much time" or "this part is confusing for me")
- identify ways to eliminate that pain point without causing other issues

Could you shorten the above three countdown-and-trigger codelets?

Sure, maybe. Make a function that accepts a ref to the timer and a type.

Would that make anything better? Would that add any value to your project?

Probably not.

What if you needed to suddenly have something special happen on "stun" that doesn't happen on the others?

Oh dear, now you have a horrible horrible horrible mess.

DO NOT OPTIMIZE "JUST BECAUSE..." If you don't have a problem, DO NOT OPTIMIZE!

If you DO have a problem, there is only ONE way to find out. Always start by using the profiler:

Window -> Analysis -> Profiler

Failure to use the profiler first means you're just guessing, making a mess of your code for no good reason.

Not only that but performance on platform A will likely be completely different than platform B. Test on the platform(s) that you care about, and test to the extent that it is worth your effort, and no more.

https://forum.unity.com/threads/is-...ng-square-roots-in-2021.1111063/#post-7148770

Remember that optimized code is ALWAYS harder to work with and more brittle, making subsequent feature development difficult or impossible, or incurring massive technical debt on future development.

Notes on optimizing UnityEngine.UI setups:

https://forum.unity.com/threads/how...form-data-into-an-array.1134520/#post-7289413

At a minimum you want to clearly understand what performance issues you are having:

- running too slowly?
- loading too slowly?
- using too much runtime memory?
- final bundle too large?
- too much network traffic?
- something else?

If you are unable to engage the profiler, then your next solution is gross guessing changes, such as "reimport all textures as 32x32 tiny textures" or "replace some complex 3D objects with cubes/capsules" to try and figure out what is bogging you down.

Each experiment you do may give you intel about what is causing the performance issue that you identified. More importantly let you eliminate candidates for optimization. For instance if you swap out your biggest textures with 32x32 stamps and you STILL have a problem, you may be able to eliminate textures as an issue and move onto something else.

This sort of speculative optimization assumes you're properly using source control so it takes one click to revert to the way your project was before if there is no improvement, while carefully making notes about what you have tried and more importantly what results it has had.

 

- Could try naming things shorter (but still keep important meaning)
- Or just remove braces from 2nd if's
- All of them have true as 2nd argument, so its not needed then?
- Could use timers inside Coroutines (if suitable in your case) https://docs.unity3d.com/ScriptReference/Coroutine.html

Can use "?" operator, but then its not as readable:

Code (CSharp):

1.     void ProcessCooldowns()
2.     {
3.         comboTimer -= comboTimer > 0 ? Time.deltaTime : 0;
4.         if (comboTimer <= 0) ToggleCooldown(Cooldown.Combo);
5.  
6.         moveTimer -= moveTimer > 0 ? Time.deltaTime : 0;
7.         if (moveTimer <= 0) ToggleCooldown(Cooldown.Move);
8.        
9.         stunTimer -= stunTimer > 0 ? Time.deltaTime : 0;
10.         if (stunTimer <= 0) ToggleCooldown(Cooldown.Stun);
11.     }

 

I assume the actual goal is reducing duplicate code not necessarily shortening it.

You could create a separate cooldown timer time class which contains the common timer logic something like this.
Note the following code is only to ilustrate the idea. Don't copy and paste it, it will probably not compile! For simplicity I have skipped some technical details. And some of the details might be different based on rest of your logic.

Code (CSharp):

1. // if you  make this class serializable you might be able to tweak the cooldown in editor
2. class CoolDownTimer
3. {
4.      public float cooldown;
5.      float timeRemaining;
6.      public CoolDownTimer(float length) {
7.            cooldown = length;
8.            timeRemaining = 0;
9.      }
10.      public void Reset() {
11.           timeRemaining = cooldown;
12.      }
13.      public  bool Update() {  // Note this is a regular method called Update, it's not related MonoBehavior Update
14.          if (timeRemaining > 0) {
15.             timeRemaining -= Time.deltaTime;
16.          }
17.          return CanBeActivated; // change this if you want true only on the frame timer changes state
18.      }
19.      public bool CanBeActivated => timeRemaining <= 0;
20. }
21.  
22. CooldownTimer comboTimer = new CooldownTimer(5);
23. CooldownTimer moveTimer = new CooldownTimer(3);
24. CooldownTimer stunTimer = new CooldownTimer(10);
25.  
26. void ProcessCooldowns()
27. {
28.      // note you may need to tweak when update returns true,
29.      // how ToggleCooldown is implemented depending on what it does
30.      if (comboTimer.Update()) {
31.           ToggleCooldown(Cooldown.Combo, true);
32.      }
33.      // or maybe even something like this
34.      // no conditional logic here, the visuals are toggled based on whether timer has run out or not
35.      comboTimer.Update();
36.      ToggleCooldwon(CooldownCombo, comboTimer.CanBeActivated)      
37. }
38.  
39. // I assume that your ToggleCooldown contains something like this
40. void ToggleCooldown(CooldownType type, bool active) {
41.      var uiWidget = GetUI(type);
42.      uiWidget.SetVisibleOrSomething(active);
43.      // I assume there are no other sidefects only change in visuals and it's fine to set them to true or false, multiple times in a row
44.      // if you want do something specifically during true <-> false transition need to handle things more carefully
45. }
46.  
47. void ProcessInput()
48. {
49.      if (PressedKeyForAbility1() && ability1Timer.CanBeActivated) {
50.           ability1Timer.Reset();
51.           DoAbility1();
52.      }
53. }
54.  

Depending on the amount of such timers you have might take step further and store either ability type, delegate or UnityEvent in the timer which call the required function associated with the corresponding ability and timer. And put all the timers in list. But if you only have 2-5 of them i wouldn't go that far, and just call the methods on each timer instance directly.

Remember that any additional abstraction while reducing one type of code it comes with the cost of different kind of code and complexity. Using them when your project doesn't need them may cause more problems than they solve. So evaluate carefully whats the right tool for the specific job. Don't go blindly implementing every fancy technique you saw some slightly more experienced using, especially if you don't understand what problem the technique is supposed to solve.

Without knowing more details It's hard for me to tell how much of it is necesarry for your project. And even if I knew more, there are no hard borders and it's somewhat subjective based on each programmers personal taste and overall experience level of the team.

 

@karliss_coldwild gave the correct answer i was going to provide. Another way of doing this would be creating a function that recieves a "ref" parameter within your current class.

Code (CSharp):

1.     void ProcessCooldowns()
2.     {
3.         ProcessCooldown(ref comboTimer, Cooldown.Combo);
4.         ProcessCooldown(ref moveTimer, Cooldown.Move);
5.         ProcessCooldown(ref stunTimer, Cooldown.Stun);
6.     }
7.  
8.     void ProcessCooldown(ref float timer, float cooldownType){
9.         timer -= timer > 0 ? Time.deltaTime : 0;
10.         if (timer <= 0) ToggleCooldown(cooldownType);    
11.     }

i havent tested the code, but should work more or less.

 

Move the 3 scenarios into strategies. Look at the strategy pattern