Snake Curve & DNA Wave

[Moderator]

12th Dec 2015
IcemanBo: For long time as NeedsFix. Rejected.

Bribe:

These waits need to be removed as

1) their wait period is random and never less than 0.26
2) it could occur with different intervals in replays
3) on battle.net a wait could under some conditions desync

Using a Timer to do this job will yield far better results.

 

[ZEEP]

Forgot to mention in the screenshot the unit life duration is increased to reveal the full trajectory of the spell.

 

[Archangel678]

Good spell. Its fully MUI and performs well.

Suggestions

Don't use immolation for damage, as the caster doesn't get credit for kills, and doesn't 'pull' units.

You use alot of dummy units, perhaps you could use special effects?
Its a mere suggestion.

Perhaps a damage splat/special effect? It was hard to see what units were taking damage.

 

[Adiktuz]

SUGGESTIONS

instead of loops using TriggerSleepAction, you might want to use timer loops... since TSA is pretty inaccurate... and it really does not go below .27 seconds...

about what you said about it being hard to be MUI with timed loop

its pretty easy to make it MUI with timed loops, you just need to learn how to use hashtables or recycling dynamic indexing...

and oh, most spells that deal through time uses timed loops...

also you can put the actions into the conditions for faster execution... and also your condition block ryt now was pretty long and is clearly converted directly from GUI... (oh, the whole trigger was converted GUI, and then added some locals and changed things to natives)

also save the Owning player into a variable as you call it over and over...

and instead of bj_Unitfacing just use a real value...

and instead of Facing angle of the caster (which actually returns limited values), you might want to use the angle between the target point and the caster position...

and your spell's configurability is bad... it won't even support custom damage formulas (like those using hero attributes to calc damage)

also I suggest you learn how to use a single dummy unit, using dummy.mdx, as most resources use that, and just attach the effect model to it via triggers, to lessen object data, and it also makes it easier for the users...

For now, it really needs a lot of fixes for optimization and configurability... (if the Vote buttons was still there, I would have voted for rejection)

 

[Magtheridon96]

A few suggestions:

function InitTrig_SnakeWaveSingle takes nothing returns nothing

    set gg_trg_SnakeWaveSingle = CreateTrigger(  )

    call TriggerRegisterAnyUnitEventBJ( gg_trg_SnakeWaveSingle, EVENT_PLAYER_UNIT_SPELL_EFFECT )

    call TriggerAddCondition( gg_trg_SnakeWaveSingle, Condition( function Trig_SnakeWaveSingle_Conditions ) )

    call TriggerAddAction( gg_trg_SnakeWaveSingle, function Trig_SnakeWaveSingle_Actions )

endfunction

Change this to:

function InitTrig_SnakeWaveSingle takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition(t, Condition( function Trig_SnakeWaveSingle_Conditions ) )
    call TriggerAddAction(t, function Trig_SnakeWaveSingle_Actions )
endfunction

I just think it would look better that way

This exitwhen (k==40) could be changed to exitwhen k >= 40
It's safer that way

One more thing:

function Trig_SnakeWaveSingle_Conditions takes nothing returns boolean

    if ( GetSpellAbilityId() == 'A001' ) then

        return true

    endif

    return false

endfunction

Can be easily changed to:

function Trig_SnakeWaveSingle_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A001' then
endfunction

I hope this helped
Good luck ^^

 

[baassee]

He can't put the actions in the conditions as long as he uses TSA as you cannot do that in the conditions (I think, pretty sure about it too).

All you need is a hashtable variable and maybe a timer recycler... My pounce spell is built that way if you need any inspiration.

im not a fan of getspellabilityunit, gettriggerunit is fine

getowningplayer could be a local player p = GetTriggerPlayer(), should be

wtf wait 0.0001? 1/10000? cool if you could prove that that value works

 

[Adiktuz]

that's why I said use Timer loops...

 

[baassee]

that's why I said use Timer loops...

yeah I just explained further

 

[Marcos DAB]

Need minor improvements =/
Maybe useful.

 

[ZEEP]

Ok for damage splat/effect to determine which unit is hit , it will lag very badly if there we alot of units , I tried that.
@bassee 0.0001 in TSA , as it is proven TSA does not go under 0.27 , 0.0001 is faster than 0.27 , I dont know why
@Adiktuz yes i had in mine for bj unit facing , also this spell is instant no target so theres no target point and so no angle for points
For the damage configurability , first I need the units near the spell effect then Unitdamagetarget and after that a unit group to add the target so it does not get hit again becouse theyr are many dummy units.

 

[Axarion]

This is kind of simple and should have only a period of 0.03. Also you are using waits. Don't think that will get approved.

 

[Adiktuz]

yeah, and that's exactly how you should do it... learn how to add the units to a group specific to the unit...

for the effects, it will only lag if you used a very high poly thing or if you have so many units or, if each dummy would cause an effect on each unit, so the solution would be use a simple effect and only show it if the unit is hit, meaning you should do my 1st suggestion on this post...

using bj_unitfacing is kinda useless as it just returns a real value, so instead of using that bj, just use a real value directly...

for the angle, well you can't do anything now that its instant cast...

Summary

Learn how to:
-Do timed loops
-Add units to a group specific to the caster or the instance of the spell
-make your spells configurable
-remove useless bj variables

if you will be stubborn and keep this as it is, I don't see it will get approved...

 

[Dr Super Good]

function Trig_DNAWave_Conditions takes nothing returns boolean
if ( GetSpellAbilityId() == 'A002' ) then
return true
endif
return false
endfunction

Branching for this is stupid...
return GetSpellAbilityId() == 'A002'
returning the boolean product from the comparision is far neater and does not rely on flow control (easier for the CPU to handle).

GetOwningPlayer(a)

This value could be cached into a local to prevent it being recomputed.

Both r2 and r4 could be kept in radians thoughout the function to prevent the overhead of constant conversions.

Loading constants into locals is not a good idea idea as the locals have to be initialized each time the function runs. Rather use global constants as those could potentially be inlined for greater efficiency.

set r2 = r + 40*Cos(45+n*k) // the key to making curve,
set r4 = r3 +40*Cos(180+n*k) // why 180? yes i want to know that too,but works

As you should be aware by now, the trig functions work in radians and not degrees.
45 radians ~ 58 degrees
180 radians ~ 233 degrees
12 radians ~ 327 degrees or -32 degrees
Thus I have no idea what you are doing with your mathematics. I advise redesiging the solution, as it seems you may not have been aware of radians.

 

[Vercas]

The shape of DNA is called "double helix" or just 1 helix if you want just 1 chain. It features exactly 10 molecules every 360 degrees.

 

[Magtheridon96]

Bribe:

These waits need to be removed as

1) their wait period is random and never less than 0.26
2) it could occur with different intervals in replays
3) on battle.net a wait could under some conditions desync

Using a Timer to do this job will yield far better results.

You forgot to tell him that he has to null those leaking local variables