• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

Mark of Punishment v1.3

This is a spell based on the DotA spell Sleight of Fist however with some difference:
- Caster do not attack but directly damages targets
- The spell creates a circular border which disables the marked units to go out until they are unmarked
- The Caster deals cleaving damage to certain AOE from its main target
- Most of the spell stats (like cleave AOE, damage, cleave damage percentage) are dependent to your hero's attributes

JASS:
///////////////////////////////////////////////////////////////////////////////////////////////////
// ============================================================================================= //
// ========================== ***    ****   ***   ****   *   *  **** =========================== //
// ========================== *  *   *     *   *  *   *  ** **  *    =========================== //
// ========================== ***    ***   *****  *   *  * * *  ***  =========================== //
// ========================== *  *   *     *   *  *   *  *   *  *    =========================== //
// ========================== *   *  ****  *   *  ****   *   *  **** =========================== //
// ============================================================================================= //
///////////////////////////////////////////////////////////////////////////////////////////////////
//                                                                                               //
// ***************************                                                                   //
// * Mark of Punishment v1.3 *                                                                   //
// *        by: AGD          *                                                                   //
// ***************************                                                                   //
//                                                                                               //
//                                                                                               //
// How to Import (In Order):                                                                     //
//                                                                                               //
// 1. Copy the dummy in object editor                                                            //
// 2. Copy the "Mark of Punishment" ability                                                      //
// 3. Copy the "Mark of Punishment by AGD" trigger category                                      //
//                                                                                               //
// But first, check the "Automatically create unknown variables when pasting trigger data"       //
// in the preference menu so you dont have to create the global variables one by one.            //
// This will help you in future importing as well.                                               //
//                                                                                               //
// Note:                                                                                         //
// Please give credits to the author if you use this resource in your map.                       //
//                                                                                               //
//                                                                                               //
// Spell Summary:                                                                                //
//                                                                                               //
// The spell will create a dummy unit with similar model as the caster at its location. The      // 
// spell will then create a circular border and will mark every target in its area. These marked //
// units will be restricted to go out of the border. The caster will then be teleporting to a    //
// random one  unit from the marked targets and will damage it and then its mark will be         //
// removed and thus, allowing it go out of the border. Units within a certain radius from        //
// that target will then receive percentage damage from the damage done to that target. The      //
// caster will then continue to execute this action until all marks will be removed. After then, //
// the caster will move back to the dummy's location and the special effects of the circular     //
// border will disappear as well.                                                                //
//                                                                                               //
//                                                                                               //
///////////////////////////////////////////////////////////////////////////////////////////////////
// ============================================================================================= //
// ============================================================================================= //
// ============================================================================================= //
///////////////////////////////////////////////////////////////////////////////////////////////////




///////////////////////////////
//***************************//
//** Configuration Section **//
//***************************//
///////////////////////////////

////////////////////////////////////////////////////////////////
// Determines the rawcode of the activation spell             //
////////////////////////////////////////////////////////////////
constant function MOP_SpellID takes nothing returns integer
    return 'MOP1'
endfunction

////////////////////////////////////////////////////////////////
// Determines the unitID of the remnant                       //
////////////////////////////////////////////////////////////////
constant function MOP_DummyID takes nothing returns integer
    return 'mop2'
endfunction

////////////////////////////////////////////////////////////////
// Determines the hero jump interval                          //
// ( Suggested value (x): 0.08 < x < 0.12 )                   //
////////////////////////////////////////////////////////////////
constant function MOP_JumpInterval takes nothing returns real
    return 0.10
endfunction

////////////////////////////////////////////////////////////////
// Determines the loop interval in units imprisonment         //
////////////////////////////////////////////////////////////////
constant function MOP_LoopInterval takes nothing returns real
    return 0.03125
endfunction

////////////////////////////////////////////////////////////////
// Determines if the bonus strength is accounted for when     //
// computing spell stats dependent on hero attributes         //
////////////////////////////////////////////////////////////////
constant function MOP_IncludeBonusStr takes nothing returns boolean
    return true
endfunction

////////////////////////////////////////////////////////////////
// Determines if the bonus agility is accounted for when      //
// computing spell stats dependent on hero attributes         //
////////////////////////////////////////////////////////////////
constant function MOP_IncludeBonusAgi takes nothing returns boolean
    return true
endfunction

////////////////////////////////////////////////////////////////
// Determines if the bonus intelligence is accounted for when //
// computing spell stats dependent on hero attributes         //
////////////////////////////////////////////////////////////////
constant function MOP_IncludeBonusInt takes nothing returns boolean
    return true
endfunction

////////////////////////////////////////////////////////////////
// Determines the AOE of the spell                            //
////////////////////////////////////////////////////////////////
constant function MOP_AOE takes integer level returns real
    return 200.00 + 100.00*level
endfunction

////////////////////////////////////////////////////////////////
// Determines the base damage done by the spell to the main   //
// targets (Excluding damage bonuses due to hero attributes)  //
////////////////////////////////////////////////////////////////
constant function MOP_DamageBonus takes integer level returns real
    return 30.00 + 20.00*level
endfunction

////////////////////////////////////////////////////////////////
// Determines the damage bonus per attribute point            //
////////////////////////////////////////////////////////////////
constant function MOP_DamageDueAttr takes integer str, integer agi, integer int returns real
    return 0.00*str + 1.20*agi + 0.00*int
endfunction

////////////////////////////////////////////////////////////////
// Determines the AOE of the cleaving damage (Excluding       //
// cleave AOE bonuses due to hero attributes)                 //
////////////////////////////////////////////////////////////////
constant function MOP_CleaveAOEBase takes nothing returns real
    return 125.00
endfunction

////////////////////////////////////////////////////////////////
// Determines the cleave AOE per attribute point              //
////////////////////////////////////////////////////////////////
constant function MOP_CleaveAOEDueAttr takes integer str, integer agi, integer int returns real
    return 0.80*str + 0.00*agi + 0.00*int
endfunction

////////////////////////////////////////////////////////////////
// Determines the percentage of the cleaving damage           //
// (Excluding cleaving damage bonuses due to hero attributes) //
////////////////////////////////////////////////////////////////
constant function MOP_CDFBase takes nothing returns real
    return 30.00
endfunction

////////////////////////////////////////////////////////////////
// Determines the cleave damage percentage bonus per          //
// attribute point                                            //
////////////////////////////////////////////////////////////////
constant function MOP_CDFDueAttr takes integer str, integer agi, integer int returns real
    return 0.10*str + 0.00*agi + 0.00*int
endfunction

////////////////////////////////////////////////////////////////
// Determines the distance of the caster to its target when   //
// moving (Instantly) to its location                         //
////////////////////////////////////////////////////////////////
constant function MOP_MoveDistance takes nothing returns real
    return 65.00
endfunction

////////////////////////////////////////////////////////////////
// Determines how far each border special effect is to each   //
// other                                                      //
////////////////////////////////////////////////////////////////
constant function MOP_BorderEffectDistance takes nothing returns real
    return 80.00
endfunction

////////////////////////////////////////////////////////////////
// Determines the attack type when hitting the main targets   //
////////////////////////////////////////////////////////////////
constant function MOP_AttackType1 takes nothing returns attacktype
    return ATTACK_TYPE_NORMAL
endfunction

////////////////////////////////////////////////////////////////
// Determines the attack type when hitting the cleave targets //
////////////////////////////////////////////////////////////////
constant function MOP_AttackType2 takes nothing returns attacktype
    return ATTACK_TYPE_NORMAL
endfunction

////////////////////////////////////////////////////////////////
// Determines the damage type of the damage dealt to the main //
// targets                                                    //
////////////////////////////////////////////////////////////////
constant function MOP_DamageType1 takes nothing returns damagetype
    return DAMAGE_TYPE_NORMAL
endfunction

////////////////////////////////////////////////////////////////
// Determines the damage type of the cleaving damage          //
////////////////////////////////////////////////////////////////
constant function MOP_DamageType2 takes nothing returns damagetype
    return DAMAGE_TYPE_NORMAL
endfunction

////////////////////////////////////////////////////////////////
// Determines the weapon type when hitting the main targets   //
////////////////////////////////////////////////////////////////
constant function MOP_WeaponType1 takes nothing returns weapontype
    return null
endfunction

////////////////////////////////////////////////////////////////
// Determines the weapon type when hitting the cleave targets //
////////////////////////////////////////////////////////////////
constant function MOP_WeaponType2 takes nothing returns weapontype
    return null
endfunction

////////////////////////////////////////////////////////////////
// Determines the model used to mark the target units until   //
// the spell finishes                                         //
////////////////////////////////////////////////////////////////
constant function MOP_EffectModel1 takes nothing returns string
    return "Abilities\\Spells\\Other\\TalkToMe\\TalkToMe.mdl"
endfunction

////////////////////////////////////////////////////////////////
// Determines the model used to attach to the caster when     //
// moving around its targets                                  //
////////////////////////////////////////////////////////////////
constant function MOP_EffectModel2 takes nothing returns string
    return "Abilities\\Weapons\\PhoenixMissile\\Phoenix_Missile_mini.mdl"
endfunction

////////////////////////////////////////////////////////////////
// Determines the model used to mark around the area of the   //
// spell                                                      //
////////////////////////////////////////////////////////////////
constant function MOP_EffectModel3 takes nothing returns string
    return "Abilities\\Spells\\Other\\BreathOfFire\\BreathOfFireDamage.mdl"
endfunction

////////////////////////////////////////////////////////////////
// Determines the model upon damaging targets                 //
////////////////////////////////////////////////////////////////
constant function MOP_EffectModel4 takes nothing returns string
    return "Abilities\\Spells\\Human\\Feedback\\SpellBreakerAttack.mdl"
endfunction

////////////////////////////////////////////////////////////////
// Determines the attachment point of MOP_EffectModel1        //
////////////////////////////////////////////////////////////////
constant function MOP_EffectAttachPoint1 takes nothing returns string
    return "overhead"
endfunction

////////////////////////////////////////////////////////////////
// Determines the attachment point of MOP_EffectModel2        //
////////////////////////////////////////////////////////////////
constant function MOP_EffectAttachPoint2 takes nothing returns string
    return "weapon"
endfunction

////////////////////////////////////////////////////////////////
// Set to false if you don't want to change the caster's      //
// transparency                                               //
////////////////////////////////////////////////////////////////
constant function MOP_TransparencySwitch1 takes nothing returns boolean
    return true
endfunction

////////////////////////////////////////////////////////////////
// Set to false if you don't want to change the remnant's     //
// transparency                                               //
////////////////////////////////////////////////////////////////
constant function MOP_TransparencySwitch2 takes nothing returns boolean
    return true
endfunction

////////////////////////////////////////////////////////////////
// Determines the transparency of the caster                  //
////////////////////////////////////////////////////////////////
constant function MOP_Transparency1 takes nothing returns real
    return 65.00
endfunction

////////////////////////////////////////////////////////////////
// Determines the transparency of the remnant                 //
////////////////////////////////////////////////////////////////
constant function MOP_Transparency2 takes nothing returns real
    return 45.00
endfunction

////////////////////////////////////////////////////////////////
// The following set of functions determines the type of      //
// units to be allowed as the spell's target                  //
////////////////////////////////////////////////////////////////
//============================================================//
//==                        Allies                          ==//
constant function MOP_Allies takes nothing returns boolean
    return false
endfunction
//============================================================//
//==                      Air Units                         ==//
constant function MOP_Air takes nothing returns boolean
    return true
endfunction
//============================================================//
//==                      Structures                        ==//
constant function MOP_Structure takes nothing returns boolean
    return false
endfunction
//============================================================//
//==                      Mechanical                        ==//
constant function MOP_Mechanical takes nothing returns boolean
    return false
endfunction
//============================================================//
//==                       Ethereal                         ==//
constant function MOP_Ethereal takes nothing returns boolean
    return false
endfunction
//============================================================//
//==                       Illusion                         ==//
constant function MOP_Illusion takes nothing returns boolean
    return true
endfunction
//============================================================//
////////////////////////////////////////////////////////////////

//////////////////////////////
//**************************//
//** End of Configuration **//
//**************************//
//////////////////////////////




//////////////////////
//******************//
//** Loop Section **//
//******************//
//////////////////////
function MOP_AddGroup takes nothing returns nothing
    call GroupAddUnit( udg_MOP_TempGroup, GetEnumUnit() )
endfunction

function MOP_GetRandomUnit takes nothing returns nothing
    set udg_MOP_Iterated = udg_MOP_Iterated + 1
    if GetRandomInt( 1, udg_MOP_Iterated ) == 1 then
        set udg_MOP_RandomUnit = GetEnumUnit()
    endif
endfunction

function MOP_MainAction takes nothing returns nothing

    // Local variables setup
    local timer t          =  GetExpiredTimer()
    local unit u           =  LoadUnitHandle( udg_MOP_Hash, GetHandleId( t ), 1 )
    local integer key      =  GetHandleId( u )
    local integer a        =  0
    local real angle       =  bj_DEGTORAD*GetRandomReal( 0, 360 )
    local real origX       =  LoadReal( udg_MOP_Hash, key, 3 )
    local real origY       =  LoadReal( udg_MOP_Hash, key, 4 )
    local real targetX
    local real targetY
    local real tempX
    local real tempY
    local unit cleavetarget
    local unit target

    // Main target
    set udg_MOP_Iterated = 0
    set udg_MOP_RandomUnit = null
    call ForGroup( LoadGroupHandle( udg_MOP_Hash, key, 9 ), function MOP_GetRandomUnit )
    set target = udg_MOP_RandomUnit

    if target != null then

        set targetX = GetUnitX( target )
        set targetY = GetUnitY( target )
        call GroupEnumUnitsInRange( udg_MOP_TempGroup, targetX, targetY, LoadReal( udg_MOP_Hash, key, 14 ), null )
        set tempX = targetX + MOP_MoveDistance()*Cos( angle )
        set tempY = targetY + MOP_MoveDistance()*Sin( angle )
        call SetUnitX( u, tempX )
        call SetUnitY( u, tempY )
        call SetUnitFacing( target, bj_RADTODEG*Atan2( targetY - tempY, targetX - tempX ) )
        call UnitDamageTarget( u, target, LoadReal( udg_MOP_Hash, key, 13 ), true, false, MOP_AttackType1(), MOP_DamageType1(), MOP_WeaponType1() )
        call DestroyEffect( AddSpecialEffect( MOP_EffectModel4(), targetX, targetY ) )
        call DestroyEffect( LoadEffectHandle( udg_MOP_Hash, key, GetHandleId( target ) ) )

        // Cleave targets
        loop

            set cleavetarget = FirstOfGroup( udg_MOP_TempGroup )
            exitwhen cleavetarget == null
            call GroupRemoveUnit( udg_MOP_TempGroup, cleavetarget )

            if ( cleavetarget != target ) and ( IsUnitEnemy( cleavetarget, LoadPlayerHandle( udg_MOP_Hash, key, 8 ) ) or MOP_Allies() ) and ( not IsUnitType( cleavetarget, UNIT_TYPE_FLYING ) or MOP_Air() ) and ( not IsUnitType( cleavetarget, UNIT_TYPE_STRUCTURE ) or MOP_Structure() ) and ( not IsUnitType( cleavetarget, UNIT_TYPE_MECHANICAL) or MOP_Mechanical() ) and ( not IsUnitType( cleavetarget, UNIT_TYPE_ETHEREAL ) or MOP_Ethereal() ) and ( not IsUnitIllusion( cleavetarget ) or MOP_Illusion() ) and ( not IsUnitType( cleavetarget, UNIT_TYPE_DEAD ) ) and ( not IsUnitHidden( cleavetarget ) ) then

                call UnitDamageTarget( u, cleavetarget, LoadReal( udg_MOP_Hash, key, 15 ), true, false, MOP_AttackType2(), MOP_DamageType2(), MOP_WeaponType2() )

            endif

        endloop

        call GroupRemoveUnit( LoadGroupHandle( udg_MOP_Hash, key, 9 ), target )

    else

        // Clean ups
        if MOP_TransparencySwitch1() then

            call SetUnitVertexColor( u, 255, 255, 255, 255 )

        endif

        call DestroyEffect( LoadEffectHandle( udg_MOP_Hash, key, 10 ) )
        call RemoveUnit( LoadUnitHandle( udg_MOP_Hash, key, 2 ) )
        call SetUnitTimeScale( u, 1 )
        call SetUnitInvulnerable( u, false )
        call SetUnitX( u, origX )
        call SetUnitY( u, origY )
        call IssueImmediateOrder( u, "stop" )
        call SetUnitFacing( u, bj_RADTODEG*Atan2( LoadReal( udg_MOP_Hash, key, 6 ) - origY, LoadReal( udg_MOP_Hash, key, 5 ) - origX ) )

        // Removing border special effects
        loop

            set a = a + 1
            call DestroyEffect( LoadEffectHandle( udg_MOP_Hash, key, StringHash( "Border" + I2S( a ) ) ) )
            exitwhen a > LoadReal( udg_MOP_Hash, key, 16 )

        endloop

        call DestroyGroup( LoadGroupHandle( udg_MOP_Hash, key, 9 ) )
        call DestroyGroup( LoadGroupHandle( udg_MOP_Hash, key, 17 ) )
        call DestroyTimer( LoadTimerHandle( udg_MOP_Hash, key, 1 ) )
        call DestroyTimer( t )
        call FlushChildHashtable( udg_MOP_Hash, key )

    endif

    // Nullifying variables
    set u = null
    set t = null

endfunction

function MOP_LockUnits takes nothing returns nothing

    // Local variables setup
    local timer t        =  GetExpiredTimer()
    local unit u         =  LoadUnitHandle( udg_MOP_Hash, GetHandleId( t ), 1 )
    local integer key    =  GetHandleId( u )
    local real AOE       =  LoadReal( udg_MOP_Hash, key, 12 )
    local real centerX   =  LoadReal( udg_MOP_Hash, key, 5 )
    local real centerY   =  LoadReal( udg_MOP_Hash, key, 6 )
    local real dx
    local real dy
    local unit picked
    local real angle

    // Setting maximum distance for the marked units, preventing them from going outside the spell borders
    call ForGroup( LoadGroupHandle( udg_MOP_Hash, key, 9 ), function MOP_AddGroup )

    loop

        set picked = FirstOfGroup( udg_MOP_TempGroup )
        exitwhen picked == null
        call GroupRemoveUnit( udg_MOP_TempGroup, picked )
        set dx = GetUnitX( picked ) - centerX
        set dy = GetUnitY( picked ) - centerY
        set angle = Atan2( dy, dx )

        if dx*dx + dy*dy > AOE*AOE then

            call SetUnitX( picked, centerX + AOE*Cos( angle ) )
            call SetUnitY( picked, centerY + AOE*Sin( angle ) )

        endif

        // Filtering marked units
        // If one of these conditions return false, remove picked unit from the group and remove its mark
        if not ( ( IsUnitEnemy( picked, LoadPlayerHandle( udg_MOP_Hash, key, 8 ) ) or MOP_Allies() ) and ( not IsUnitType( picked, UNIT_TYPE_FLYING ) or MOP_Air() ) and ( not IsUnitType( picked, UNIT_TYPE_STRUCTURE ) or MOP_Structure() ) and ( not IsUnitType( picked, UNIT_TYPE_MECHANICAL) or MOP_Mechanical() ) and ( not IsUnitType( picked, UNIT_TYPE_ETHEREAL ) or MOP_Ethereal() ) and ( not IsUnitIllusion( picked ) or MOP_Illusion() ) and ( not IsUnitType( picked, UNIT_TYPE_DEAD ) ) and ( not IsUnitHidden( picked ) ) ) then

            call GroupRemoveUnit( LoadGroupHandle( udg_MOP_Hash, key, 9 ), picked )
            call DestroyEffect( LoadEffectHandle( udg_MOP_Hash, key, GetHandleId( picked ) ) )

        endif

    endloop

    // Nullifying variables
    set u = null
    set t = null

endfunction


/////////////////////////////
//*************************//
//** Spell Setup Section **//
//*************************//
/////////////////////////////
function MOP_Setup takes nothing returns boolean

    // Local variables setup
    local unit u
    local unit dummy
    local unit picked
    local player pl
    local group targetgroup
    local integer key
    local integer level
    local integer str
    local integer agi
    local integer int
    local integer targetcount = 0
    local integer a = 0
    local real origX
    local real origY
    local real centerX
    local real centerY
    local real AOE
    local real damage
    local real b
    local real angle
    local timer t1
    local timer t2

    if GetSpellAbilityId() == MOP_SpellID() then

        // Setup
        set u           =  GetTriggerUnit()
        set pl          =  GetOwningPlayer( u )
        set key         =  GetHandleId( u )
        set level       =  GetUnitAbilityLevel( u, MOP_SpellID() )
        set str         =  GetHeroStr( u, MOP_IncludeBonusStr() )
        set agi         =  GetHeroAgi( u, MOP_IncludeBonusAgi() )
        set int         =  GetHeroInt( u, MOP_IncludeBonusInt() )
        set origX       =  GetUnitX( u )
        set origY       =  GetUnitY( u )
        set centerX     =  GetSpellTargetX()
        set centerY     =  GetSpellTargetY()
        set AOE         =  MOP_AOE( level )
        set damage      =  MOP_DamageBonus( level ) + MOP_DamageDueAttr( str, agi, int )
        set b           =  2*bj_PI*AOE/MOP_BorderEffectDistance()
        set t1          = CreateTimer()
        set t2          = CreateTimer()
        set dummy       = CreateUnit ( pl, MOP_DummyID(), origX, origY, bj_RADTODEG*Atan2( centerY - origY, centerX - origX ) )
        set targetgroup = CreateGroup()

        call GroupEnumUnitsInRange( udg_MOP_TempGroup, centerX, centerY, AOE, null )

        // Setting remnant properties
        if MOP_TransparencySwitch2() then
            call SetUnitVertexColor( dummy, 255, 255, 255, R2I( 2.55*MOP_Transparency2() ) )
        endif

        call SetUnitAnimation( dummy, "stand" )
        call SetUnitPathing( dummy, false )
        call SaveGroupHandle( udg_MOP_Hash, key, 17, CreateGroup() )

        // Marking units
        loop

            set picked = FirstOfGroup( udg_MOP_TempGroup )
            exitwhen picked == null
            call GroupRemoveUnit( udg_MOP_TempGroup, picked )

            if ( IsUnitEnemy( picked, pl ) or MOP_Allies() ) and ( not IsUnitType( picked, UNIT_TYPE_FLYING ) or MOP_Air() ) and ( not IsUnitType( picked, UNIT_TYPE_STRUCTURE ) or MOP_Structure() ) and ( not IsUnitType( picked, UNIT_TYPE_MECHANICAL) or MOP_Mechanical() ) and ( not IsUnitType( picked, UNIT_TYPE_ETHEREAL ) or MOP_Ethereal() ) and ( not IsUnitIllusion( picked ) or MOP_Illusion() ) and ( not IsUnitType( picked, UNIT_TYPE_DEAD ) ) and ( not IsUnitHidden( picked ) ) then

                call GroupAddUnit( targetgroup, picked )
                call GroupAddUnit( LoadGroupHandle( udg_MOP_Hash, key, 17 ), picked )
                call SaveEffectHandle( udg_MOP_Hash, key, GetHandleId( picked ), AddSpecialEffectTarget( MOP_EffectModel1(), picked, MOP_EffectAttachPoint1() ) )
                set targetcount = targetcount + 1
                call SaveUnitHandle( udg_MOP_Hash, key, targetcount, picked )

            endif

        endloop

        // Preparing caster
        call SetUnitTimeScale( u, 10 )
       
        if MOP_TransparencySwitch1() then
            call SetUnitVertexColor( u, 255, 255, 255, R2I( 2.55*MOP_Transparency1() ) )
        endif

        call SetUnitInvulnerable( u, true )

        // Creating border special effects
        loop

            set a = a + 1
            set angle = 2*bj_PI*a/b
            call SaveEffectHandle( udg_MOP_Hash, key, StringHash( "Border" + I2S( a ) ), AddSpecialEffect( MOP_EffectModel3(), centerX + AOE*Cos( angle ), centerY + AOE*Sin( angle ) ) )
            exitwhen a > b

        endloop

        // Saving data to hashtable
        call SaveUnitHandle( udg_MOP_Hash, GetHandleId( t1 ), 1, u )
        call SaveUnitHandle( udg_MOP_Hash, GetHandleId( t2 ), 1, u )
        call SaveTimerHandle( udg_MOP_Hash, key, 1, t2 )
        call SaveUnitHandle( udg_MOP_Hash, key, 2, dummy )
        call SaveReal( udg_MOP_Hash, key, 3, origX )
        call SaveReal( udg_MOP_Hash, key, 4, origY )
        call SaveReal( udg_MOP_Hash, key, 5, centerX )
        call SaveReal( udg_MOP_Hash, key, 6, centerY )
        call SaveInteger( udg_MOP_Hash, key, 7, targetcount )
        call SavePlayerHandle( udg_MOP_Hash, key, 8, pl )
        call SaveGroupHandle( udg_MOP_Hash, key, 9, targetgroup )
        call SaveEffectHandle( udg_MOP_Hash, key, 10, AddSpecialEffectTarget( MOP_EffectModel2(), u, MOP_EffectAttachPoint2() ) )

        // Saving spell stats
        call SaveInteger( udg_MOP_Hash, key, 11, level )
        call SaveReal( udg_MOP_Hash, key, 12, AOE )
        call SaveReal( udg_MOP_Hash, key, 13, damage )
        call SaveReal( udg_MOP_Hash, key, 14, MOP_CleaveAOEBase() + MOP_CleaveAOEDueAttr( str, agi, int ) )
        call SaveReal( udg_MOP_Hash, key, 15, damage*( MOP_CDFBase() + MOP_CDFDueAttr( str, agi, int ) )*0.01 )
        call SaveReal( udg_MOP_Hash, key, 16, b )

        // Running loop functions
        call TimerStart( t1, MOP_JumpInterval(), true, function MOP_MainAction )
        call TimerStart( t2, MOP_LoopInterval(), true, function MOP_LockUnits )

        // Nullifying variables
        set u = null
        set dummy = null
        set targetgroup = null
        set t1 = null
        set t2 = null

    endif

    return false

endfunction


/////////////////////////////////
//*****************************//
//** Initialization Function **//
//*****************************//
/////////////////////////////////
function InitTrig_Mark_of_Punishment takes nothing returns nothing

    local trigger t = CreateTrigger()

    set udg_MOP_Hash = InitHashtable()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( t, Filter( function MOP_Setup ) )

endfunction


//////////////////////
//******************//
//** End of Spell **//
//******************//
//////////////////////

Changelogs:

v1.0
- First Upload

v1.1
- Put the configurations into constant functions
- Allowed target units are now included in the config section
- Hero jump interval and units imprisonment loop interval is now configurable
- Replaced all the locations with x/y coordinates
- Removed the excess locals as said by Tank
- Fixed the Special Effect leak
- Moved the dummy and ability IDs at the top for convenience
- Added configuration for the attachment points of the special effects
- Made some variable naming more decent
- You can now configure if you want to change the remnant's and caster's transparency or not ( so you can disable it if you have units with colors not equal to 255 )
- Added attacktype, damagetype, and weapontype to the configuration
- other minor changes

v1.2
- Combined the cast action and condition into one function
- Replaced the periodic events with timers
- Simplified the conditioning statements
- Replaced some local groups with recyclable global groups
- combined some configurations (constant functions)

v1.2b
- Moved the local variables setup and calculations after the casted spell matches the spellID
- Replaced the StringHashes with integers
- Removed some one time used local vars
- Used not IsUnitType( unit, UNIT_TYPE_DEAD ) to check if the unit is alive instead of checking if life is > 0

v1.3
- Slight code optimization
- Replaced the two BJs GroupPickRandomUnit() and GroupAddGroup with a custom function
- Set the unit imprisonment interval from 0.03 => 0.03125
- Added to the configuration whether to include stat bonuses for consideration in spell stats computation


Keywords:
Slash, Sleight of Fist, Omnislash
Contents

Mark of Punishment v1.3 (Jass) (Map)

Reviews
KILLCIDE
I think this spell is ready to be approved, but just consider these suggestions: 0.03 > 0.03125 loop interval because you can For MOP_MoveDistance, is the value defined how many units the caster will move over a second or for every loop iteration? A...

Moderator

M

Moderator


Reviewer:
KILLCIDE

Time:
15:50

Date:
08 June 2016

Review:
Link

Status:
Awaiting Update


Reviewer:
KILLCIDE

Time:
12:33

Date:
30 May 2016

Review:
Link

Status:
Awaiting Update
 
Last edited by a moderator:
Level 33
Joined
Apr 24, 2012
Messages
5,113
GetSpellTargetLoc -> GetSpellTargetX, GetSpellTargetY
and blah blah

You are now using JASS, please, as much as possible, use x/y coordinates instead of locations
Because this made me laugh(sorry):

JASS:
set tempX = GetUnitX(p)
set tempY = GetUnitY(p)
set loc = Location(tempX, tempY)
//...
set newX = GetLocationX(c) + AOE * Cos(angle * bj_DEGTORAD)
set newY = GetLocationY(c) + AOE * Sin(angle * bj_DEGTORAD)
call SetUnitX( p, newX )
call SetUnitY( p, newY )

, use timers instead of periodic triggers and use, as better case, indexing over groups

( (IsUnitType(p, UNIT_TYPE_ETHEREAL) == true) or (GetUnitState(p, UNIT_STATE_LIFE) <= 0) or (IsUnitHidden(p) == true) or (IsUnitEnemy(p, LoadPlayerHandle( udg_MOP_Hash, key, StringHash("Owner") )) == false)
->
IsUnitType(p, UNIT_TYPE_ETHEREAL) or GetUnitState(p, UNIT_STATE_LIFE) <= 0 or IsUnitHidden(p) or IsUnitEnemy(p, LoadPlayerHandle( udg_MOP_Hash, key, StringHash("Owner"))



This needs a lot of improvements. I just put down some details.
 
Level 37
Joined
Jul 22, 2015
Messages
3,485
On top of what Almia said, here's just a quick look at everything:
  • Spell configurables are normally kept in constant functions
  • It's okay to use TriggerRegisterAnyUnitEventBJ() to catch the spell event. The loop and extra variable just make it look more convoluted
  • SOF_SpellCheck is pontless since you can have SOF_Setup inside a function that returns a boolean
  • Instead of using a local variable that you have to constantly create and destroy, use GroupEnumUnitsInRange() alongside with FirstOfGroup() so you can just recycle it
  • It's faster to just multiply the same number twice over Pow()

The code is pretty readable excluding the fact you have some weird naming conventions like how a location is called c and how the first unit selected from FirstOfGroup() is p.
 
You've done the same thing as last time and put in loads of extra local variables on the setup function that do not need to be there - particularly because you use them only for one-time calculations and then store the results into hash tables once you're done (in the case of attributes for example this is an excess of 5 local variables for each different thing controlled by attributes)
 
Level 37
Joined
Jul 22, 2015
Messages
3,485
The spell concept is kind of overused. However, with the added cleave attack damage and range indicator, it is slightly different than the other omnilash-like spells we have here in the spell section. Both Almia and Tank-Commander made some good points you should consider, but here are other important things I noticed in your code:
  • All though there is no real difference between a function or constant function in JASS, it is normally good habit to use constant functions to show users that it is a configurable. It also makes it a lot easier to implement linear scaling for the configurables like so:
    JASS:
    constant function Damage takes integer level returns real
        return 50 + (50 * level)
    endfunction
  • A 0.10 second loop timer seems way too slow. Maybe consider sticking to 0.03125 and add a configurable that determines how fast the caster will switch between targets
  • Keep the unit and ability ID at the top of the configurable list
  • The loop function should only be on if there is a spell instance active
  • call DisableTrigger( gg_trg_Mark_of_Punishment ) & call DisableTrigger(t2) look useless
  • It's okay to use TriggerRegisterAnyUnitEventBJ() to catch the spell event. The extra local variable and loop make's it all convoluted and unnecessary
  • SOF_Setup can be moved into SOF_SpellCheck
  • I recommend adding a configurable of where MOP_EffectModel[2] attaches to
  • Some of your naming conventions are a little awkward (ex: local unit p). Please make them a little more obvious like "local unit target"
  • JASS:
    local group g
    set g = CreateGroup()
    
    // can be changed to
    
    local group g = CreateGroup()
  • Not all units have 255,255,255 vertex color across the board. This will also conflict with other systems that users can possibly have in their maps. I suggest adding a configurable that lets users change the colors to what they want AND if they even want to change the color in the first place
  • I recommend having the filter function for the group enumeration in a seperate function near the configurables so users can change it to their liking without having to dig for it in the code
  • I recommend adding a configurable of where MOP_EffectModel[1] attaches to
  • JASS:
    set angle = 360 / b * a
    set x = GetLocationX( c ) + AOE * Cos( angle * bj_DEGTORAD )
    set y = GetLocationY( c ) + AOE * Sin( angle * bj_DEGTORAD )
    
    // can be changed to
    
    set angle = (2*bj_PI) / b * a
    set x = GetLocationX( c ) + AOE * Cos(angle)
    set y = GetLocationY( c ) + AOE * Sin(angle)
  • call GroupAddGroup( udg_MOP_LoopGroup, loopgroup ) seems useless. Why not just use udg_MOP_LoopGroup alone?
  • Add configurables for the attack and damage type
  • You can set weapon type to null
  • JASS:
    call AddSpecialEffectLoc( udg_MOP_EffectModel[4], temploc )
    call DestroyEffect( bj_lastCreatedEffect )
    You leak a special effect here. MOP_EffectModel[4] isn't set to bj_lastCreatedEffect => call DestroyEffect(AddSpecialEffectLoc())
  • Instead of always turning on the trigger when it's already on, or turning off whenever there is no one in the group, I suggest using an integer counter that checks how many spell instances there are

For now, I will set the spell's status to Awaiting Update. Please add a changelog for your next update!
 
Last edited:

AGD

AGD

Level 16
Joined
Mar 29, 2016
Messages
688
The reason I did this: call GroupAddGroup( udg_MOP_LoopGroup, loopgroup ) every time before a loop is so that the picked unit (FirstOfGroup) will be removed only from the temporary group and not from the "actual" group, in which case the loop will exit next time it runs because the original group is already empty.

Any alternative for this?
 
Level 37
Joined
Jul 22, 2015
Messages
3,485
Code is starting look better, but there are still issues:
  • Instead of having seperate constant functions that calculates things like AoE and AoEPerLevel, you can put them together into one function like so:
    JASS:
    constant function MOP_AoE takes integer level returns real
        return 200.00 + (100.00 * level)
    endfunction
  • Please do what Almia suggested about using timers over periodic triggers
  • MOP_SpellCheck and MOP_Setup can be combined
  • loopgroup can be a global recycled group instead of a local created before every callback
  • In all of your condition statements, you do a lot of useless things like IsUnitEnemy() == true. Almia mentioned this in his review.

There are other issues in the code I would bring up, but I don't want to give you a long to do list like last time (especially how you have another submission set to Awaiting Update). Status set to Awaiting Update.
 
Level 14
Joined
Jul 1, 2008
Messages
1,314
Did I understand correctly, that you want to loop through a group and refill it back later?

You can use a simple group swap for this purpose:

JASS:
globals
        // group helper for group swap
        public group GROUP_SWAP = CreateGroup()
        public group GROUP_TEMP
    endglobals
  
    private function fgSwap takes nothing returns nothing
        local unit u
        loop
            set u = FirstOfGroup( your_group)
            exitwhen u == null
            //
            call GroupAddUnit( GROUP_SWAP, u)
            call GroupRemoveUnit( your_group, u)
        endloop
        set GROUP_TEMP   = your_group
        set your_group      = GROUP_SWAP
        set GROUP_SWAP   = GROUP_TEMP
    endfunction

EDIT: sry only realized now, that Almia already suggested, what I posted here.
 
Last edited:
Level 37
Joined
Jul 22, 2015
Messages
3,485
KILLCIDE
I'm not sure what you mean by this. Do you mean adding MOP_SpellCheck to a conditional if/then/else inside MOP_Setup?

JASS:
function OnCast takes nothing returns boolean
    if (GetSpellAbilityId() == 'A000') then
        //do MOP_Setup actions here
    endif
  
    return false
endfunction

function InitTrig_RandomSpell takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(t, Condition(function OnCast))
endfunction
 

AGD

AGD

Level 16
Joined
Mar 29, 2016
Messages
688
1. Instead of having seperate constant functions that calculates things like AoE and AoEPerLevel, you can put them together into one function like so:
2. Please do what Almia suggested about using timers over periodic triggers
3. MOP_SpellCheck and MOP_Setup can be combined
4. loopgroup can be a global recycled group instead of a local created before every callback
5. In all of your condition statements, you do a lot of useless things like
IsUnitEnemy() == true
. Almia mentioned this in his review.

1. Done
2. Done
3. Done
4. Done
5. Done
 
Level 37
Joined
Jul 22, 2015
Messages
3,485
The code is almost there :D sorry for being so picky, but this is what you get for uploading more resources before you get the others fixed!
  • Should you really be calculating all those locals in MOP_Setup before you check if the right ability was even casted? This can help with lag for people who have like a billion spells in their map :p
  • GetUnitState( picked, UNIT_STATE_LIFE ) > 0 is not a proper way to check if a unit is alive. A unit is classified dead in WC3 if their life is <= 0.405. Use not IsUnitType(picked, UNIT_TYPE_DEAD)
  • You should only have to create a local if you are going to use it in one more than one line. I see quite a bit of it in your code. An example would be:
    JASS:
    set SFX = AddSpecialEffectTarget( MOP_EffectModel2(), u, MOP_EffectAttachPoint2() )
    //... random code
    //... a lot of random code
    call SaveEffectHandle( udg_MOP_Hash, key, StringHash("Effect"), SFX )
  • Do you mind explaining what the 0.01 means here? Random numbers are never good in code :D some explanation would be nice
    call SaveReal( udg_MOP_Hash, key, StringHash("CleaveDamage"), damage * ( MOP_CDFBase() + MOP_CDFDueAttr( str, agi, int ) ) * 0.01 )
Status is still set at Awaiting Update. Almost there :D
 

AGD

AGD

Level 16
Joined
Mar 29, 2016
Messages
688
The code is almost there :D sorry for being so picky, but this is what you get for uploading more resources before you get the others fixed!
Its okay:D

Do you mind explaining what the 0.01 means here? Random numbers are never good in code :D some explanation would be nice
call SaveReal( udg_MOP_Hash, key, StringHash("CleaveDamage"), damage * ( MOP_CDFBase() + MOP_CDFDueAttr( str, agi, int ) ) * 0.01 )
The MOP_CDFs are multiplied by 0.01 to convert them from percent to decimal (35%=>0.35) before multiplying it to the damage

I'll start fixing
 

AGD

AGD

Level 16
Joined
Mar 29, 2016
Messages
688
Should you really be calculating all those locals in MOP_Setup before you check if the right ability was even casted? This can help with lag for people who have like a billion spells in their map :p
Done
GetUnitState( picked, UNIT_STATE_LIFE ) > 0
is not a proper way to check if a unit is alive. A unit is classified dead in WC3 if their life is <= 0.405. Use
not IsUnitType(picked, UNIT_TYPE_DEAD)
Done
You should only have to create a local if you are going to use it in one more than one line. I see quite a bit of it in your code. An example would be:
Done
Do you mind explaining what the 0.01 means here? Random numbers are never good in code :D some explanation would be nice
I explained it above ^
 
Level 37
Joined
Jul 22, 2015
Messages
3,485
I think this spell is ready to be approved, but just consider these suggestions:
  • 0.03 > 0.03125 loop interval because you can
  • For MOP_MoveDistance, is the value defined how many units the caster will move over a second or for every loop iteration? A quick clarification for that will be useful for some users
  • GetOwningPlayer() > GetTriggerPlayer()
  • Keep some consistency with your code! local unit cleavetarget > local unit cleaveTarget
Everything looks good to me! The code is organized, fairly easy to read (excluding the use of StringHash() (; ), and extremely easy configurability. I would give this a 4/5, but due to originality, I'll have to bop it down to 3/5. Nonetheless, Approved!
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
Instead of having seperate constant functions that calculates things like AoE and AoEPerLevel, you can put them together into one function like so:

For affine transformations you can argue either way.

GetOwningPlayer() > GetTriggerPlayer()

What purpose does this server? (I know the answer, do you?) This isn't necessary.
 

Cokemonkey11

Code Reviewer
Level 29
Joined
May 9, 2006
Messages
3,516
How could you argue with affine transformation here?
It does not really bring you any benefits to build up functions on each other just for affine sake.

I don't understand the question. My point is that, for simple relationships (like affine transforms), having the extra function may or may not aid readability and maintainability.
 
Top