Enchanted Owl 1.06

[Moderator]

12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.

Moderator: Pharaoh_ Date: 04:18, 24th Apr 2012 You need to merge some of the actions in the loop trigger. Check if the dummy is in Owl_Group once and in the then branch check if the Distance is Less than or Equal to 1. If it is, perform the actions, else do the other actions. I don't see any reason why you convert the distance variable to integer to make the check. Certain variables are global, when they should be enhanced with an array (using the current index). For readability, use unit group variables, when you pick the "random unit from units in random units matching...).

Older Reviews

Bribe:

Instead of (Owner of (Triggering unit)) you should use (Triggering player). A lot more simple and executes faster in-game. It may surprise you that this works but it works.

To fix the leak you need to do this:

• Set TempPoint = (Position of OWL_Dummy[OWL_LoopInt])
• Set OWL_MovePoint = (TempPoint offset by OWL_Speed towards (Facing of OWL_Dummy[OWL_LoopInt]) degrees)
• Custom script: call RemoveLocation(udg_TempPoint)

You don't need to convert this thing to an integer:

(Integer(OWL_Distance[OWL_LoopInt])) Less than or equal to 1

You can simply compare it to 1.00. This will save a little complexity.

I also recommend setting "OWL_Dummy[OWL_LoopInt2]" to a normal unit variable before repeat-referencing because it will improve readability and is a bit faster in-game.

Maker:
Leaks MovePoint in the looping trigger, and the looping trigger should be turned off when there are no active instances of the spell.

 

[Genji Lone]

Nice spell

 

[Adiktuz]

I believe that the region leaks... it would be better to just have a distance variable and reduce it every loop... also, using move order is not that good, its better to move the owl via triggers...

also, you already set last created unit to a variable but still uses the last created unit on the next set of actions...

 

[coolyoshi1]

is there any way to remove region leaks with triggers?
im a bit confused with wat u mean with the last created unit stuff
thanks!

 

[Adiktuz]

Animation - Play (Last created unit)'s birth animation
Set OWL_Dummy[OWL_CastCountInt] = (Last created unit)
Unit - Order (Last created unit) to Move To OWL_TargetPoint[OWL_CastCountInt]
Unit Group - Add (Last created unit) to OWL_UnitGroup
Unit Group - Add (Last created unit) to OWL_UnitGroup3

-->You set (Last created unit) to OWL_Dummy[OWL_CastCountInt], but you still use (Last created unit) on the next steps... it reduces performance, as (Last created unit), is a function call which is slower than a variable...

it should be like this:

Set OWL_Dummy[OWL_CastCountInt] = (Last created unit)
Animation - Play OWL_Dummy[OWL_CastCountInt]'s birth animation
Unit - Order OWL_Dummy[OWL_CastCountInt] to Move To OWL_TargetPoint[OWL_CastCountInt]
Unit Group - Add OWL_Dummy[OWL_CastCountInt] to OWL_UnitGroup
Unit Group - Add OWL_Dummy[OWL_CastCountInt] to OWL_UnitGroup3

You can remove rects(the GUI regions) with script, but I really dont suggest the way that you're doing this, (issuing move order)... its better if you just move the owl via the loop trigger, and save the distance from the initial point to the target point into a variable, and reduce it every second, if it becomes equal or less than 0, do the removal actions...

also the current set-up is pretty bad, because you can just combine those two timed loops... anyway, if you do the above suggestion, you will also end up with just one timed loop, so don't bother merging the current triggers...

 

[coolyoshi1]

ic the problem thanks.
ill try changing the move order but i wanna know why its not recommended.

 

[Adiktuz]

it could be weird sometimes... and not that flexible as compared to moving it via triggers...

you also leak unit groups on the loop...

sample:

this line:

Random unit from (Units within OWL_AOE of (Position of OWL_Dummy[OWL_LoopInt2]) matc

Units within blablah, leaks a group...

 

[Narogog]

using two loops for one spell.......
well other then that

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
  • OWL_ManaBurnCastCount[OWL_LoopInt2] Equal to OWL_ManaBurnCastMax[OWL_LoopInt2]
  • Then - Actions
  • Unit Group - Remove OWL_Dummy[OWL_LoopInt2] from OWL_UnitGroup3
  • Unit - Add a 1.30 second Generic expiration timer to OWL_Dummy[OWL_LoopInt2]
  • Custom script: call RemoveLocation(udg_OWL_Point2)

why is the remove location inside of a if then else that it is not used in? i am pretty sure this will cause leaks.

 

[Manasurge]

Love the idea, fix what Adiktuz pointed out and this will be a nice addition to hive.
3/5 for now

 

[Nightmare Moon]

I think the idea is pretty good, but i would like to see in this spell is second owl! I mean create two owls; one burns mana second hitpoints. Would have more effects - cooler. And ofc do what Adiktuz said, till the leaks are cleared they will hunt you! Good luck! Oh and make the owl not move in line, it's kinda ugly, but a bit to the right then to the left.
To remove the group leaks before creating a group add this line: Custom script: set bj_wantDestroyGroup = true

 

[coolyoshi1]

i think its fixed now

 

[Marcos DAB]

SHould be improved, per now seems fine.

 

[Narogog]

you got rid of the 2 loops, it looks great now! good job.

 

[Magtheridon96]

Unacceptable
Bribe:

Instead of (Owner of OWL_Caster[OWL_CastCountInt]) you should use (Triggering player). A lot more simple.

This leaks (Position of OWL_Dummy[OWL_LoopInt]):

Set OWL_MovePoint = ((Position of OWL_Dummy[OWL_LoopInt]) offset by OWL_Speed towards (Facing of OWL_Dummy[OWL_LoopInt]) degrees)

This leaks a unit group and could be fixed by placing "bj_wantDestroyGroup = true" directly above it, and also leaks a location:

Unit - Order OWL_Dummy[OWL_LoopInt2] to Night Elf Demon Hunter - Mana Burn (Random unit from (Units within OWL_AOE o

And you do the same thing further down your script which needs to be leak-cleared as well.

Actually you do the same thing a lot... looks like you have a LOT of duplicate text in your loop trigger...

Needs a ton of work.

EPIC XD

Leaks are very important to clear though :/
If you ignore them, your maps will lag like hell

 

[coolyoshi1]

Instead of (Owner of OWL_Caster[OWL_CastCountInt]) you should use (Triggering player). A lot more simple.

This leaks (Position of OWL_Dummy[OWL_LoopInt]):

Set OWL_MovePoint = ((Position of OWL_Dummy[OWL_LoopInt]) offset by OWL_Speed towards (Facing of OWL_Dummy[OWL_LoopInt]) degrees)

This leaks a unit group and could be fixed by placing "bj_wantDestroyGroup = true" directly above it, and also leaks a location:

Unit - Order OWL_Dummy[OWL_LoopInt2] to Night Elf Demon Hunter - Mana Burn (Random unit from (Units within OWL_AOE o

And you do the same thing further down your script which needs to be leak-cleared as well.

Actually you do the same thing a lot... looks like you have a LOT of duplicate text in your loop trigger...

Needs a ton of work.

I just changed the stuff you said in red.
-why is it better to use owner of triggering unit over the variable?
-I already had destroy group triggers so i guess you didnt see them..
-(Position of OWL_Dummy[OWL_LoopInt]) dosnt leak cuz i set it a variable and destroyed it

i hope its ok i reply here. I tried to message back to mod but it didnt work.

 

[Bribe]

i hope its ok i reply here. I tried to message back to mod but it didnt work.

I got all three of your messages.

 

[coolyoshi1]

lol srry

 

[mckill2009]

- ALL configurables must be put in an initialization trigger so that it will not run every time the spell is casted...
- Its better to use an index, instead of checking if unit group is empty...

• (Number of units in OWL_UnitGroup) Not equal to 0

--->

• IntegerIndex Greater than 0

- The Booleans in the loop is not indexed in the casting
- Use a loop group, not a single line
- 0.03 is too fast for playing animations, better to make an indexed real value=0, if it reaches 2, then play the animation and reset it to 0 in the loop
- It's highly recommended to use Hanky's Indexing system or Bribes UnitIndexer

 

[Tank-Commander]

Reminds me of my triggered summoned units, although you /could/ deal all the damage (manaburning) Without the use of object data, all you need to do is use basic lightning control and do triggered mana damage, and this spell would be instantly 100x better