• Listen to a special audio message from Bill Roper to the Hive Workshop community (Bill is a former Vice President of Blizzard Entertainment, Producer, Designer, Musician, Voice Actor) 🔗Click here to hear his message!
  • Read Evilhog's interview with Gregory Alper, the original composer of the music for WarCraft: Orcs & Humans 🔗Click here to read the full interview.

Blessed Shield V1.11

This bundle is marked as useful / simple. Simplicity is bliss, low effort and/or may contain minor bugs.
Hello This is my first spell I post.

Discription: When the unit that has this spell gets taken under A% of his total health he has a B% chance to gain C amount of charges that will reduce the damage taken of the next C attacks by D%. Every time you take damage there is a E% chance a charge is removed and the damag reduction is lowered by F%. This effect cannot occure more then once every G seconds.
All capital letters are customizable.
Requires: Damage from J4L, AIDS from J4L, Event from J4L and TimerUtils from Vexorian
all systems are ofcourse in the demo map
Damage: http://www.thehelper.net/forums/showthread.php?t=131287
AIDS: http://www.thehelper.net/forums/showthread.php?t=130752
Event: http://www.thehelper.net/forums/showthread.php?t=126846
TimerUtils: http://www.wc3c.net/showthread.php?t=101322
Event
Script:
JASS:
scope BlessedShield initializer Init // requires AIDS, Damage, TimerUtils
//+-----------------------------------------------------------------------------------------------+
//| BLESSED SHIELD v1.11 by dudeim
//| ¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯    ¯¯¯¯¯¯
//| Description
//| ¯¯¯¯¯¯¯¯¯¯¯
//| Passive effect that causes the hero to receive charges when health drops below
//| 50%. Charges will reduce future damage by a percentage. Everytime damage is taken
//| a charge is removed and you will next time reduce less damage.
//|
//| Requires
//| ¯¯¯¯¯¯¯¯
//| - AIDS by Jesus4Lyf
//| - Damage by Jesus4Lyf
//|   (Event by Jesus4Lyf, as a requirement from Damage)
//| - TimerUtils by Vexorian
//|
//| Credits
//| ¯¯¯¯¯¯¯
//| Romek for helping - with a little error.
//| Bribe for helping - with a little error and cleaning conditions.
//| xBlackRose for    - almost entirely redoing my code to make it much more efficient and better looking
//|
//| How to import
//| ¯¯¯¯¯¯¯¯¯¯¯¯¯
//| Copy this trigger into your map.
//| Copy the "Systems" folder into your map.
//| Go to AIDS and Damage, follow their implementation steps regarding the object ability.
//|         OR
//| Copy the abilities from this map to your map. Be sure they keep the saw Rawcode.
//|
//| Changelog
//|¯¯¯¯¯¯¯¯¯¯
//| V1.00
//| Created and posted on the hiveworkshop
//| V1.10
//| xBlackRose rewrote lots of my code for efficenty and to make it better looking
//| The spell now features level based variables so you can edit it for each level
//| V1.11
//| Using GetWidgetLife instead of GetUnitState for efficiency
//| Fixed a variable name
//+-----------------------------------------------------------------------------------------------+

globals
//************************************************************************************************
//*                                    CONFIGURABLES SECTION                                         *
//************************************************************************************************

    private constant integer      AID                 = 'A000'    // Rawcode of the passive spell
    private constant integer    BOOK_ID             = 'A001'    // Rawcode of the spellbook, contains the aura which provides the visual buff.
  
    private constant boolean    STACK               = false        // If you already have charges and you take damage,
                                                                // will the charges stack or will they refill to initial value.

    private constant string     BLOCK_SFX           = "Abilities\\Spells\\Human\\DivineShield\\DivineShieldTarget.mdl" 
    private constant string     BLOCK_SFX_LOC       = "origin"

endglobals

        private function Cooldown takes integer level returns real
        //the cooldown on the spell if the spell is triggered (taking damage under x% health)
        //the cooldown triggers and you cannot have this spell triggered by that unit during
        //that time
        local real array cd
        set cd[1] = 10.00 // 10 seconds cooldown at lvl 1
        set cd[2] = 9.50 // 9.5 seconds cooldown at lvl 2
        set cd[3] = 9.00 // 9 seconds cooldown at lvl 3
        return cd[level]
        endfunction
        
        private function Reduction takes integer level returns real
        //the damage reduction of the unit when it has charges
        //at lvl 1 the unit will only take 50% of the incoming damage
        local real array red
        set red[1] = 50 // 50% dmg reduction at lvl 1
        set red[2] = 55 // 55% dmg reduction at lvl 2
        set red[3] = 60 // 60% dmg reduction at lvl 3
        return red[level]
        endfunction
        
        private function ReductionperCharge takes integer level returns real
        //if you lose damage reduction when you lose a charge
        // at lvl 1:
        //5 charges = 50% dmg reduction
        //4 charges = 40% dmg reduction
        //3 charges = 30% dmg reduction
        //etc..
        local real array red
        set red[1] = 10 // 10% dmg reduction loss when losing a charges at lvl 1
        set red[2] = 11 // 11% dmg reduction loss when losing a charges at lvl 2
        set red[3] = 12 // 12% dmg reduction loss when losing a charges at lvl 3
        return red[level]
        endfunction
        
        private function ChargesGain takes integer level returns integer
        //the amount of charges you gain at lvl 1 you will gain 5 charges
        local integer array char
        set char[1] = 5 // 5 charges gained at lvl 1
        set char[2] = 5 // 5 charges gained at lvl 2
        set char[3] = 6 // 6 charges gained at lvl 3
        return char[level]
        endfunction
        
        private function ChargesMax takes integer level returns integer
        //the maximum amount of charges you can ever have even when you have 
        //stack on
        local integer array char
        set char[1] = 5 // 5 charges gained at lvl 1
        set char[2] = 5 // 5 charges gained at lvl 2
        set char[3] = 5 // 5 charges gained at lvl 3
        // no use when you don't have stack on
        return char[level]
        endfunction
        
        private function ActivationPerc takes integer level returns real
        //the percentage health you need to be at or below before
        //the spell can trigger
        local real array perc
        set perc[1] = 50 // When below 50% health the spell can trigger at lvl 1
        set perc[2] = 53 // When below 53% health the spell can trigger at lvl 2
        set perc[3] = 55 // When below 55% health the spell can trigger at lvl 3
        return (perc[level]/100)
        endfunction
        
        private function ChargeChance takes integer level returns real
        //the chance that you actually get a charge instead
        //for example at lvl 1 you have 50% chance that you get the charges
        //you take damage below 50% health and you can get a charge if you have
        //bad luck you won't get any charges
        local real array perc
        set perc[1] = 100 // 100% chance you will actually gain a charge at lvl 1
        set perc[2] = 100 // 100% chance you will actually gain a charge at lvl 2
        set perc[3] = 100 // 100% chance you will actually gain a charge at lvl 3
        return perc[level]
        endfunction
        
        private function KeepChargeChance takes integer level returns real
        //the chance that when an attack has landed on you and you have charges
        //that you will keep one. So you have 5 charges take damage. Due to this
        //you can still have 5 charges left.
        local real array perc
        set perc[1] = 0 // 0% chance you will keep a charge that would otherwise be lost at lvl 1
        set perc[2] = 0 // 0% chance you will keep a charge that would otherwise be lost at lvl 2
        set perc[3] = 0 // 0% chance you will keep a charge that would otherwise be lost at lvl 3
        return perc[level]
        endfunction
        
            
//************************************************************************************************
//*                            THE ABILITY BEGINS HERE - DO NOT TOUCH                                 *
//************************************************************************************************

    globals
        private unit tempUnit         = null                    // global unit used for filter
    endglobals

    private struct UnitData extends array
        //! runtextmacro AIDS()
        boolean inCooldown
        integer charges
        timer    t
        integer spellLevel
        
        private static method AIDS_filter takes unit u returns boolean
            return GetUnitAbilityLevel(u, 'Aloc') == 0
        endmethod
        
        private method AIDS_onCreate takes nothing returns nothing
            set this.inCooldown = false
            set this.charges    = 0
            set this.t      = null
            set this.spellLevel = 0
        endmethod
    endstruct
//where the damage reduction is done and the charges removed
private function BlockDamage takes nothing returns boolean
    local UnitData data  = UnitData[GetTriggerUnit()]
    local integer remainingCharges
    local real    reduction
    set data.spellLevel = GetUnitAbilityLevel(GetTriggerUnit(),AID)
    
    if data.charges > 0 then
        set remainingCharges = ChargesGain(data.spellLevel) - data.charges
        
        call DestroyEffect(AddSpecialEffectTarget(BLOCK_SFX, data.unit, BLOCK_SFX_LOC))
        if ReductionperCharge(data.spellLevel) > 0 then
            set reduction = Reduction(data.spellLevel) - (remainingCharges * ReductionperCharge(data.spellLevel))
        else
            set reduction = Reduction(data.spellLevel)
        endif

        if KeepChargeChance(data.spellLevel) < GetRandomInt(0, 100) then
            set data.charges = data.charges - 1
        endif

        if data.charges == 0 then
            call UnitRemoveAbility(data.unit, BOOK_ID)
        endif
        
        call Damage_Block(GetEventDamage() * (Reduction(data.spellLevel) / 100))
    endif
    
    return false
endfunction

// "callback" function. Where cooldown is placed.
private function ResetCooldown takes nothing returns nothing
    local UnitData data = GetTimerData(GetExpiredTimer())
    set data.inCooldown = false
    call ReleaseTimer(data.t)
endfunction
//Here are the charges given and cooldown is starting
private function ActivateAbility takes nothing returns nothing
    local UnitData data = UnitData[GetTriggerUnit()]
    set data.spellLevel = GetUnitAbilityLevel(GetTriggerUnit(),AID)
    
    if ChargeChance(data.spellLevel) >= GetRandomInt( 0, 100 ) and data.inCooldown == false then
        set data.inCooldown = true
        if data.charges == 0 then
            call UnitAddAbility( data.unit, BOOK_ID )
        endif
        
        static if STACK then
            if data.charges + ChargesGain(data.spellLevel) <= ChargesMax(data.spellLevel) then
                set data.charges = data.charges + ChargesGain(data.spellLevel)
            else
                set data.charges = ChargesMax(data.spellLevel)
            endif
        else
            set data.charges = ChargesGain(data.spellLevel)
        endif
        
        
        call SetTimerData( data.t, data )
        call TimerStart( data.t, Cooldown(data.spellLevel), false, function ResetCooldown )
    endif
endfunction
// Checking the conditions needed for the spell to run
private function AbilityCheckCondition takes nothing returns boolean
    local real lifePercent
    local UnitData data = UnitData[GetTriggerUnit()]
    set tempUnit = GetTriggerUnit()
    set lifePercent = GetWidgetLife(tempUnit) / GetUnitState(tempUnit, UNIT_STATE_MAX_LIFE)
    return GetUnitAbilityLevel(tempUnit, AID) > 0 and lifePercent <= ActivationPerc(data.spellLevel)              
endfunction
//the init function
private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    local integer i = 0
    call Damage_RegisterEvent( t )
    call TriggerAddCondition(t, function AbilityCheckCondition )
    call TriggerAddAction(t, function ActivateAbility )
    
    set t = CreateTrigger()
    call Damage_RegisterEvent(t)
    call TriggerAddCondition(t, function BlockDamage)
    
    loop
        call SetPlayerAbilityAvailable( Player(i), BOOK_ID, false )
        set i = i + 1
        exitwhen i == bj_MAX_PLAYER_SLOTS
    endloop
endfunction

endscope

Credits: Jesus4Lyf for the systems used, Vexorian for TimerUtils, Romek for fixing an error, Bribe for helping me with an error and cleaning my conditions and xBlackRose for redoing my code to make it much more efficient and better looking.

Changelog

V1.00
Created and posted on the hiveworkshop
V1.10
xBlackRose rewrote lots of my code for efficenty and to make it better looking
The spell now features level based variables so you can edit it for each level
V1.11
Fixed variable name
Use GetWidgetLife instead of GetUnitState

Hope you like it;)

Keywords:
Shield, Holy, Blessed, Divine, Damage, Reduction, Safeguard, Protection
Contents

Blessed Shield V1.11 (Map)

Reviews
12th Dec 2015 IcemanBo: Too long time as NeedsFix. Rejected. 12:15, 23rd Jul 2010 TriggerHappy: I would prefer that your level support supported more than three levels. Make some equation for each. Also, indent.

Moderator

M

Moderator

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

12:15, 23rd Jul 2010
TriggerHappy:

I would prefer that your level support supported more than three levels. Make some equation for each. Also, indent.
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
I have only checked the spell's code on this site.

Code Stuff:

  • Use GetWidgetLife(YourUnit)/SetWidgetLife(YourUnit,NewLife) instead of GetUnitState(YourUnit,UNIT_STATE_LIFE)/SetUnitState(YourUnit,UNIT_STATE_LIFE,NewLife)
    It's a bit faster.
  • You use GetTriggerUnit so many times in function Remove and add that it would be better to use a local variable for that.
  • You should disable the BOOKID ability for all players in the Init function.
  • Take
    JASS:
    call TimerStart(t,COOLDOWN,false,function callback)
    out of that big if-then-else block and put it last since it will always be called regardless of the condition.
  • Probably should try preloading BOOKID otherwise there will be a bit of lag upon the first time the ability is added.
Other:

  • Indenting is a bit strange.
  • Explanation at times was a bit awkward, but at least there's documentation. ;D
I will edit this post once I test this in-game.

Okay I tested it. The visual effects are a bit lacking. It might look better to use the Defend sfx instead of Divine Shield. (Well it can be changed so it's just my opinion.)

You should have multi-level support for this spell in case people would want to use this as a hero spell. That means making more of your configurables scale up with level. =P

This is just about the test-map: You should probably show that tip only once at map init.
You should also make it easier to test the spell by making the Paladin have less than 50% life at the start. (It's not that bad though since he can be instantly healed anyway)

Good luck on vJassing! =D
 
Last edited:
Level 17
Joined
Jun 17, 2007
Messages
1,433
  • The indenting is terrible, which makes some parts difficult to read.
  • In your 'cons' function, the variable isn't needed.
  • Instead of Set/GetUnitState use Get/SetWidgetLife.
  • Store GetTriggerUnit() in a variable.
  • This:
    JASS:
        if CHARGECHANCE > 0 then
            if CHARGECHANCE < rand then
                set currentcharges[GetUnitId(GetTriggerUnit())] = currentcharges[GetUnitId(GetTriggerUnit())] - 1
            endif
        else
            set currentcharges[GetUnitId(GetTriggerUnit())] = currentcharges[GetUnitId(GetTriggerUnit())] - 1
        endif
    can be replaced with this:
    JASS:
        if not (CHARGECHANCE > 0 and CHARGECHANCE > rand) then
            set currentcharges[GetUnitId(GetTriggerUnit())] = currentcharges[GetUnitId(GetTriggerUnit())] - 1
        endif
  • Start the outside the if statement.
  • Your 'chleft' variable is no needed.
 
Last edited:
Level 15
Joined
Jul 6, 2009
Messages
889
Ergh. I have serious problems with your spell.

THREAD

Thread OP said:
Hello This is my first spell I post.

Name: Blessed Shield
Code: VJass
Leakless: Think so
Lagless: Should be
Mui: Yes
Customizable: Yes
Discription: When the unit that has this spell gets taken under A% of his total health he has a B% chance to gain C amount of charges that will reduce the damage taken of the next C attacks by D%. Every time you take damage there is a E% chance a charge is removed and the damag reduction is lowered by F%. This effect cannot occure more then once every G seconds.
All capital letters are customizable.
Requires: Damage from J4L, AIDS from J4L, Event from J4L and TimerUtils from Vexorian
all systems are in the demo map

  • The "Name" is redundant, all of us can see it from the top of the page.
  • Code: vJass, is once again, redundant, firstly, we can see it at the top, and it's not VJass but rather vJASS or vJass.
  • Leakless and lagless, MUI should not be even mentioned. Leakless and MUI is what spell code reviewers should automatically assume.

    "Spells must be free of bugs, leaks, unreasonable lag, and must be fully multiinstanceable."
  • "Customizable: Yes" - once again, useless.
    Rules said:
    Spells must be easy to configure (change their stats) and implement into a map. Documentation must be provided if the spell is not implemented with a simple copy and paste.
  • Description should be made easier to read. Right now, it's a wall of text.
  • Requirements should be linked, Jesus4Lyf's systems aren't on TheHiveworkshop, so link them.
  • "all systems are in the demo map" - obviously. How else would your spell demo function?

Some points are already mentioned above so ignore if so.

DEMO MAP

  • The map name should not be "Just another Warcraft III map", it should be "Blessed Shield v1.00" or something presentable.
  • Footman are so weak. Add some option to make seeing the map in action better, perhaps stronger units, or a trigger that damages your hero to enable it.
  • Spell tooltip is terrible.

    "When taken under 50% of your max life you will gain 5 blessed shield charges wich will reduce the damage by 100%. Everytime you take damage a charge is removed and the next time you will reduce 10% less damage!"

    Should be something to this effect:

    "When the Paladin's health drops below 50%, he will gain 5 blessed shield charges w[h]ich will reduce future damage. Everytime you recieve damage, a charge is loss and the shield blocks 10% less damage."
  • The ability should be a learnable hero ability, with proper tooltips, correct button position (since this is only one ability, (0,2) works fine. Perhaps find a more suitable icon that has a passive border and a learnable bordered icon.
  • I checked your abilities, you DO realize that Damage and AIDS both require an ability, it's even in the import instructions of them. Allow the objectMerger script to be run... I don't see them anywhere.

CODING

Here is what your header should like like (not exactly, but keep in mind presentation). //\\//\\//\\//\\//\\//\\//\\ looks exceptionally ugly.'

Your configuration is also ugly. You should have lines to seperate them and categorise them, rawcodes should be kept at the top if possible (unless there is some othe reason), and //comment comments should have spaces between the comment delimiter and the actual comment. // comment.

JASS:
//+-----------------------------------------------------------------------------------------------+
//| BLESSED SHIELD v1.00
//| ¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯
//| Description
//| ¯¯¯¯¯¯¯¯¯¯¯
//| Passive effect that causes the hero to recieve charges when health drops below
//| 50%. Charges will reduce future damage. (Whatever).
//|
//| Requires
//| ¯¯¯¯¯¯¯¯
//| - AIDS by Jesus4Lyf
//| - Damage by Jesus4Lyf
//|   (Event by Jesus4Lyf, as a requirement from Damage)
//| - TimerUtils by Vexorian
//|
//| Credits
//| ¯¯¯¯¯¯¯
//| Romek for helping - blah.
//| Bribe for helping - blah.
//| 
//| How to import
//| ¯¯¯¯¯¯¯¯¯¯¯¯¯
//| Copy this trigger into your map.
//| Copy the "Systems" folder into your map.
//| Go to AIDS and Damage, follow their implementation steps regarding the object ability.
//| 		OR
//| Copy the abilities from this map to your map. Be sure they keep the saw Rawcode.
//|
//+-----------------------------------------------------------------------------------------------+

Ignore any points already mentioned but:

1) This is redundant. You are checking if it is 0. Then resetting it to 0.
JASS:
.
    if currentcharges[GetUnitId(GetTriggerUnit())] == 0 then
        set currentcharges[GetUnitId(GetTriggerUnit())] = 0

2) Loop through every player onInit and disable it there to save any calls.

JASS:
call SetPlayerAbilityAvailable(GetOwningPlayer(GetTriggerUnit()), BOOKID, false)

3) You can just use if STACK then

JASS:
if STACK == true then

Although it's not a big deal.

4) You are using up timers if the if block in function add is not met, and taking up structs as you don't destroy them.

5) You can inline this.
JASS:
local integer rand = GetRandomInt(1,100)

6) Your function names are terrible, they should be initial capitals and the functions should be grouped. I'm going to post an improved version of your code soon.

7) Rather than have an array, use an AIDS struct, for that is what it was made for, and intended to be used like that.

JASS:
.
        private integer array currentcharges 
        //to keep track of the current unit charges 
        private boolean array cd

8) Your globals are hard to understand O.O?

THIS is how your code is supposed to look like.

Erm. WTF does TheHiveworkshop jass tags do to the coding? It makes the spacing all messed up :mad:

JASS:
scope BlessedShield initializer Init // requires AIDS, Damage, TimerUtils
//+-----------------------------------------------------------------------------------------------+
//| BLESSED SHIELD v1.00 by dudeim
//| ¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯¯    ¯¯¯¯¯¯
//| Description
//| ¯¯¯¯¯¯¯¯¯¯¯
//| Passive effect that causes the hero to recieve charges when health drops below
//| 50%. Charges will reduce future damage. (Whatever).
//|
//| Requires
//| ¯¯¯¯¯¯¯¯
//| - AIDS by Jesus4Lyf
//| - Damage by Jesus4Lyf
//|   (Event by Jesus4Lyf, as a requirement from Damage)
//| - TimerUtils by Vexorian
//|
//| Credits
//| ¯¯¯¯¯¯¯
//| Romek for helping - blah.
//| Bribe for helping - blah.
//| 
//| How to import
//| ¯¯¯¯¯¯¯¯¯¯¯¯¯
//| Copy this trigger into your map.
//| Copy the "Systems" folder into your map.
//| Go to AIDS and Damage, follow their implementation steps regarding the object ability.
//| 		OR
//| Copy the abilities from this map to your map. Be sure they keep the saw Rawcode.
//|
//+-----------------------------------------------------------------------------------------------+

globals
//************************************************************************************************
//*								    CONFIGURABLES SECTION										 *
//************************************************************************************************

    private constant integer 	 AID 				= 'A000'	// Rawcodes should take priority, be at the top.
    private constant integer    BOOK_ID         	= 'A001'    // Rawcode of the spellbook, contains the aura which provides the visual buff.

																// Should be made into a function that takes a level. But anyways:
	private constant real 		LIFE_PERCENT		= 50.00		// The unit's health must match this or below for the ability to												
	private constant real		ACTIVATE_PERCENT	= 100.00	// Chance that you will gain a charge.
    private constant real       DMG_REDUCTION       = 100.00	// Percent that the damage is reduced by.
    
    private constant integer    CHARGES         	= 5			// How many instances are blocked.
    private constant integer    KEEP_CHARGE_PERCENT = 0 		// There is a 0% chance the unit will not lose a charge.
    private constant integer    MAX_CHARGES     	= 5			// How many charges can you keep? Useless I think?
    private constant real       CHARGE_REDUCTION    = 0		    // You lose 10% damage reduction for each charge loss.
																// 4 charges = 90% reduction
																// 3 charges = 80% reduction
    
    private constant boolean    STACK           	= false		// If you already have charges and you take damage,
                                                                // will the charges stack or be set to max.

    private constant string     BLOCK_SFX       	= "Abilities\\Spells\\Human\\DivineShield\\DivineShieldTarget.mdl" 
    private constant string     BLOCK_SFX_LOC   	= "origin"

    private constant real       COOLDOWN        	= 0.00
endglobals
//************************************************************************************************
//*							THE ABILITY BEGINS HERE - DO NOT TOUCH								 *
//************************************************************************************************

	globals
		private unit tempUnit 	    = null					// global unit used for filter
		private real sys_PERCENT = LIFE_PERCENT * 0.01	// Just so you don't need to calculate on every damage instance.
	endglobals

    // USE AIDS LIKE IT IS MEANT TO !
	private struct UnitData extends array
		//! runtextmacro AIDS()
		boolean inCooldown
		integer charges
		timer	timer
		
		private static method AIDS_filter takes unit u returns boolean
			return GetUnitAbilityLevel( u, 'Aloc' ) == 0
		endmethod
		
		private method AIDS_onCreate takes nothing returns nothing
			set this.inCooldown = false
			set this.charges	= 0
			set this.timer		= null
		endmethod
	endstruct

private function BlockDamage takes nothing returns boolean
    local UnitData data  = UnitData[GetTriggerUnit()]
    local integer remainingCharges
    local real    reduction
    
    if data.charges > 0 then
        set remainingCharges = CHARGES - data.charges
        
        call DestroyEffect(AddSpecialEffectTarget( BLOCK_SFX, data.unit, BLOCK_SFX_LOC) )
        if CHARGE_REDUCTION > 0 then
            set reduction = DMG_REDUCTION - (remainingCharges * CHARGE_REDUCTION)
        else
            set reduction = DMG_REDUCTION
        endif

		// 0% chance that you will not lose a charge.
		if KEEP_CHARGE_PERCENT < GetRandomInt( 0, 100 ) then
			set data.charges = data.charges - 1
		endif

        if data.charges == 0 then
            call UnitRemoveAbility( data.unit, BOOK_ID )
        endif
        
        call Damage_Block( GetEventDamage() * (reduction / 100 ) )
    endif
    
    return false
endfunction

// "callback" function. Where cooldown is placed.
private function ResetCooldown takes nothing returns nothing
    local UnitData data = GetTimerData( GetExpiredTimer() )
    set data.inCooldown = false
    call ReleaseTimer( data.timer )
endfunction

// Cleaned up a lot in here.
private function ActivateAbility takes nothing returns nothing
	local UnitData 	data = UnitData[GetTriggerUnit()]
	
	if ACTIVATE_PERCENT >= GetRandomInt( 0, 100 ) and data.inCooldown == false then
        set data.inCooldown = true
		if data.charges == 0 then
			call UnitAddAbility( data.unit, BOOK_ID )
		endif
		
        // STACK is a constant boolean. So we use a static if.
		static if STACK then
			// If max charges allowed is greater than 0.
			set data.charges = data.charges + CHARGES
		else
			set data.charges = CHARGES
		endif
		
        if data.timer == null then
            set data.timer = NewTimer()
        endif
		
		call SetTimerData( data.timer, data )
		call TimerStart( data.timer, COOLDOWN, false, function ResetCooldown )
	endif
endfunction

// GetTriggerUnit() was used more than once. Set a global unit (local will leak and will look messy if a local
// boolean is used) to it. lifePercent could be inlined, but it looks cleaner without doing so.
private function AbilityCheckCondition takes nothing returns boolean
	local real lifePercent
	set tempUnit = GetTriggerUnit()
    set lifePercent = GetUnitState( tempUnit, UNIT_STATE_LIFE ) / GetUnitState( tempUnit, UNIT_STATE_MAX_LIFE )
    return 	GetUnitAbilityLevel(tempUnit, AID) > 0 and lifePercent <= sys_PERCENT				
endfunction

private function Init takes nothing returns nothing
    // We don't need t and t2, we can just do it like so.
	local trigger t = CreateTrigger()
    local integer i = 0
    call Damage_RegisterEvent( t )
	call TriggerAddCondition(t, function AbilityCheckCondition )
	call TriggerAddAction(t, function ActivateAbility )
	
	set t = CreateTrigger()
    call Damage_RegisterEvent(t)
	call TriggerAddCondition(t, function BlockDamage)
    
    // Spellbook ID is disabled onInit rather than while the spell takes place.
    loop
        call SetPlayerAbilityAvailable( Player(i), BOOK_ID, false )
        set i = i + 1
        exitwhen i == bj_MAX_PLAYER_SLOTS
    endloop
endfunction

endscope

There are far more things I could comment about.

You shouldn't have posted this spell here in my opinion, you should have posted it in the triggering section to get feedback 'for your first spell'. I don't think it is going to be approved in it's current state - simply fixing everything mentioned won't work. I would take this into consideration, and make sure your next spell is presented at higher quality.
 
Last edited:
Level 4
Joined
Mar 27, 2008
Messages
112
Hmmm ok thanks guys will certainly work on it;)
Never understood the aids struct thing so didn't use it.
About the timer in the if block I first thought that if the percentage wasn't so high the the spell triggered the cooldown also didn't triggered but this is better I think otherwise the next hit would've triggered it.
@watermelon
why would I make a variable of triggering unit? it's kinda useless I'd say as it's already in some sort of variable why should I make a variable of it and use that instead? < One part I didn't really get
Thanks guys for the critisism will sure help me on my way!
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
Okay you updated it.

  • The way you supported levels in your abilities is inefficient. You should have either used a global array or a function that does calculations like
    JASS:
    private constant function Cooldown takes integer lvl returns real
        return 10.5 - 0.5*lvl
    endfunction
  • I'm pretty sure you should name the timer variable in your struct to something other than timer or else that will conflict. Also, data.unit? (Then again, that was from xBlackRose.)
  • You don't need to null the timer if you use TimerUtils as the timer just gets recycled. That also makes your null check useless.
  • Like hvo-busterkomo and I mentioned, "Instead of Set/GetUnitState use Get/SetWidgetLife."
 
Level 10
Joined
Sep 21, 2007
Messages
517
I dont know who the hell put that 1/5 rating; thats just absurd.. anyways.

The code is pretty good cus you are incorporating external libraries in it + doing traditional vJASS.

I think description should be changed, and ofc the coding improvements above should be made.

More functionality should be added and pfft thats about it i think?

gj on the spell mate
 
Level 4
Joined
Mar 27, 2008
Messages
112
The level functions i did where there for easy change so anyone with absolutly no skill in math or anything could easily change em and about the global array how would I do that? Where do I make it editable for each level then? Same way using a formula? Will change the names of the unit and timer variable and will use get/setwidgetlife. And the data.unit it refers to the unit wich the struct instance is about so no need to change that.

@up the first version of the spell lacked alot of things that was why the rating was 1/5 thanks to xblackrose it was updated alot!

V1.11 is up!
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
The level functions i did where there for easy change so anyone with absolutly no skill in math or anything could easily change em and about the global array how would I do that?
I like math so I like using functions instead. (Besides, the way you set up your level support was pretty much the same as using multiple ifs...)

Anyway, here's how you would do it if you use a global array variable.
JASS:
globals
    private real array Cooldown
endglobals

...

private function SetCooldown takes nothing returns nothing //Needs to be called at Init.
    set Cooldown[1] = 10.
    set Cooldown[2] = 9.5
    set Cooldown[3] = 9.
endfunction
And you would refer to Cooldown in your code like so:
Cooldown[GetUnitAbilityLevel...]
 
Level 15
Joined
Jul 6, 2009
Messages
889
I'm pretty sure you should name the timer variable in your struct to something other than timer or else that will conflict. Also, data.unit? (Then again, that was from xBlackRose.)

I didn't intend for him to copy it exactly. And, what's wrong with data.unit?

The code is pretty good cus you are incorporating external libraries in it + doing traditional vJASS.

That makes no sense. So as long as I use 'external' libraries, it is considered good code? And what is traditional vJASS o_O

Also, link all required libraries...
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
And, what's wrong with data.unit?
I just thought it would conflict since isn't there already unit? It seems like I was mistaken then.

Edit: Emphasis on the last statement.
To the post below, if he ever reads this again: I'm sorry that I didn't experiment with it, which I should have before saying something. However, I don't need someone yelling at me after I admitted I was wrong. >_>
 
Last edited:
Level 7
Joined
Jun 6, 2010
Messages
224
I just thought it would conflict since isn't there already unit? It seems like I was mistaken then.



How the heck does
JASS:
unit
stack with
JASS:
data.unit
?
pff....
JASS:
*/facepalm
unit is a reference of
JASS:
type unit extends handle
data.unit is a syntax of a struct called data.
we don't see that in JASS but we do in vJASS

This doesn't look like a system, it's more like a spell.
And it needs to be configurable!!! Also why the use of soooo many external libraries? I mean come on, i wouldn't use so many external libraries for just 1 single simple spell. -.-
 
Level 15
Joined
Jul 6, 2009
Messages
889
This doesn't look like a system, it's more like a spell.
And it needs to be configurable!!! Also why the use of soooo many external libraries? I mean come on, i wouldn't use so many external libraries for just 1 single simple spell. -.

  • It's not a system, it is a spell?
  • It is configurable.
  • You might not use so many vJass libraries, but others will. 3 is nothing. AIDS is a common indexing system, Damage is as well, it saves the author from having to code the damage detection himself. Why do what other's have done for you? And TimerUtils to attach the timer data. Hashtables are just damn plain ugly.
 
Top