Cluster Bomb v1.2b

[Moderator]

10th Aug 2011
Bribe:

Memory leaks
Not MUI
Too simple

Please ask for help in Triggers & Scripts on how to make your spell MUI and leakless.

 

[Magtheridon96]

1- Don't use waits
2- You're leaking (Position of Triggering Unit)
3- It's too simple
4- You're repeating stuff like (Triggering Unit) and (Owner of Triggering Unit).
You should store them in variables for efficiency..
5- Owner of Triggering Unit is the same as (Triggering Player)
6- Use 'Expiration Timer' (Timed Life) for removing the units instead of waiting a second and then removing them
7- You're leaking a group (Use

• Custom script: call DestroyGroup(udg_NAMEOFGROUP)

to destroy it at the end.
8- You're leaking 'point' (Use Custom script to remove it with the function RemoveLocation):

• Custom script: call RemoveLocation(udg_point)

9- You shouldn't be using groups here...

For the waits, you could replace them with periodic triggers.

 

[taxikar]

on it sir!
edit:
so whats better now? triggering player or owner of cluster_bomb_caster ?

 

[maddeem]

Uhh your wait in there could possibly cause a leak. How you may ask? Well if a person casts it, it sets the point n all and waits. If a person casts it right after the other person it would set your point to another location before it could be removed, causing a leak.

Your unit groups leak. In my opinion you should only use one dummy for casting, and use the special effects action for special effects.
Oh and your wait leaks. You used a location that you didn't even attempt to destroy.
All in all I'll rate this 1/5. If you make it without such things I have pointed out. I'll give you 3/5 and +Rep.
Also you seem to be using triggering player a lot. You should set it to a variable and use it in place of "owner of cluster_bomb_caster" or "Triggering player"
But if you're too lazy to do that, triggering player is always better that owner of *unit*.

 

[Magtheridon96]

Triggering Player is better (Save it in a variable then use it)

 

[taxikar]

Im on it people! just give me some time! also making the spell more complex!
anyway thanks

 

[mckill2009]

use hashtables or indexing system for the wait...

this should be done in a loop...use ForIntegerA or B is fine for this...
you should also create a group when casting then destroy it...

• Unit - Create 1 secondary...
• Unit Group - Add LastCreatedUNit...

this should be put also in a multiple action unit group...

• Unit Group - Pick every unit
• Unit Group - Pick every unit
• Unit Group - Pick every unit
• Unit Group - Remove picked unit...

 

[maddeem]

this should be done in a loop...use ForIntegerA or B is fine for this...

You shouldn't use ForIntegerA or B, I've heard it causes errors.

 

[mckill2009]

You shouldn't use ForIntegerA or B, I've heard it causes errors.

not when it's instant cast...

 

[maddeem]

not when it's instant cast...

Lol.