Shadow Images Sword v1.1

[Moderator]

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

15:57, 1st Jan 2015
Maker: IcemanBo and edo did a pretty good review.
Follow the advice given there.

 

[xorkatoss]

the spell it's self is a pretty neat idea.

the bug: this is what happens when 2 or more casts occur at the same time

 

[nhocklanhox6]

the spell it's self is a pretty neat idea.

the bug: this is what happens when 2 or more casts occur at the same time

That's my minor mistake, thanks >.<..

 

[dhguardianes]

I feel that, should be hidden shadow units when they are flying

 

[Kazeon]

• set dummy= CreateUnit(Player(15),GetUnitTypeId(caster),x,y,angle)

  Store Player(15) in a constant global variable

•                 if UnitAddAbility(dummy,FLY_ABILITY) and UnitAddAbility(caster,FLY_ABILITY) then
                      call UnitRemoveAbility(dummy,FLY_ABILITY)
                      call UnitRemoveAbility(caster,FLY_ABILITY)
                      call UnitAddAbility(dummy,'Aloc')
                      
                      call SetUnitVertexColor(dummy,255,255,255,DUMMY_ALPHA)
                      
                      call SetUnitX(dummy,x)
                      call SetUnitY(dummy,y)
                  endif

  Just follow the convention:
  

  if UnitAddAbility(dummy,FLY_ABILITY) and UnitRemoveAbility(dummy,FLY_ABILITY) then
  endif
  if UnitAddAbility(caster,FLY_ABILITY) and UnitRemoveAbility(caster,FLY_ABILITY) then
  endif

  Then put the rest actions outside the block. Currently, it may fails if user has a dummy unit with Crow Form ability defaultly from OE (some coders prefer this way). And call UnitAddAbility(dummy,'Aloc'), dummy unit should have locust ability by default in OE. Just remove that line.
• For sake of elegance, I would prefer to add all of dummy abilities via trigger. Beside, you only need those abilities just for one dummy caster. And in case users want to use their own dummy unit. Users don't need to have multiple dummy units for every spell on their maps right?
• Personally, I don't like spells which make the caster become invulnerable. It's often considered as overpowered spell.
• Stun ability has 5 levels?

 

[edo494]

Shadow Images Sword v1.1 - Review by IcemanBo & edo494​ Concept: Hero jump up to the sky, and while mid-air, he creates 2 illusions that jump to different directions, and upon reaching the ground they deal varying AoE damage and stun all enemies in certain range for certain time. After they land, the illusions disappear and the caster goes invisible for certain amount of time. Code: instead of it being library, make it a scope. Library is used for systems that are meant to be seen by the whole code. instead of using strings, you could use orderids(integers) in the configurable Spell should not work in deep water, because if unit gets pushed there(or caster jumps there). You will have to somehow check if user doesnt try to jump into deep water Inside AddSpecialEffectZ you dont use the destructable you create, if you meant to create special effect on the destructable, use AddSpecialEffectTarget(path, d, "some attachment"). If you dont, you can directly return AddSpecialEffect(path, x, y) struct KnockBackZ => private struct KnockBackZ, struct ShadowImagesSword => private struct ShadowImagesSword Inside ShadowImagesSword.onPeriodic, inside the if not stopFading then and if percent>= DISTANCE_PERCENT then you create 2 effects, but you never remove them(if reach>=max evaluates to false). My(Edo) point of view on this is that if you create handle in certain scope, you should remove it in that scope, or before end of loop or end of function. You do neither in that if, you follow this on the rest tho. Return statement inside KnockBackZ.onPeriodic inside the else block is useless. It may cause glitches, because you can potentially stop iterating mid-way The same return statement problem appears inside ShadowImagesSword.onPeriodic, you potentially return while iterating over the list. 57.29583, *.0174533 => what is this magical sorcery constant? if UnitAddAbility(dummy,FLY_ABILITY) and UnitAddAbility(caster,FLY_ABILITY) then could be if UnitAddAbility(dummy,FLY_ABILITY) and UnitAddAbility(caster,FLY_ABILITY) and UnitRemoveAbility(dummy,FLY_ABILITY) and UnitRemoveAbility(caster,FLY_ABILITY) then, + it can fail if unit already has crow form ability on it when you call it(it will return false) set dummy= CreateUnit(Player(15),GetUnitTypeId(caster),x,y,angle) You could store Player(15) inside constant variable, or use one provided by blizzard(Yes, I am looking at you PLAYER_NEUTRAL_PASSIVE Quality of life: You dont need to actually loop inside the onInit, you could've just called the BJ, it does it for you anyways. You also dont need to null the trigger, as you never destroy it(I agree it is good practice, but technically just waste of 22 bytes ) Inside ShadowImagesSword.onPeriodic you periodically call SelectUnit(caster,false/true), and this is not really mentioned, nor is it really required. You could make a constant boolean DE_SELECT_ON_PERIODIC and wrap the selection in static if Also, you should follow JPAG with naming, for instance private integer M=-1 is not really descriptive. And yes, not only the configurable should be descriptive. Visuals: When jumping over cliffs, the jump does not look very smooth The skill icon does not fit well into the spell tooltip could use some love(Improve the wordings, because it is kind of a mess as of now) The ending effect is not really all that fitting, considering its origins(Mind control I believe) Conclusion: The code is pretty solid, with a few flaws in it, but no real logical errors. The concept of the spell is above average, because it "stands out of ordinary" with the spells of type "Jump, hit ground, knockback, damage", but this spell does a little bit unique stuff by spliting to two different units and Comboing the units. The visuals could've been improved, mainly the tooltip and icon of the spell. Rating: 3.5/5 Unique, will change to 4/5 when updated

 

[IcemanBo]

As stated by edo 3.5/5 for now, but definitly 4/5 when updated... for motivation. And a good spell.

 

[nhocklanhox6]

Thank all of you. Will fix it soon^^.