Consume Mana V1.2

[Moderator]

20:31, 1st Sep 2015
IcemanBo: https://www.hiveworkshop.com/threads/consume-mana-v1-2-a.269240/post-2725315

 

[Chaosy]

Checked the triggers real quick. A really good submission if it really is your first one.

There might be flaws, but they are likely minor. MUI, Configurable, Not simple, Leakless (did not check this properly, there are leak removal actions at least.)

 

[KILLCIDE]

Pretty impressive for your first spell however, there are still a lot things you're going to need to work on.

• Why are you doing variable index = level - 1 to find out the level of an ability? You can just save time and do variable index = level.
• All your unit groups leak.
• I wouldn't recommend using (Integer A). Create your own variable for it.
• What's the purpose of using Skip remaining actions?
• You're going to want to recycle any variable that's in the Cast trigger that is being used in the loop.
• You're not turning the loop off. Use a condition that checks if index = 0, if so turn loop off.
  
  
• What is the purpose of this? You can just make the unit group only pick enemy units.
  
  • Unit Group - Add all units of (Units within W_Range[(W_Level[W_Loop] - 1)] of W_Point[W_Loop] matching (((Matching unit) is A ground unit) Equal to True)) to W_Group[W_Loop]
  • Player Group - Pick every player in (All allies of (Owner of W_Caster[W_Loop])) and do (Actions)
    • Loop - Actions
      • Unit Group - Remove all units of (Units owned by (Picked player)) from W_Group[W_Loop]
• You don't need to put And - All (Conditions are true)
  
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    • If - Conditions
      • And - All (Conditions) are true
        • Conditions
          • (Mana of (Picked unit)) Greater than 0.00
          • ((Picked unit) is alive) Equal to True

 

[MatiS]

[*]Why are you doing variable index = level - 1 to find out the level of an ability? You can just save time and do variable index = level.

I used to think that array size matters, and after I found out it doesn't, I just didn't have enough patience to change this fragment of code :-D.

[*]All your unit groups leak.

When I used custom script to remove them, the instance stopped workin' at all. I thought that when I remove all units from group, I'll be able to recycle this group next time the instance runs, so I will only have as many groups as maximum number of instances at one time. I'll look into it again.

[*]I wouldn't recommend using (Integer A). Create your own variable for it.

I'll do it, although I don't think I really need new variable in this place, as I don't use it in loop .

[*]What's the purpose of using Skip remaining actions?

I thought it works like break function from C++, so it stops current istance of the loop. I'll delete it if it doesn't.

[*]You're going to want to recycle any variable that's in the Cast trigger that is being used in the loop.

What do you mean?

[*]You're not turning the loop off. Use a condition that checks if index = 0, if so turn loop off.

I forgot :-D.

[*]What is the purpose of this? You can just make the unit group only pick enemy units.

• Unit Group - Add all units of (Units within W_Range[(W_Level[W_Loop] - 1)] of W_Point[W_Loop] matching (((Matching unit) is A ground unit) Equal to True)) to W_Group[W_Loop]
• Player Group - Pick every player in (All allies of (Owner of W_Caster[W_Loop])) and do (Actions)
  • Loop - Actions
    • Unit Group - Remove all units of (Units owned by (Picked player)) from W_Group[W_Loop]

So I can put 'AND' condition in adding units to group. I didn't know, thanks.

[*]You don't need to put And - All (Conditions are true)

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • And - All (Conditions) are true
      • Conditions
        • (Mana of (Picked unit)) Greater than 0.00
        • ((Picked unit) is alive) Equal to True

Ok, thank you. I'm gonna work on this things :-D.

 

[KILLCIDE]

When I used custom script to remove them, the instance stopped workin' at all. I thought that when I remove all units from group, I'll be able to recycle this group next time the instance runs, so I will only have as many groups as maximum number of instances at one time. I'll look into it again.

You're probably destroying it to early. Place them where you're removing all the units from the group.

I'll do it, although I don't think I really need new variable in this place, as I don't use it in loop .

Using Integer A is slower because its another function call. Someone else could also decide to use your spell in their map, and what's going to happen if they are using Integer A in another trigger? Baaad things.

I thought it works like break function from C++, so it stops current istance of the loop. I'll delete it if it doesn't.

You use break in a ranged-based loop to reach a specific array in C++, which is not at all what you're doing here in a spell xP You would want to stop the current instance of the loop when the spell instance is done. That's where recycling + counter timers come into play.

What do you mean?

You basically re-adjust all current instances of the spell. It's what helps a spell become MUI. Think of it like this:

You have 3 instances of your spell that happen almost simultaneously. Data would be anything like Caster, CasterLoc, Target, TargetLoc, etc.

• Instance 1 (data x)
• Instance 2 (data y)
• Instance 3 (data z)

Instance 1 will finish first because it was casted first. Once Instance 1 is done casting, you need to re-adjust Instance 2 to be Instance 1, and Instance 3 to be Instance 2. We want a result like this:

• Instance 1 (data y)
• Instance 2 (data z)

This is currently what your trigger is doing.

• Instance 1 (data x + y)
• Instance 2 (data y + z)

So I can put 'AND' condition in adding units to group. I didn't know, thanks.

Yes I would recommend a method like this. It looks cleaner and its a lot easier to read.

 

[MatiS]

I fixed loop, deleted 'skip remaining actions', added custom integer, but after I try to remove group with:

• Custom script: call DestroyGroup (udg_W_Group[udg_W_Loop])

I can't recycle this instance again. Group is permanently deleted, and I cannot use instance 1.

 

[KILLCIDE]

I fixed loop, deleted 'skip remaining actions', added custom integer, but after I try to remove group with:

• Custom script: call DestroyGroup (udg_W_Group[udg_W_Loop])

I can't recycle this instance again. Group is permanently deleted, and I cannot use instance 1.

You aren't recycling that xP its not used in the Init trigger. Can you also update your triggers in the description?

 

[BPower]

You have location leaks in your code.

 

[IcemanBo]

Leaks must been fixed. Create a thread in Triggers & Scripts forum if you have problems with your code. https://www.hiveworkshop.com/threads/memory-leaks.263410/post-2661133

Where do you deindex? https://www.hiveworkshop.com/forums...orials-279/visualize-dynamic-indexing-241896/

Make as much configurable as possible. For example attack- damage type should be.

Responses like (Picked Unit) should be stored into a variable when used more often.

'And All Conditions are True' only can be useful with combination with 'Or Any Condtions" block. In your case it's not needed.

"W" is not enough for a prefix, it should have at least 2-3 letters.

The (Level -1) could be done once onCast.