Venom Spell v1.7a

[xyzier_24]

You should post the triggers.

 

[Bribe]

You should post the triggers.

You can find triggers through here: Trigger Viewer | HIVE

 

[Pyrogasm]

Gives me a "The requested resource could not be found." error page, @Bribe.

 

[Bribe]

The link changes each time the map is changed. now it's Trigger Viewer | HIVE

 

[xyzier_24]

Okay. This needs description though.

 

[Bribe]

Okay. This needs description though.

Yeah the tooltips could stand to be typed out at least instead of just be pictures.

 

[xyzier_24]

Yeah the tooltips could stand to be typed out at least instead of just be pictures.

What I meant is the Setup Trigger. Some of the variables might be confusing to others.

 

[JC Helas]

Yeah the tooltips could stand to be typed out at least instead of just be pictures.

I want to,, but I forgot some of the code which hiding some descriptions,, I only remeber, [size=#] [trigger.] [jass.] [quote.]

 

[Bribe]

You're looking for [hidden.]

Also:

BB Codes | HIVE

 

[MyPad]

Spell Review:

Notes:

• In trigger Harp Setup, there is a specific line that might not guarantee correct behavior in Melee scoring (or so I've heard). The line in question...
  
  
  • Custom script: set udg_HarpBlink_BuffCaster = CreateUnit(Player(15), udg_HarpBlink_BuffCasterType, 0, 0, 0)
  ... might give it to the 16th player in later versions of warcraft 3, but would work as intended for previous versions (1.28.5-). A recommended course of action is to use this instead:
  
  
  • Custom script: set udg_HarpBlink_BuffCaster = CreateUnit(Player(PLAYER_NEUTRAL_PASSIVE), udg_HarpBlink_BuffCasterType, 0, 0, 0)
  In addition, a handle leak may occur (only once, though), involving the following array variable: udg_HarpDagger_GroupIndexed[1], since the object could be lost at the very first cast of the spell (from the assignment via set).
  
  
• In trigger Harp Dagger Cast, the following line does not seem to serve a purpose other than to incur additional memory usage (operation cost, if you will)
  
  
  • For each (Integer HarpDagger_Integer) from 1 to HarpDagger_NumberOfDagger, do (Actions)
    • Loop - Actions
      • -------- ---- --------
      • Unit Group - Add all units of HarpDagger_GroupIndexed[HarpDagger_Indexer] to HarpDagger_GroupIndexed[HarpDagger_Indexer]
  The function above would appear to add all the units within a certain group ... to itself?!
  
  
• In trigger Harp Blink Loop, the following line may cause some issues if the area of effect of the dummy ability is very small, or close to 0.
  
  
  • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
    • If - Conditions
      • HarpBlink_DistanceIndexed[HarpBlink_Loop] Less than or equal to HarpBlink_TravelSpeed
    • Then - Actions
      • -------- ---- --------
      • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
        • If - Conditions
          • (HarpBlink_Target[HarpBlink_Loop] has buff HarpDagger_BuffType) Equal to True
        • Then - Actions
          • Custom script: call SetUnitPosition(udg_HarpBlink_BuffCaster, GetUnitX(udg_HarpBlink_Target[udg_HarpBlink_Loop]),GetUnitY(udg_HarpBlink_Target[udg_HarpBlink_Loop]))
        • -------- ---- --------
    • Else - Actions
  Two solutions are possible, either alter the stun ability, or use SetUnitX and SetUnitY in place of the SetUnitPosition native.
  
  As for the first solution, you can simply make it a 0-cooldown, single-target ability. As for the second solution, SetUnitPosition will usually place the unit within a certain offset of the actual point, due to it performing path checks, whereas the natives above do as their name implies, without the pathing checks.

Suggestions:

• For the two spells included here, there is only one setup trigger. Why not split them into two triggers so that you save the user from having to scroll a bit to modify wanted data?
  
  
• In trigger Harp Dagger Loop, there is a possibility of the dummy missile to not behave as a true missile, due to the usage of SetUnitPosition, which may make the result look awkward (e.g. Missile not moving through its' trajectory). Rectify this with SetUnitX and SetUnitY instead.
  
  Unlike the case above within the trigger Harp Blink Loop, the missile, if displaced using SetUnitX and SetUnitY, might cause the dummy unit to go out of bounds, given extreme range. That is why this is a suggestion instead of an urgent key-point that demands immediate attention.
  
  
• Allow some sort of counter mechanism for attacking, where the number of attacks when applying the bonus damage can be made modifiable. (Instead of 1 attack stack, you can make it 2, 3 or 5 stacks before being reduced).

Overview:

The Harp Spell has some potential to become a decent spell, though the idea behind it (given that it isn't necessarily the same) has been seen in a lot of media, which lowers the rating a bit. The spell itself is highly resistant to leaks, and optimized to the best extent that it had been by the author, bumping it's rating upward.

A lot of thought was given on how the spell/s would work, based on how the triggers themselves were implemented, although the presence of unnecessary variables digresses a commendable effort. Clearly, the spell concept and implementation has been showered with utmost care by the author. Yet, an update will be required before the final rating will be given.

Needless to say, as it is, the overall rating (on the scale from 1 to 5, with 5 being the highest) is: 3.7
​

Status:

• Awaiting Update

 

[JC Helas]

I set the HarpDagger Dummy's movement into SetUnitX,SetUnitY,, the result is the dummy is invisible,, i cant see it,, but the damaging is working and so the buff.. maybe the effect that i attached is not moving along to the dummy,, so I change my mind,, I better make an effect on each path where it moves.
or maybe I will create another dummy object and set a constant model of the effect that I attach,, instead doing a special effect ..

solved:
The movement speed of dummy must set atleast 1

 

[MyPad]

I think I figured out the cause, the movement speed of the projectile is accounted for when using the coordinate setter natives, whereas SetUnitPosition forces the displacement of these units. The solution to this is to set the movement speed of the dummy unit type being used to at least 1 (that way, a coordinate setter native will work on the unit).

This may be an interesting find, albeit a bit off-topic, about the behavior of SetUnitX, SetUnitY and SetUnitPosition.

 

[JC Helas]

I think I figured out the cause, the movement speed of the projectile is accounted for when using the coordinate setter natives, whereas SetUnitPosition forces the displacement of these units. The solution to this is to set the movement speed of the dummy unit type being used to at least 1 (that way, a coordinate setter native will work on the unit).

This may be an interesting find, albeit a bit off-topic, about the behavior of SetUnitX, SetUnitY and SetUnitPosition.

Exactly maybe some editors having this problem and cant figure out how to fix,, so go for it,, make a topic for this kind of behavior.

And after that please review my spell hehehe,, also credit me for the topic hehehehe

By the way,, my next update for this spell is,, I will add new spell,, "Harp Berserk"
Description:
Increases attack speed by 100/200/300/400% that last over 5 seconds, and blink to enemy within 600/650/700/750 distance that has a buff of dagger.

CD: 15 seconds
Manacost: 70

 

[Almia]

I think I figured out the cause, the movement speed of the projectile is accounted for when using the coordinate setter natives, whereas SetUnitPosition forces the displacement of these units. The solution to this is to set the movement speed of the dummy unit type being used to at least 1 (that way, a coordinate setter native will work on the unit).

This may be an interesting find, albeit a bit off-topic, about the behavior of SetUnitX, SetUnitY and SetUnitPosition.

it is actually an old behavior, which is why most Dummy libraries (such as MissileRecyler by Bribe) do not set the umvs (Unit's Move Speed) to 0.

 

[MyPad]

Looks like this will be a spellpack, neat-o. Just remember to split up the config triggers, and you're good to go.

@Almia

So it is the case that it was an already known bug, eh? Looks like I really need to refine my skills to dig deep back up.

 

[JC Helas]

Looks like this will be a spellpack, neat-o. Just remember to split up the config triggers, and you're good to go.

@Almia

So it is the case that it was an already known bug, eh? Looks like I really need to refine my skills to dig deep back up.

I change my mind sir to not update this reasources,, and by the way I already fixed it and your suggestion is already made.

 

[MyPad]

From my tests, there still exists some unnecessary overhead when casting either one of the spells. This happens once the enemy is stunned as the undefend order is being issued by the stunned target (due to the inclusion of a detector defend ability).

In conjunction with this, I noticed that you're using an older version of Unit Event. I suggest using the latest version to ensure that the spell will work optimally, or if you cannot import it into your map, all you need to change is this function, in trigger Unit Event:

function UnitEventCheckDeath takes nothing returns nothing
    local integer i = udg_CheckDeathList[0]
    set udg_CheckDeathList[0] = i
    loop
        exitwhen i == 0
        if udg_IsUnitNew[i] then
            //The unit was just created.
            set udg_IsUnitNew[i] = false
        elseif udg_IsUnitAlive[i] then
            //The unit has started reincarnating.
            set udg_IsUnitReincarnating[i] = true
            set udg_IsUnitAlive[i] = false
            set udg_UDex = i
            set udg_DeathEvent = 0.50
            set udg_DeathEvent = 0.00
        endif
        set i = udg_CheckDeathList[i]
        set udg_CheckDeathList[udg_CheckDeathList[i]] = 0    // add this line...
    endloop
endfunction

In trigger HarpBlink Cast, all the conditional statements within Pick Unit Group actions are hardcoded. Perhaps you can add some configurability onto this trigger via introduction of new flag variables?

Excluding dependencies, I would say the spell is ready for approval, as long as those things above are addressed either now or in the future.

EDIT:

Changed old to older

 

[Chaosy]

Expected music related spell, disappointed.

 

[JC Helas]

From my tests, there still exists some unnecessary overhead when casting either one of the spells. This happens once the enemy is stunned as the undefend order is being issued by the stunned target (due to the inclusion of a detector defend ability).

In conjunction with this, I noticed that you're using an older version of Unit Event. I suggest using the latest version to ensure that the spell will work optimally, or if you cannot import it into your map, all you need to change is this function, in trigger Unit Event:

function UnitEventCheckDeath takes nothing returns nothing
    local integer i = udg_CheckDeathList[0]
    set udg_CheckDeathList[0] = i
    loop
        exitwhen i == 0
        if udg_IsUnitNew[i] then
            //The unit was just created.
            set udg_IsUnitNew[i] = false
        elseif udg_IsUnitAlive[i] then
            //The unit has started reincarnating.
            set udg_IsUnitReincarnating[i] = true
            set udg_IsUnitAlive[i] = false
            set udg_UDex = i
            set udg_DeathEvent = 0.50
            set udg_DeathEvent = 0.00
        endif
        set i = udg_CheckDeathList[i]
        set udg_CheckDeathList[udg_CheckDeathList[i]] = 0    // add this line...
    endloop
endfunction

In trigger HarpBlink Cast, all the conditional statements within Pick Unit Group actions are hardcoded. Perhaps you can add some configurability onto this trigger via introduction of new flag variables?

Excluding dependencies, I would say the spell is ready for approval, as long as those things above are addressed either now or in the future.

EDIT:

Changed old to older

Latest Unit Event is now imported,, and added some configurable flag that conditionally pick an enemy of harp blink also I fixed the missile's flag that checks the enemy's condition weather alive or not,,I use the variable of unit event which is "isUnitAlive[]" also instead of calling the target's user data in loop triggeraction I create a variable integer that will do it once at trigger cast.

Expected music related spell, disappointed.

Ahmm I refer harp for it sounds like creeps,, like murloc hehehe I forgot the real world which means harp is an instrument hahaha

 

[Bribe]

Chaosy means that a Harp is a musical instrument, and the fact this spell lacks a musical component is a disappointment.

 

[Wrda]

Yes, everytime I look in the spell section and see this spell I always wonder what exactly means simply because I see no harp, no glissandos damaging the units and no pedals
Epic spell to get baited everytime. We have been bamboozled!

 

[KILLCIDE]

Needs Fixed

• Please follow the GPAG - GUI Proper Application Guide
• More documentation in the triggers would be appreciated.

Suggestions

• It's not necessary to "null" variables such as Angle, AbilityLevel[], AngleDistance, etc at the end of the cast trigger.
• HarpDaggerMissleSize is a constant, so instead of multiplying it by 100 everytime the spell is cast, just multiply it by 100 at the end of the setup triggers and reference that instead.
• Make the periodic timer a configurable variable
• The DamageDetect trigger shouldn't be on unless a spell instance is active

Status

Awaiting Update​

 

[JC Helas]

actaully I didnt expect that there was already meaning for Harp,, I had no Idea what harp is,, I thought It is some kind of creepy,, because it is sounds like creepy sorry ✌

 

[JC Helas]

waiting for review please

 

[MyPad]

Now that this is in JASS, there would be a few points that need to be addressed:

• Variables V_LoopTrig, VB_LoopTrig, and VA_LoopTrig behave like timers, despite the fact that they are triggers. A recommendation is to change the types of these variables into timers (Countdown Timers in GUI). This would automatically generate the needed handles during the initialization phase.
  
  
• Remember to null dynamic handles in JASS/vJASS. In functions VB_Create, VD_Create, the spell leaks a unit handle every time it is called.

There are some quirky things about this version of the Venom Spell, such as the usage of a local in defining a customizable property, returning that local. ("property" in this case is defined as a constant function that returns some value).

Awaiting Update

 

[JC Helas]

Now that this is in JASS, there would be a few points that need to be addressed:

• Variables V_LoopTrig, VB_LoopTrig, and VA_LoopTrig behave like timers, despite the fact that they are triggers. A recommendation is to change the types of these variables into timers (Countdown Timers in GUI). This would automatically generate the needed handles during the initialization phase.
  
  
• Remember to null dynamic handles in JASS/vJASS. In functions VB_Create, VD_Create, the spell leaks a unit handle every time it is called.

There are some quirky things about this version of the Venom Spell, such as the usage of a local in defining a customizable property, returning that local. ("property" in this case is defined as a constant function that returns some value).

Awaiting Update

What else bro? :>

 

[MyPad]

There are some quirky things about this version of the Venom Spell, such as the usage of a local in defining a customizable property, returning that local. ("property" in this case is defined as a constant function that returns some value).

Aside from that, certain buffs need some renaming from "Harp" to "Venom". Regardless, this will be approved.

Approved