Pair Annihilation

[Daffa]

Thank you for submitting such a wonderful spell. Below is a list of feedback from me:

• Spell is not MUI. It gets bugged when multiple casters or when the caster use it multiple times before the previous instance finishes. Resource Submission Rules (Models, skins, icons, spells) --> I recommend one of the following tutorials:
  • Visualize: Dynamic Indexing
  • A Complete Beginners Guide to Hashtables
  • Hashtables and MUI
• Do not use waits.
• Do not use 0.01 seconds in loop since that means 100 runs in one second. The standard is either 0.03 (33.33 per second) or 1/32 (32 per second) for duration in loop.
• Do not instantiate arrays with large sizes such as 999. This consumes a lot of memory. Warcraft is capable of dynamically creating the necessary size depending on the need as long as it is properly set with set variable.
• Memory Leaks

The following leaks (and much more):

• Lightning - Create a Finger of Death lightning effect from source (Position of Electron) to target ((Position of Positron) offset by (((Distance between (Position of Electron) and (Position of Positron)) / 2.00) x -1.00) towards (Angle from (Position of Electron) to (Position of Positron)) degrees)

• Lightning - Create a Chain Lightning - Primary lightning effect from source (Position of Positron) to target ((Position of Electron) offset by (((Distance between (Position of Electron) and (Position of Positron)) / 2.00) x 1.00) towards (Angle from (Position of Electron) to (Position of Positron)) degrees)

• Special Effect - Create a special effect at (Position of Electron) using Pa_Electron_SpecialEffect

• Special Effect - Create a special effect at (Position of Positron) using Pa_Positron_SpecialEffect

• Special Effect - Create a special effect at ((Position of Positron) offset by (((Distance between (Position of Electron) and (Position of Positron)) / 2.00) x -1.00) towards (Angle from (Position of Electron) to (Position of Positron)) degrees) using Pa_Lightning_SpecialEffect

 

[Arthourias]

Thank you for your feedbacks.
Spell resets when another caster use it, so it's not a bug.as you know these are hero abilities and are unique. It designed to work only for one hero not all the map!
About the waits, again there is no need to change, it's okay
You are right about array sizes I will change them
All the leaks you have mentioned are wrong because they will all destroy when spell finishes.(check for each integer pa_sp_loop_integer line in Pair Annihilation 2 and lightning destroy functions)

 

[Pyrogasm]

Spell resets when another caster use it, so it's not a bug.as you know these are hero abilities and are unique. It designed to work only for one hero not all the map!

Such a submission will never be approved as complete/acceptable on this website; at best it will get marked as substandard and moved to that subforum. This is also not in any way how abilities work in wc3 so there is no precedent to say this should not be MUI just because it's a hero ability. Furthermore, if this is your specific intent nowhere in your description or documentation do you say this is the case so how would an unaware user know this? Imagine a user who doesn't want this limited to one cast by one unit at a time but then doesn't understand why it doesn't work. The reason there are submission rules/guidelines is to provide standardized resources that generally all behave in the same way.

About the waits, again there is no need to change, it's okay

First of all a 0.01 wait will never wait only 0.01s; the minimum wait time is somewhere around ~0.27s. Second, waits proceed when the game is paused or lagging so can be unreliable (this point doesn't matter for your implementation here, however). Third, many event responses are lost after a wait (or work internally as global variables so are liable to be overwritten), which can complicate triggers or cause unexpected behavior. None of those concerns are really relevent here, but there is no real reason for that wait to exist in the trigger and it is (IMO) wholly unnecessary.

I also agree with Daffa that a 0.01s timer is extremely wasteful and can be resource intensive for no real gain. If a player is running at 60 fps, there won't even be any visual change roughly every other timer tick!

All the leaks you have mentioned are wrong because they will all destroy when spell finishes.

While you are somewhat correct, the way you are cleaning these leaks is very nonstandard/unorthodox and is an artifact of the non-MUI behavior you have baked into this spell. It is always best to remove/destroy the objects in the same trigger or function block or scope, etc. as they are created. This ensures that nothing will interrupt this process and cause them to become permanent memory leaks. It can be acceptable leave them for another trigger to clean up, but frankly the way you've done some of it here just comes across as lazy. I understand why the special effects are done the way they are.

Furthermore, there are a massive number of point leaks and group leaks that Daffa did not point out that you seem to be unaware of:

• -------- lines like these --------
• Unit Group - Pick every unit in (Units within Positron_Electron_Damage_Aoe of (Position of Electron).) and do (Actions)
• -------- --------
• Unit - Create 1 . Electron and Positron Annihilation for (Owner of Pa_Caster) at (Pa_Target_Point offset by (Pa_Ability_Range x -2.00) towards Pa_Angle degrees.) facing Default building facing degrees

All of the groups and points created in these (and other similar) lines are not being cleaned up and are becoming permenant memory allocation. This includes the points that you are storing into variables but are not cleaning up properly. Considering the frequency of the periodic trigger, the amount of memory over time will be significant.

• Conditions
  • Pa_Special_d Equal to Pa_Special_Rate

Comparing real numbers for equality is never a good idea because of floating point precision. It's possible that one of the numbers gets a tiny 0.00000001 added to one of them in the incrementing process which would make this condition always fail. Instead you should check "greater than or equal to". In general, using "equal to" with integer or real number comparisons is inferior to using "greater than or equal to" comparisons on those numbers to account for any erroneous incrementation of those numbers or FPP errors.

 

[Arthourias]

You are almost wrong about the waits, minimum wait time is 0.09 seconds but it's not the case.I just need a minimum delay time for my spell to work, it's amount is not important.
About leaks you are correct, it's not interesting how I destroy lightnings and special effects but I tested it so many times and no leak was observed.
And also you are right about the remaining.as you said I have to clean unit groups and edit those real equality.
Thank you for your guidance

 

[Daffa]

All the leaks you have mentioned are wrong because they will all destroy when spell finishes.

Position of Electron, Position of Positron, Position with Polar Offset

All these points are leaks since they are not preserved in point variables with proper call RemoveLocation(PointVariable) called . Given how the trigger runs 100x per second, it can accumulate very quickly.

When I pointed those leaks, I am pointing the point leak inside those functions, not the special effect or lightning since those get destroyed at end of cast. My apologies for the confusion.

 

[Arthourias]

Position of Electron, Position of Positron, Position with Polar Offset

All these points are leaks since they are not preserved in point variables with proper call RemoveLocation(PointVariable) called . Given how the trigger runs 100x per second, it can accumulate very quickly.

When I pointed those leaks, I am pointing the point leak inside those functions, not the special effect or lightning since those get destroyed at end of cast.

Yes you are completely right about these leaks. I try to fix them thanks

 

[Pyrogasm]

it's not interesting how I destroy lightnings and special effects but I tested it so many times and no leak was observed

I specifically said those were fine though unorthodox.

You are almost wrong about the waits, minimum wait time is 0.09 seconds but it's not the case.

It's only that short in single player maps. When the map is hosted for multiplayer the minimum is again ~0.22.

About the waits, again there is no need to change, it's okay

As the spell is presently coded it's not okay. As you can see in this thread, one of the event responses you are using (target point of ability being cast) is in the "works but not MUI" category. That means that any other spell cast by any unit in the wait time (which is longer in multiplayer so more likely to occur) will overwrite that event response and cause the wrong point to be used (or the point will just return null, according to one post in that thread which I have quoted below). Make the wait longer and order something to cast during the wait if you want to see that in action. You can get around this potential (but yes, relatively unlikely) error by storing the target point information before the wait, or by not using the wait.

target point of ability being cast

This in in the list, I said this behaves weird because it acts differently with finishcasting event and other spellcast events.Also ,if I remember correctly, when I tested with starts effect of an ability event it was returning null after wait if casting unit finishes spell.But it is returning last used value if casting unit still continues spell (channeling spells)

 

[Wrda]

You should have a configuration trigger to set all the variables that only need to be set once.

• Actions
  • Set VariableSet Pa_t[1] = ((Angle from (Position of Pa_Caster) to (Target point of ability being cast)) + 190.00)
  • Set VariableSet Pa_t[2] = ((Angle from (Position of Pa_Caster) to (Target point of ability being cast)) + 170.00)
  • Set VariableSet Pa_p[1] = ((Target point of ability being cast) offset by Pa_Ability_Range towards Pa_t[1] degrees.)
  • Set VariableSet Pa_p[2] = ((Target point of ability being cast) offset by Pa_Ability_Range towards Pa_t[2] degrees.)

Pa_Angle for Pa_t and Pa_Point for Pa_p would be more descriptive names
This spell, along with many others, must be MUI, like Daffa and Pyrogasm told you. Even if the users downloading this resource are notified that it only work for one unit, chances are that it's not very useful to them. In addition, it's in the rules, I can't make exceptions.
Here is this link to help you understand it, using dynamic indexing: Visualize: Dynamic Indexing
If you need a delayed effect (delayed action) you can check this out: MUI Spells Using Artificial Waits. Waits are almost always bad.

• Set VariableSet Pa_Angle = (Angle from (Position of (Casting unit)) to (Target point of ability being cast))

Position of (Casting Unit) leaks, along with (Target point of ability being cast)

In Pair Annihilation 2:
You don't need to create and destroy lightning to simulate movement, there's an action already for that.
On the section with the condition "Pa_Special_d Greater than or equal to Pa_Special_Rate" you are creating unit groups with
E_Unit_Group, P_Unit_Group, L_Unit_Group and then picking them and damage, but not cleaning right after, so again it's still memory leaks. This could all be done with just one group variable by the way.

• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
  • If - Conditions
    • ((Owner of (Picked unit)) is an enemy of (Owner of Pa_Caster).) Equal to True
  • Then - Actions
    • Unit - Cause Pa_Caster to damage (Picked unit), dealing (Pa_Damage / 3.00) damage of attack type Spells and damage type Normal
    • Unit Group - Remove all units from P_Unit_Group.
  • Else - Actions

I don't even know what happens when someone attempts to remove all units of a group inside of a "pick every unit" action, but this isn't good at all. Do it outside of the action.

The spell looks great, but not being MUI is a big nono.

Needs fix.