Light of Dawn v1.5

[Moderator]

Light of Dawn v1.5 | Reviewed by Maker | 12th Oct 2013

APPROVED

Approved Use a variable for the ability DestroyForce is in a wrong place

[tr]

Old reviews 11:59, 3rd Jun 2011 Maker: Causes leaks in the cast trigger, the call RemoveLocation(udg_Set SS_Point2) is in wrong place: Set SS_Point2 = (Position of SS_RandomUnit) Lightning clear shouldn't be on all the time.

 

[Manasurge]

My second spell, nothing amazing but still pretty usefull i'd say.
Update: Fixed leak and tooltip (slightly)
Update: Fixed Spell to hit air properly and use lightnings in a much better way.
Update: More efficient loop
Update: Minor value change and lightning effect

RATE AND COMMENT!

 

[Nillere]

Well, by taking a quick look in the triggers, i couldn't see anything wrong with them, though you've got a unit group leaking, if i didn't miss something

But maybe you should change the "heals X dmg" to "heals X hp/hit points/health", or whatever pleases you

 

[88WaRCraFT3]

1 more nice spell, nothing to say +REP idk i won't rate it for now. Nice fun system it's fun

 

[Marcos DAB]

Nice work, codes seems fine.
I also like because you use floating text to show the damage in all of your spells.
, like me.
=)

 

[Adiktuz]

care to use AddLightningEx so that you can take into account the height of the unit?

 

[Magtheridon96]

• Set SS_Caster = (Triggering unit)
• Set SS_Point1 = (Position of (Triggering unit))

I loled x)
You should use SS_Caster instead of Triggering Unit in the second line.

Overall, you did a pretty good job

Just use AddLightningEx to give the lightnings height so that the effect
would be nicer ^^

5/5

EDIT: One more thing,
For efficiency, in the following block, store Triggering Unit in a variable and use it
instead of calling "GetTriggerUnit()" every time (Jass thing ;p)

• Unit Group - Remove (Triggering unit) from SS_RemoveLightningGroup
• Lightning - Destroy (Load 0 of (Key (Triggering unit)) in SS_Hashtable)
• Hashtable - Clear all child hashtables of child (Key (Triggering unit)) in SS_Hashtable

 

[Adiktuz]

he's currently working on it... ^_^

 

[Magtheridon96]

One more thing, you're test map was really entertaining ^^
I always imagined mods to be mutant creatures ... well .. 2/3 of them at least x) -jk jk ;p

By the way, you should store (Last Created Floating Text) into a variable to null later.

 

[Narogog]

nice!

cool spell

but you might change the for each integer to a pick every unit, that way you don't have to remove the unit from the unit group, and you are doing a pick every unit. save the handle of triggering unit as an integer so

• set handint = key(triggering unit)

and use that instead of key(triggering unit)
it has to do with what mag said, efficiency.

you do a good job making spell keep up the good work

 

[Manasurge]

Thanks for all feedback, working on the lightning and when im done with that ill fix efficiency.

 

[Bribe]

Set SS_LightningGroup = (Random Actual_NrHitUnits units from (Units within Actual_Range of SS_LastHitPos matching

- This actually creates two unit groups, not just one. You need to store "(Units within Actual_Range of SS_LastHitPos matching..." into a unit group and destroy it seperately.

Actually, if I understand correctly, you want to pick a minimum of Actual_NrHitUnits compared to the actual count of SS_NrUnits, there is an easier way. You just count the units in "(Units within Actual_Range of SS_LastHitPos matching..." and loop through whichever count is smaller. There is also "Custom script: set udg_SS_RandomUnit = FirstOfGroup(udg_SS_LightningGroup)".

I think you may be better off doing a "pick all units in unit group" and keeping a counter that just ignores all actions after the limit is reached.

 

[Manasurge]

@Bribe
Ok, fixed some, rest was no longer needed with the remake of the loop.
@Adiktuz:
OPS! :/ guess i was in too much of a hurry

 

[Adiktuz]

You used picked every unit, but still uses the set unit = random unit from group???

you just made it more inefficient...

what you need to do is:

1) remove the SS_NrUnits --> this would be useless
2) remove all lines using the SS_NrUnits variable
3) instead of SS_RandomUnit = (Random unit from SS_LightningGroup), make it: SS_RandomUnit = PickedUnit

 

[Maker]

Comments

Uploaded abilities don't need imported icons.
The learn tooltip coild mention the hotkey, and you could add level stats like standard Blizz abilities have. Also use colour codes.
Lightning Clear shouldn't be on all the time, only when dummies exist.
You could set these in reversed order and use the unit variable in the level thingy:

• Set SS_Level = (Level of (Ability being cast) for (Triggering unit))
• Set SS_Caster = (Triggering unit)

The lightning height could be set to a variable that one can configure. The default 35 is too low IMO.
You could use

• Custom script: set bj_wantDestroyGroup = true
• Unit Group - Pick every unit in (Units withi Actual range...)

Then you don't need a variable and don't need to use call DestroyGroup() line.
Instead of creating the lightning and then moving it, just create it like this and you don't have to move it:

• Custom script: set bj_lastCreatedLightning = AddLightningEx(udg_SS_LightningType,true,udg_x1,udg_y1,udg_z1,udg_x2,udg_y2,udg_z2)

Then you can use (last created lightning effect) GUI function to deal with the lightning and you don't need a separate variable for that.
Leaks the player group:

• Floating Text - Show SS_TextTag for (Player group((Triggering player)))

 

[Manasurge]

Comments

Uploaded abilities don't need imported icons.
The learn tooltip coild mention the hotkey, and you could add level stats like standard Blizz abilities have. Also use colour codes.
Lightning Clear shouldn't be on all the time, only when dummies exist.
You could set these in reversed order and use the unit variable in the level thingy:

• Set SS_Level = (Level of (Ability being cast) for (Triggering unit))
• Set SS_Caster = (Triggering unit)

The lightning height could be set to a variable that one can configure. The default 35 is too low IMO.
You could use

• Custom script: set bj_wantDestroyGroup = true
• Unit Group - Pick every unit in (Units withi Actual range...)

Then you don't need a variable and don't need to use call DestroyGroup() line.
Instead of creating the lightning and then moving it, just create it like this and you don't have to move it:

• Custom script: set bj_lastCreatedLightning = AddLightningEx(udg_SS_LightningType,true,udg_x1,udg_y1,udg_z1,udg_x2,udg_y2,udg_z2)

Then you can use (last created lightning effect) GUI function to deal with the lightning and you don't need a separate variable for that.
Leaks the player group:

• Floating Text - Show SS_TextTag for (Player group((Triggering player)))

Will fix.
set bj_wantDestroyGroup = true is often a good way of doing it, but i want to keep it a group so i can add SS_LastHit to the group outside the loop itself. and i can't add SS_LastHit to a group that doesn't exist.
35 height? now a variable, change it if you don't like 35 after you download it

EDIT: Noticed i hadn't implemented that change so your set bj_wantDestroyGroup = true would work.
But not now x) added the function.

 

[Shyster]

Downloaded it, the lightning effects don't remove themselves. Why?