• 🏆 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!

[System] Shield

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
Shield

Preface

Have you ever noticed that League of Legends contains many shield-based abilities? And DotA has very few? This isn't a coincidence. JASS simply doesn't contain an API for shields. There's no easy way to "shield a unit for up to 50 damage for up to 10 seconds" without long, boring scripts. This system aims to duplicate the functionality of league of legends shields with the easiest possible API.

Design Explanation

This library works by attaching an object to a unit which contains a special effect, a duration, and a shield amount. With some logic, we can actually display the unit's HP as being higher so that it will shield even damage that would otherwise kill it. If you attach a shield to a unit with a shield already, then the shield objects will link to each other. Checking the duration on the objects is done with a stack on a periodic timer.

Limitations:

  • This resource requires vJass to compile.
  • Shield requires DamageType which in turn requires StructuredDD, thus it is subject to those systems' limitations. This implies that Shield is likely incompatible with other damage detection systems.
  • Shield is in an awkward position where features could be taken away for a performance improvement, or added for improved functionality. Things like layers of shields aren't used in league of legends, so they're not included here.
  • There were many important considerations when designing this system in regards to "features vs performance". Some of these questions I asked myself include:
    • Should the system display temporary "false" hp on a shielded unit to visually show it will need more damage to kill it? Unlike league of legends, we have no way of modifying the color of different sections of the units health bar, and yet the system would be lacking in preventing damage if it were not included.
    • What if a unit is shielded and the result is that it temporarily has maximum HP? This implies that the passive hit point regeneration it would otherwise incur is now prevented. (I've accepted this as a limitation of the system)
    • If a unit has multiple shields, should I split the damage up into fractions for each shield member? This would surely be the most "fair" approach since for example a unit might have a shield that lasts 1 second and blocks 500 damage, and a shield that lasts 10 minutes and block 50 damage - but I have no way of knowing which one in the list is which. League of Legends implements a FILO technique (which happens to be the most efficient) - and I've done the same.

API


Shield.TIMER_PERIOD- This value refers to how often Shield should search for instances that are expired in regards to their duration. A smaller value means that shields are less likely to block damage when their duration has passed, while a larger value is less computationally expensive. A good value lies on [1./60.,1./2.].



Shield.add(unit u,real a,real t,string f,string p)- This is the one and only important method for instanciating a Shield. It imparts a shield of size 'a' for time 't' on 'u' using effect with model 'f' on attachment point 'p'. An 'a' value of -1. will shield unlimited damage for the duration. A 't' value of -1. will shield the unit indefinitely until 'a' is reached.


The script

JASS:
/**
 * Shield is a simple resource which is used to add temporary health to a unit. Shield has some
 * limitations which are addressed in the resource thread on hiveworshop.com. The API is as
 * follows: 
 *
 * Configurable: TIMER_PERIOD:
 * 		How often Shield should search for expired shields. A smaller value is more accurate
 * 		but will be more computationally expensive. Try something like 1/10 of a second to
 *      begin with.
 *
 * Public Method: Shield.add(unit target, real amount, real duration, string sfx, string attachPt):
 *		The only method necessary for instaniating a shield. A negatve duration lasts forever, and
 *		a negative amount shields an arbitrarily high value. 
 */
library Shield requires DamageType
    /**
	 * Native declaration in case it's not already done
	 */ 
    native UnitAlive takes unit u returns boolean

    /**
	 * This struct is just a facade for a pretty API (Don't try to Shield.create())
	 */ 
    struct Shield
        //< BEGIN CUSTOMIZABLE SECTION ****************************

        private static constant real TIMER_PERIOD=1./10.
        
        //> END CUSTOMIZABLE SECTION ******************************
        
		// Values for the instance stack
        private static thistype array allShields
        private static integer shieldIndex=-1
        private static timer clock=CreateTimer()
        private static HandleTable tab
        
		// Instance variables
        private boolean useDuration=true
        private boolean finiteShield=true
        private unit u
        private real shieldLeft
        private real falseHP
        private real timeLeft
        private effect fx
        private thistype prevShield=0
        private thistype nextShield=0
        
        /**
		 * The StructuredDD handler which catches damage events
		 */ 
        private static method handler takes nothing returns nothing
            local thistype tempDat
            local thistype destroyingDat
            local unit tU=GetTriggerUnit()
            local real damageLeft=GetEventDamage()
            local boolean noShieldsLeft=false
            if DamageType.get()!=DamageType.SPELL and UnitAlive(tU) and tab.exists(tU) then
                set tempDat=thistype.tab[tU]
                //we need to loop until either all the damage is mitigated
                //or there's no shield left on this unit.
                loop
                    exitwhen damageLeft<=0. or noShieldsLeft
                    if tempDat.shieldLeft>damageLeft then
                        if tempDat.finiteShield then
                            set tempDat.shieldLeft=tempDat.shieldLeft-damageLeft
                        endif
                        call SetWidgetLife(tempDat.u,GetWidgetLife(tempDat.u)+damageLeft)
                        set damageLeft=0.
                    else
                        set damageLeft=damageLeft-tempDat.shieldLeft
                        call SetWidgetLife(tempDat.u,GetWidgetLife(tempDat.u)+tempDat.shieldLeft)
                        set tempDat.shieldLeft=0.
                        if tempDat.falseHP>damageLeft then
                            set tempDat.falseHP=tempDat.falseHP-damageLeft
                            set damageLeft=0.
                        else
                            set damageLeft=damageLeft-tempDat.falseHP
                            set tempDat.falseHP=0.
                            set destroyingDat=tempDat
                            if destroyingDat.nextShield!=0 then
                                set tempDat=destroyingDat.nextShield
                                set tempDat.prevShield=0
                                set thistype.tab[tempDat.u]=tempDat
                            else
                                set noShieldsLeft=true
                            endif
                            set destroyingDat.nextShield=0
                        endif
                    endif
                endloop
            endif
            set tU=null
        endmethod
        
        /**
		 * Checks for dead Shield instances, destroying them.
		 */ 
        private static method timeout takes nothing returns nothing
            local integer index=0
            local thistype tempDat
            local thistype otherDat
            loop
                exitwhen index>thistype.shieldIndex
                set tempDat=thistype.allShields[index]
                set tempDat.timeLeft=tempDat.timeLeft-TIMER_PERIOD
                if (tempDat.timeLeft<=0. and tempDat.useDuration) or (tempDat.shieldLeft+tempDat.falseHP)<=0. or UnitAlive(tempDat.u)==false then
                    //deallocate it, depending on the values of prevShield 
                    //and nextShield
                    if tempDat.prevShield==0 and tempDat.nextShield==0 and thistype.tab[tempDat.u]==tempDat then
                        //this was the only shield left on the unit so:
                        call thistype.tab.flush(tempDat.u)
                    elseif tempDat.prevShield==0 and tempDat.nextShield!=0 then
                        //the shield was the first in a list so we have to 
                        //make next into the new first!
                        set otherDat=tempDat.nextShield
                        set otherDat.prevShield=0
                        set thistype.tab[tempDat.u]=otherDat
                    elseif tempDat.prevShield!=0 and tempDat.nextShield==0 then
                        //the shield was the end of the list so let's do the 
                        //proper changes to the previous member.
                        set otherDat=tempDat.prevShield
                        set otherDat.nextShield=0
                    elseif tempDat.prevShield!=0 and tempDat.nextShield!=0 then
                        //this shield was somewhere in the middle of the list 
                        //so let's make the prev and next link to each other
                        set otherDat=tempDat.prevShield
                        set otherDat.nextShield=tempDat.nextShield
                        set otherDat=tempDat.nextShield
                        set otherDat.prevShield=tempDat.prevShield
                    endif
                    //let's reset the unit's hp if he has extra
                    if tempDat.falseHP>0. then
                        call SetWidgetLife(tempDat.u,GetWidgetLife(tempDat.u)-tempDat.falseHP)
                    endif
                    call DestroyEffect(tempDat.fx)
                    call tempDat.destroy()
                    set thistype.allShields[index]=thistype.allShields[thistype.shieldIndex]
                    set thistype.shieldIndex=thistype.shieldIndex-1
                    set index=index-1
                    if thistype.shieldIndex==-1 then
                        call PauseTimer(thistype.clock)
                    endif
                endif
                set index=index+1
            endloop
        endmethod
        
        /**
		 * The public method for building a Shield instance
		 */ 
        public static method add takes unit u, real amount, real duration, string fx, string fxpoint returns nothing
            local thistype tempDat=thistype.create()
            local thistype linkedDat
            local real initialLife=GetWidgetLife(u)
            local real maxLife=GetUnitState(u,UNIT_STATE_MAX_LIFE)
            local real diff=maxLife-initialLife
            set tempDat.u=u
            set tempDat.timeLeft=duration
            set tempDat.fx=AddSpecialEffectTarget(fx,u,fxpoint)
            if duration<0. then
                set tempDat.useDuration=false
            endif
            if <0. then
                set amount=100000.
                set tempDat.finiteShield=false
            endif
            //figure out how to partition the damage
            if diff>=amount then
                //the unit has plenty of missing HP, so:
                set tempDat.shieldLeft=0
                set tempDat.falseHP=amount
                call SetWidgetLife(u,initialLife+amount)
            else
                //the unit has less hp missing than the shield has value, so:
                call SetWidgetLife(u,maxLife)
                set tempDat.falseHP=diff
                set tempDat.shieldLeft=amount-diff
            endif
            //Now let's figure out if the unit already has a shield - if he 
            //does, we add this shield to the front:
            if thistype.tab.exists(u) then
                set linkedDat=thistype.tab[u]
                set tempDat.nextShield=linkedDat
                set linkedDat.prevShield=tempDat
            endif
            set thistype.tab[u]=tempDat
            //now let's add this shield to the stack so it can be properly
            //deallocated when its time runs out:
            set thistype.shieldIndex=thistype.shieldIndex+1
            set thistype.allShields[thistype.shieldIndex]=tempDat
            //if allShields was previously empty we have to jumpstart the timer:
            if thistype.shieldIndex==0 then
                call TimerStart(thistype.clock,thistype.TIMER_PERIOD,true,function thistype.timeout)
            endif
        endmethod
        
        /**
		 * Initialization method
		 */ 
        private static method onInit takes nothing returns nothing
            set thistype.tab=HandleTable.create()
            call StructuredDD.addHandler(function thistype.handler)
        endmethod
    endstruct
endlibrary

[/hidden]

Installation
  1. Make sure you have an up-to-date copy of DamageType and all of its requirements: http://www.hiveworkshop.com/forums/submissions-414/system-damagetype-structureddd-extension-228883/
  2. Copy this script in its entirety into an empty trigger (custom script) and name it Shield.
  3. (If necessary) adjust the 'CUSTOMIZABLE SECTION' to your liking.

Change Log

2014.03.04 - Improved documentation and small ambiguities.
2013.05.25 - Updated Shield to work with the new versions of StructuredDD and DamageType. Added functionality for unlimited duration and unlimited shield values.
2013.01.28 - Reduced the size of the header and included a shim to prevent thread crash if Shield is called in a module initializer function.
2013.01.25 - Initial submission to Hive: Jass Resources: Submissions

Special Thanks:

  • -Kobas- for creating a map submission template, which turned out useful for system submissions as well.
  • Vexorian for developing JassHelper. Without vJass, I almost certainly would not still be scripting for wc3. He also created the version of Table used in this script.
  • The various developers of JNGP including PitzerMike and MindworX. Integrating JassHelper and TESH is a godsend.
 
Last edited:
Level 23
Joined
Apr 16, 2012
Messages
4,041
I have saw that it doesnt allow me to shield for SPELL or NORMAL or CODE damage types, but it still requires DamageType, could it be possible to create something like addEx method which takes as argument which specific damage type we want to shield?
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
I have saw that it doesnt allow me to shield for SPELL or NORMAL or CODE damage types, but it still requires DamageType, could it be possible to create something like addEx method which takes as argument which specific damage type we want to shield?

Yes, it's certainly possible. The only reason I didn't add it is because I didn't need it and wasn't sure of anyone else would.
 
Level 16
Joined
Aug 7, 2009
Messages
1,403
I'd also add something to it: buffs. Custom buffs based on Tornado (Slow) should be added to the unit; all you'd need is an ability and buff rawcode. When the shield breaks, it'd be removed as well. (If you decide to implement this feature as well, don't forget about UnitMakeAbilityPermanent!)

But speaking of coding style: I like yours, it's really similar to mine. I also don't use spaces in function calls, I also always type out this and thistype, and although I don't really use comments (as most of the code I write is for my map - like the "Absorb" snippet of my own :p), but for a public resource, it's definitely useful.
 
So it means I could make any resource public without comments and bad spacing (like my CustomInventory) and it should be fine?

I don't see the use of it. If you can do it better, what's so bad about actually doing it? Laziness? Come on. Takes few minutes to make it more readable for mods and users. (I am one of the users who likes to understand the code I am importing)
 
No.

Nestharus makes proper documentation, uses proper spacing and variable names in his code.

If you're going to refer to anything before the time Vexorian fixed his optimizer, I'm going to dismiss it because the only reason he started making his scripts more readable is because the optimizer was fixed. :V

Nestharus' documentation is proper because it contains everything you need to know. Nothing more and nothing less.
 
Level 31
Joined
Jul 10, 2007
Messages
6,306
hm...

comparison of 2 resources, both with 2 public things (1 with field/method, the other with 2 functions) and both with settings areas

you be the judge

more complex systems with more details are thrown into tabs within the post, never into the code header

example of post with tons of extra info -> http://www.hiveworkshop.com/forums/spells-569/codeless-save-load-1-0-1-1-a-227231/?prev=mmr=6

JASS:
//*     Introduction
//* While creating a certain spell I needed a way to add a temporary shield to a unit - I searched around, and from what I see, there is none. So I've created
//* this library in hopes of addressing that problem. It's not perfect but if I may say so myself, it is quite good. This resource will allow you to revert damage
//* dealt to a unit instantly - like a shield. There are many advantages to this system including that a unit can have more than one shield attached to it, it's
//* incredibly user friendly with only two public members, and it's *very* efficient.
//*
//*     Design Explanatiom
//* This library works by attaching an object to a unit which contains a special effect, a duration, and a shield amount. With some logic, we can actually display
//* the unit's HP as being higher so that it will shield even damage that would otherwise kill it. If you attach a shield to a unit with a shield already, then
//* the shield objects will link to each other. Checking the duration on the objects is done with a stack on a periodic timer.
//*
//*     Limitations
//* - This resource requires vJass to compile.
//* - Shield requires DamageType which in turn requires StructuredDD, thus it is subject to those systems' limitations. This implies that Shield is likely 
//*   incompatible with other damage detection systems.
//* - Shield could *probably* be optimized better, and *definitely* be optimized better if features were taken away (like multiple shields on one unit)
//* - There were many important considerations when designing this system which dealt with its optimization vs its features. Some of these questions I asked
//*   myself include:
//*   > Should the system display "false" hp such that it can mitigate damage that would otherwise kill it? For example if you shield a unit with 1 hp left should
//*     the next attack kill it anyway? Thus I decided to use this 'falseHP' idea.
//*   > If we use false hp, what happens if we shield a unit with full hp? Does the remainder of the shield value go away? I decided to implement two separate
//*     values in each shield member - a falseHP value, and a shieldLeft value. This added some complexity to the logic of the handler, but I think it's worth it.
//*   > What about the passive health that the unit would otherwise be regenerating? Should we then use only a limited amount of falseHP? I decided to simply
//*     shield full hp and ignore the issue that the unit would otherwise be regenerating.
//*   > How do I handle the addition of a shield to a unit that has some list of shields? Should I linear search through the linked list so I can add the shield
//*     to the end? Should I attach it to the front? Should I use a second Table instance to keep track of the 'last' member of the list? I decided to add it to
//*     the front of the list, because I think in this case the optimization is worth it.
//*   > If a unit has multiple shields, should I split the damage up into fractions for each shield member? This would surely be the most "fair" approach since
//*     for example a unit might have a shield that lasts 1 second and blocks 500 damage, and a shield that lasts 10 minutes and block 50 damage - but I have no
//*     way of knowing which one in the list is which. However, I decided not to implement this because it would simply be too expensive a process.
//*
//*     API
//* - public static constant string DEFAULT_EFFECT: This is a string which can be used when calling Shield.add() in the case that you want uniform shield effects.
//* - public static method add takes unit u, real amount, real time, string fx returns nothing: This is the only important method for the client. let time be the
//*   duration of your shield and fx be a string that is the effect you wish to attach to the chest of the target. That's it.
//*
//*     Installation
//* 1: Make sure you have an up-to-date copy of DamageType and all of its requirements:
//*    DamageType Library: [url]http://www.hiveworkshop.com/forums/submissions-414/system-damagetype-structureddd-extension-228883/[/url]
//* 2: Copy this script in its entirety into an empty trigger (custom script) and name it Shield.
//* 3: (If necessary) adjust the 'CUSTOMIZABLE SECTION' to your liking.
//*
//*     Final Notes
//* Although I've tested this system many times, I'm still not completely sure it is perfect. Please report any bugs to the resource thread on hiveworkshop.com - 
//* there you can also find example usage of Shield, discussion, and up-to-date versions. Lastly, If you are skilled in jass or computer algorithms, I'd 
//* appreciate comments or criticism on the logic in methods 'handler' and 'timeout', as they are quite complex and bound to have some improvement.
library Shield requires DamageType

    //* I've been known to criticize the overloading of a library name with a struct, but here I am doing it - so let's try it out shall we? This struct contains
    //* what's effectively all of the code for Shield.
    struct Shield
        //** BEGIN CUSTOMIZABLE SECTION
        
        //* An effect from this string will be added to shield members whenever DEFAULT_EFFECT is passed as the argument 'fx' in add.
        public static constant string DEFAULT_EFFECT="Abilities\\Spells\\Items\\StaffOfSanctuary\\Staff_Sanctuary_Target.mdl"
        
        //* If for some reason you have it possible to shield units that don't have a 'chest', or your effect looks better somewhere else, feel free to change
        //* this.
        private static constant string FX_ATTACH_POINT="chest"
        
        //* This can be much more accurate, but honestly people probably won't see a difference. A smaller period will reduce the likelihood of a unit blocking
        //* damage with a shield that would otherwise be expired, while a higher value will improve performance. I suggest using a value between 1./30. and 1./2.
        private static constant real TIMER_PERIOD=1./10.

vs

JASS:
library IsPathBlocked /* v4.0.0.1
************************************************************************************
*
*   function IsPathBlocked takes unit obstruction returns boolean
*       -   Determines whether or not there is a path after a new unit is placed
*
*   function LoadPathingMap takes nothing returns nothing
*       -   Loads pathing for map
*       -   Call before using system
*
************************************************************************************
*
*   */uses/*
*
*       */ IsPathable /*                hiveworkshop.com/forums/submissions-414/snippet-ispathable-199131/
*       */ RegisterPlayerUnitEvent /*   hiveworkshop.com/forums/jass-resources-412/snippet-registerplayerunitevent-203338/
*       */ UnitIndexer /*               hiveworkshop.com/forums/jass-resources-412/system-unit-indexer-172090/
*       */ GetUnitCollision /*          hiveworkshop.com/forums/submissions-414/snippet-needs-work-getunitcollision-180495/
*
************************************************************************************
*
*   SETTINGS
*
*       This is how much work is done per thread when loading pathing map.
*       Tweak this value in debug mode until the pathing map crash message goes away to minimize load time.
*/
globals
    private constant integer BUFFER = 6500
endglobals
/*
************************************************************************************/

specific looks
JASS:
//*     API
//* - public static constant string DEFAULT_EFFECT: This is a string which can be used when calling Shield.add() in the case that you want uniform shield effects.
//* - public static method add takes unit u, real amount, real time, string fx returns nothing: This is the only important method for the client. let time be the
//*   duration of your shield and fx be a string that is the effect you wish to attach to the chest of the target. That's it.

vs (i think that the below should even have new lines in between functions and descriptions to further lessen what I call the wall of text effect)

JASS:
/*
*   function IsPathBlocked takes unit obstruction returns boolean
*       -   Determines whether or not there is a path after a new unit is placed
*
*   function LoadPathingMap takes nothing returns nothing
*       -   Loads pathing for map
*       -   Call before using system
*/

JASS:
    struct Shield
        //** BEGIN CUSTOMIZABLE SECTION
        
        //* An effect from this string will be added to shield members whenever DEFAULT_EFFECT is passed as the argument 'fx' in add.
        public static constant string DEFAULT_EFFECT="Abilities\\Spells\\Items\\StaffOfSanctuary\\Staff_Sanctuary_Target.mdl"
        
        //* If for some reason you have it possible to shield units that don't have a 'chest', or your effect looks better somewhere else, feel free to change
        //* this.
        private static constant string FX_ATTACH_POINT="chest"
        
        //* This can be much more accurate, but honestly people probably won't see a difference. A smaller period will reduce the likelihood of a unit blocking
        //* damage with a shield that would otherwise be expired, while a higher value will improve performance. I suggest using a value between 1./30. and 1./2.
        private static constant real TIMER_PERIOD=1./10.

vs

JASS:
/***********************************************************************************
*
*   SETTINGS
*
*       This is how much work is done per thread when loading pathing map.
*       Tweak this value in debug mode until the pathing map crash message goes away to minimize load time.
*/
globals
    private constant integer BUFFER = 6500
endglobals
/*
************************************************************************************/

that's all I have to say


Cokemonkey11 can use any style he wants. Mag pointed out the spacing because no spacing = a brutal wall of text that makes it very difficult to find anything (see API comparison). I don't see why I get to be dragged into this (@edo494). Mag made a valid comment and it would be an improvement to the readability of his documentation, should he choose to apply it ; ).



As for the actual resource
private static method onInit takes nothing returns nothing

Keep in mind that inside of a struct, that will run after module initializers, assuming that you are using Vex's jasshelper. I recommend it be moved into a module. I'll leave mag to review the rest of your code ; ).

The reasoning for the above is quite simple. The below line is inside of your add method.
if tab.exists(u) then

Inside of a module, a call to the add method will crash the thread.

A general good rule to follow is to always use module initializers for any and all resources. For general map scripts, struct initializers can be used.
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
Hey guys, sorry for my lack of replies - this was actually my birthday weekend.

I'd also add something to it: buffs. Custom buffs based on Tornado (Slow) should be added to the unit; all you'd need is an ability and buff rawcode. When the shield breaks, it'd be removed as well. (If you decide to implement this feature as well, don't forget about UnitMakeAbilityPermanent!)

Can you explain why this is necessary? Adding and removing a buff when a unit has a shield should be quite trivial.

I'd like to see better spacing :(

So it means I could make any resource public without comments and bad spacing (like my CustomInventory) and it should be fine?

I don't see the use of it. If you can do it better, what's so bad about actually doing it? Laziness? Come on. Takes few minutes to make it more readable for mods and users. (I am one of the users who likes to understand the code I am importing)

Guys I've literally never used any other spacing style in any public resource.

It has nothing to do with laziness, it has to do with coding style preferences, and the sooner you stop bashing your head against the wall and learn to read other styles, the sooner you'll be at nirvana with your understanding of other people's code.

That's not to say that all coding styles are okay all the time, and certainly you should do your best to standardize in a corporate environment (especially with many developers working on one project), but in this case complaining about local string s="hello" vs local string s = "hello" is just pedantic and distracting.

Also, I'm not trying to force you to implement an arbitrary coding style that I've chosen to work well for myself - in fact I'm open to changes to the API (which I'll get to next)

you be the judge

If you guys dislike the style of adding the 'full system notes' into the script, I'd be willing to change that. I think 'limitations' is a pretty important concept though and I wish other resources would make a point of doing the same.

As for the actual resource
private static method onInit takes nothing returns nothing

Keep in mind that inside of a struct, that will run after module initializers, assuming that you are using Vex's jasshelper. I recommend it be moved into a module. I'll leave mag to review the rest of your code ; ).

The reasoning for the above is quite simple. The below line is inside of your add method.
if tab.exists(u) then

Inside of a module, a call to the add method will crash the thread.

A general good rule to follow is to always use module initializers for any and all resources. For general map scripts, struct initializers can be used.

Thanks, I was actually unaware of this (had to read the module documentation again before replying)

Basically you're saying that if someone implements Shield and they call Shield.add() inside of a moduler initializer, it will crash because my initializer won't have yet run? Is there any reason you suggest putting this inside of a module (and hence decreasing legibility of code) instead of listing this as a limitation?

A better question might be: Do you foresee a reason why someone would actually use Shield.add() in a module initializer?

And lastly, (and this isn't just for Nestharus), has anyone taken a look at the logic in my 'handler' and 'timeout' methods? I'd like to have a second opinion if possible.

Thanks,
 
Level 16
Joined
Aug 7, 2009
Messages
1,403
Can you explain why this is necessary? Adding and removing a buff when a unit has a shield should be quite trivial.

I don't see any ways to check whether the given shield is up or not, nor an event that fires when a shield breaks - but they tend to break before they expire, in which case the buff shouldn't stay. The easiest way to make these shields have buffs attached to them is including this feature in the code itself.
 
Level 31
Joined
Jul 10, 2007
Messages
6,306
If you guys dislike the style of adding the 'full system notes' into the script, I'd be willing to change that. I think 'limitations' is a pretty important concept though and I wish other resources would make a point of doing the same.

I personally think that limitations and any other system notes belong in the thread, not in the header. Assume that they have read the thread before putting the code into their map, so they should only need the API and basic descriptions =). However, this is only my stance. As I said, it's up to you. I've been kind of pushing for a standardized documentation style (the one that was based off of purge's, some c++ stuff, and my own experimentation), but that doesn't mean that anyone has to use it ^)^. Personally, to me, it's the most readable style and allows me to quickly browse the API and use the system in a couple of seconds ;D, which is why I like it. For your style, I had to actually search for the API : (. Mag wants better spacing so that everything is easier to find. Spacing helps accentuate categories, which helps people jump to specific areas of your documentation more easily. For example, with good spacing, I would likely find your API in half the time (instead of 10 seconds, it'd take 5).

edit
A better question might be: Do you foresee a reason why someone would actually use Shield.add() in a module initializer?

Any resource that takes the approach of using a module initializer.

If you can stop a limitation from being there with no overhead and little added code, it's better to get rid of it in my opinion.
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
I don't see any ways to check whether the given shield is up or not, nor an event that fires when a shield breaks - but they tend to break before they expire, in which case the buff shouldn't stay. The easiest way to make these shields have buffs attached to them is including this feature in the code itself.

Yes that's a good idea - it would be nice to detect if a unit has a shield if you want to make some hero spells and have them interact.

Can anyone else comment on this? I want to know that more than one person wants spell/physical specific shields and buff icons before I implement it.

I personally think that limitations and any other system notes belong in the thread, not in the header. Assume that they have read the thread before putting the code into their map, so they should only need the API and basic descriptions =). However, this is only my stance. As I said, it's up to you. I've been kind of pushing for a standardized documentation style (the one that was based off of purge's, some c++ stuff, and my own experimentation), but that doesn't mean that anyone has to use it ^)^. Personally, to me, it's the most readable style and allows me to quickly browse the API and use the system in a couple of seconds ;D, which is why I like it. For your style, I had to actually search for the API : (. Mag wants better spacing so that everything is easier to find. Spacing helps accentuate categories, which helps people jump to specific areas of your documentation more easily. For example, with good spacing, I would likely find your API in half the time (instead of 10 seconds, it'd take 5).

edit


Any resource that takes the approach of using a module initializer.

If you can stop a limitation from being there with no overhead and little added code, it's better to get rid of it in my opinion.

You're probably right, I think I will clean up the script header in the next version.

But after careful consideration the module onInit thing, I think it would be a waste of my time and effort to fix this. I don't foresee any example when someone would want to apply a shield to a unit in a module initialization method - and it's such a specific example I don't think it even deserves to be included in the limitations list.

If someone else has a comment on that then my opinion may be swayed I suppose.
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
*Sigh* I guess I'm outnumbered on that then. I've updated it.

Now that that's done can we talk about some actually pertinent things?

  • Should I add support for custom buff icons and/or an option to check if a unit has a shield?
  • Should I add support for shields which block only a certain damage type?
  • Has anyone looked at the script logic for fails/optimizations?

In regards to those things, I'd be debating on the "new" api.

JASS:
local Shield s=Shield.add(unit,amount,time,fx)
call s.addBuff(rawcode)
call s.setBlockType(DamageType_?)
if Shield.unitHas(rawcode) then
...

Or maybe the typical style is preferred?

call Shield.addEx(unit,amount,time,fx,rawcode,DamageType_?)

Obviously just tentative for now, I want to know what more people think.
 
Level 16
Joined
Aug 7, 2009
Messages
1,403
You need both the ability's and the buff's rawcode; aura buffs are not removed instantly upon removing the ability, so you need to remove them too.

As for the API, I'd vote for this:

JASS:
local Shield s=Shield.add(unit,amount,time,fx)
call s.addBuff(abilityId,buffId)
call s.setBlockType(DamageType_?)
if Shield.unitHas(rawcode) then
call Shield.addEx(unit,amount,time,sfx,abilityId,buffId,DamageType_?)

Additionally, "addBuff", "setBlockType" and similar methods should return the current instance (this), so that you could do something like this:

JASS:
call Shield.add(unit,amount,time,fx).addBuff(abilityId,buffId).setBlockType(DamageType_?)

This is optional, though I've done this a couple of times in my private resources, and I like it.
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
storing shields into items, what if hero has 6 items? gf shields (Thats how I understand what you meant, I could and most likely am wrong)
I think the way it works right now is great, but the control of getting if a unit has shield and maybe the amount left or time left(that could user do with timer tho)

Great system, just lacks the little functionality
 
Level 16
Joined
Aug 7, 2009
Messages
1,403
What Nes meant is that every Shield instance would be something like a Shield type. The values like effect, duration, damage type, buff id's, and such would be attached to them, and all you'd need to do is call Shield.apply(targetUnit) to apply the shield to a unit.

Just like I said, my problem with that is that it'd require constant values (like constant shield health and such), and as a big fan of scaling spells/items, I dislike the idea.
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
Hey guys, before I start blabbering about the subject of API any more, I learned something new:

With this sequence of events:

  1. Detect damage
  2. if damage>health
  3. Set damaged unit to invulnerable
  4. Start a 0. second timer
  5. Set damaged unit to vulnerable

You can actually prevent a unit from dying.

My question is: Should I implement this into Shield? If so, should I still keep the "falseHP" feature?

Thanks,

Edit:

You can use this script to test it (proof):

JASS:
scope placeUnits initializer i
    globals
        unit u
    endglobals
    
    private function after takes nothing returns nothing
        call BJDebugMsg("hello")
        call SetUnitInvulnerable(u,false)
        call DestroyTimer(GetExpiredTimer())
    endfunction
    
    private function c takes nothing returns boolean
        if GetEventDamage()>GetUnitState(u,UNIT_STATE_LIFE) then
            call SetUnitInvulnerable(u,true)
            call TimerStart(CreateTimer(),0.,false,function after)
        endif
        return false
    endfunction
    
    private function i takes nothing returns nothing
        local trigger t=CreateTrigger()
        set u=CreateUnit(Player(1),'hfoo',0.,512.,270.)
        call TriggerRegisterUnitEvent(t,u,EVENT_UNIT_DAMAGED)
        call TriggerAddCondition(t,Condition(function c))
        call CreateUnit(Player(0),'Hmkg',0.,0.,270.)
        set t=null
    endfunction
endscope
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
didnt really understand right now what falseHP does

It displays a unit as temporarily having more HP than they actually do. Then, at the end of the shield duration, the unit's actual hp is shown.

Redefine DDS systems : O

I'm not sure I understand how this does anything for DD.
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
What Nes was trying to say that it was discovered LOOOOONG ago; look at J4L's Damage library that's been released in IDK... 2009? 2010? Something like that.

First of all, I don't claim to have discovered anything new.

Second of all, J4L's Damage library doesn't use SetUnitInvunerable at all.
 
Level 16
Joined
Aug 7, 2009
Messages
1,403
Just to keep your response style:

First of all, I didn't say you said you had discovered anything.

Seconds of all, just becaue J4L's Damage library doesn't use SetUnitInvulnerable, it doesn't mean you can't use it to save a unit from death (indeed you can, it adds craploads of HP, from which you can get to choose how much you want to let through).
 
Level 31
Joined
Jul 10, 2007
Messages
6,306
Ok, SetUnitInvulnerable isnt a viable solution. Consider a unit fighting many units at once, like a hero line wars map or a footy map. In this case, it's very possible for 2-3 attacks to occur at the same instant. If the unit is invulnerable, the latter 1-2 attacks won't make it through at all (they won't even register). While the SetUnitInvulnerable is an interesting idea, it wouldn't work :\.

If you think that the above scenario is impossible, I've had reports from people about Bribe's DDS fully healing units. This is exactly due to what I just described as Bribe assumes that two attacks can't occur in the same instant when they can.

Now, I could be wrong... if two attacks are already set to go, they may go through the invul. I'm planning on testing it.

edit
@Luorax

No, I wasn't trying to say that. By redefine DDS, I meant moving from life buff to invul. However, now that I see the issues of invul, I would no longer recommend that move.
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
If anyone is interested, I also tested it:

Press escape to calculate the difference between the footman's health and the health he "should" have.

Basically there are a few issues with this:

  • A unit can be attacked more than once in the same "frame"
  • If attacked enough times, the unit's state in the GUI will flash "Invulnerable"
  • If the unit becomes invulnerable long enough due to constant attacks, it can interrupt other units who are trying to attack continuously

There may be other issues, but you get the idea.
 

Attachments

  • 0secondtest.w3m
    18.6 KB · Views: 94
Level 14
Joined
Dec 12, 2012
Messages
1,007
Hm, this scope will mess up completly when there are many attacks on different units due to the asynchronous after-function.
Your global unit variable u can get overwritten before the timer expires, so you would have to do something like this:

JASS:
scope placeUnits initializer i
    globals
        real n=0.
        hashtable h
        unit target
    endglobals
    
    private function after takes nothing returns nothing
        local timer time = GetExpiredTimer()
        local integer id = GetHandleId(time)
        
        call SetUnitInvulnerable(LoadUnitHandle(h, id, 1), false)
        call FlushChildHashtable(h, id)
        call DestroyTimer(time)
        set time = null
    endfunction
    
    private function c takes nothing returns boolean
        local timer time = CreateTimer()
        local integer id = GetHandleId(time)
        
        set target = GetTriggerUnit()
        call DisplayTextToPlayer(GetLocalPlayer(),0.,0.,"DamageEvent")
        call SetUnitInvulnerable(target,true)
        call SetWidgetLife(target, GetWidgetLife(target) - 1.0)
        set n=n+1.
        call SaveTimerHandle(h, id, 0, time)
        call SaveUnitHandle(h, id, 1, target)
        call TimerStart(time, 0.0, false, function after)
        return false
    endfunction
    
    private function c2 takes nothing returns boolean
        call DisplayTextToPlayer(GetLocalPlayer(), 0.0, 0.0, I2S(R2I(10000.0 + n - GetWidgetLife(target))))
        return false
    endfunction
    
    private function i takes nothing returns nothing
        local trigger t = CreateTrigger()
        local trigger t2 = CreateTrigger()

        set h = InitHashtable()
        call TriggerRegisterPlayerEvent(t2, Player(0), EVENT_PLAYER_END_CINEMATIC)
        call TriggerAddCondition(t2, Condition(function c2))
        set target = CreateUnit(Player(0), 'hfoo', 0.0, 0.0, 270.0)
        call TriggerRegisterUnitEvent(t, target, EVENT_UNIT_DAMAGED)
        call TriggerAddCondition(t, Condition(function c))
        set t = null
        set t2 = null
    endfunction
endscope

But this doesn't solve all your problems, it just ensures the timer picks the right unit.

Basically there are a few issues with this:

  • A unit can be attacked more than once in the same "frame"
  • If the unit becomes invulnerable long enough due to constant attacks, it can interrupt other units who are trying to attack continuously

Yes and these two points make it basically unusable. If you don't use enemy archers but your own archers to do the testing, you will see that after every attack all archers stop attacking, because the orders get lost.

If you display a textmessage for each damage event and let 12 archers attack your target, you will recognize that you will allways get less than 12 textmessages (sometimes 10, sometimes only 2, and so on),
the timer makes it completly random.
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
Hm, this scope will mess up completly when there are many attacks on different units due to the asynchronous after-function.

Actually, I wasn't testing that functionality since such an issue is easily solvable.

Your global unit variable u can get overwritten before the timer expires

That's incorrect. u is set on initialization and then never reset. Notice this scope doesn't use any DD system.

If you display a textmessage for each damage event and let 12 archers attack your target, you will recognize that you will allways get less than 12 textmessages (sometimes 10, sometimes only 2, and so on),
the timer makes it completly random.

The same can be seen in the uploaded test map (press escape)
 
Level 14
Joined
Dec 12, 2012
Messages
1,007
Ok, you are right. For one unit that gets never changed the test scope is sufficient.

But that doesn't help unfortunatly, as the invul breaks the game behavior completly. I mean it would be better to use Invul in theory to block damage, but until the issues with it can't get solved its not usable.

I did some research on damage blocking (including the invul approach), and these are the results:

  • It breaks unit orders. I tried to save the unit order and order it right after the damage event again, but you can see and "feel" the difference
    in both unit animation and behaviour.
  • It breaks damage events. As already mentioned, attack instances between the damage event and the corresponding timer event will get lost.
    I tried it also without a timer but with the unit life change event, so that the after damage event gets fired directly after the damage applied,
    but that doesn't help at all, there is allways a (very little) time window.
  • It breaks buffs. Some buffs get removed by invul, e.g. Shadow Strike, so this ability will never cause any damage over time.
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
Yep, that's quite right. I think this invulnerable blocking method still could have a place in some systems, but for now it looks like I have no way to mitigate deathblows (since adding temporary health also has issues)

Back to the API, here's what I have it down to:

JASS:
Shield.create(health,duration)
Shield.createEx(health,duration,source,permanent,blockedType,identifier,buff,callback)
Shield.apply(shield,target)
Shield.applyEx(shield,target,source)

//vs

Shield.create(health,duration)
Shield.createEx(health,duration,source,permanent,blockedType,identifier,buff,callback)
Shield.apply(shield,target)
Shield.applyEx(shield,target,source)
Shield.add(target,health,duration)
shield.addEx(target,health,duration,source,permanent,blockedType,identifier,buff,callback)

//vs

Shield.create(health,duration)
Shield.setSource(unit)
Shield.setPermanent(boolean)
Shield.setBlockedType(Shield.BLOCKED_TYPE_SPELLS)
Shield.setIdentifier('iden') //or Shield.setIdentifier("identifier")
Shield.setBuff('B000')
Shield.setCallback(function xyz)
Shield.apply(shield,target)
Shield.applyEx(shield,target,source)
Shield.add(target,health,duration)

Looking for opinions - if people could choose one in a bit of an informal poll that would be cool.
 
Level 23
Joined
Apr 16, 2012
Messages
4,041
as I noticed createEx in the second one has all the usufulness of the third version in one function and while its pain in ass to remember arguments when there is a ton of them, it is great to make it one liner imho

for me, the second one is totaly best
 
Level 14
Joined
Dec 12, 2012
Messages
1,007
(since adding temporary health also has issues)

Which are? Basically every damage blocking system is based on this idea and I couldn't find any hard issues while developing mine.

Back to the API

I think there are many things to fix and to think about in this system before caring about API, honestly.

I tested your system and read the documentation twice, but I still don't really get what it is supposed to do, so... It should just block damage, right? Because the system behaves really strange and somehow unpredictable.

I used this line: call Shield.add(u, 10000.0, 1000.0, Shield.DEFAULT_EFFECT), so that all damage should be blocked, right? Btw. it would be cool if the shield is permanent if zero is passed as an duration argument, and make it block infinite damage if zero is passed as an amount argument.

But now, physical damage is sometimes blocked, sometimes not (especially when the unit has full hp), sometimes the damage is reduced to a very low value. Spell damage sometimes heals, sometimes does damage, sometimes is blocked - which is really strange. Is the system supposed to heal on spell damage? I couldn't find a logic in this...

Finally, if I add a shield like explained above to a unit, I would expect that it really blocks that amount of damage, but when I deal 1000 damage to a unit with 1000 max hp, it dies anyway. So this isn't really a comprehensible behaviour for the user.
 
@Cokemonkey:

I'm sorry for the lack of attention I've given to this thread.

Use the first variation:

JASS:
Shield.create(health,duration)
Shield.createEx(health,duration,source,permanent,blockedType,identifier,buff,callback)
Shield.apply(shield,target)
Shield.applyEx(shield,target,source)

setSource and setPermanent are cool too, but I'm not sure if you should complicate things.
That's up to you though.

The add method doesn't need to be in there because it's the same as the apply function, except, instead of passing in a shield, you'd be passing in the values you used to create the shield.

JASS:
globals
    // Assume we're initializing this somewhere.
    private Shield powerShield
endglobals

// Code:
local integer i = 0
loop
    exitwhen i == unitCount
    call Shield.apply(powerShield, units[i])
    set i = i + 1
endloop

This is one cool advantage to model that involves having a Shield be only an information struct :D
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
Hey Guys,

I haven't forgotten about this resource. I'm currently doing some research to understand the correctness of my algorithm.

What I've discovered is just as I expected: The shield works properly except for in the case that (unitCurrentHP+unitTotalShields)>unitMaxHP.

This implies that if a unit has 1000/1000 hp and 1000 shield, and receives 1000 damage, it will die.

There are a few options but I have a generous amount of testing left to do.

I'm uploading a test suite so you can see the results for yourself, but generally speaking, this resource is quite stable and ready for use.
 

Attachments

  • Shield Test Suite - Correctness 001.w3x
    44 KB · Views: 96

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
Sorry for the double post.

I've discussed the state of the API with Magtheridon and Nestharus pretty extensively.

What I don't want is for this system to turn into a framework that no one will use. I've chosen to add functionality to it in small blocks and take the API tears.

New functionality includes shields which have infinite duration, and/or infinite amount.

Next I'll be creating Guardian :3
 
Hello, I've read your system and would like to say some things:

This struct is only an API ; you do not allocate objects, so there is no reason not to extend array.

I personally think that using a list instead of an array for the enumeration of the shield would make more sense.

When the user add shields, you should handle all cases. For example :

JASS:
if time==-1. then
    set tempDat.useDuration=false endif
It should be changed to

JASS:
if time<0

And here you use "time" as real, whereas you use also "time" for your timer. It's a bit surprising.

Same at this part :

JASS:
if amount==-1.
    then set amount=100000.
    set tempDat.finiteShield=false
endif

This should be changed to
JASS:
amount < 0

And what if the user wants a negative shield?

Moreover its very subjective, but I dont like some of your names:
JASS:
private unit u

JASS:
private static timer time
Time should be a real.
JASS:
private static method handler takes nothing returns nothing
Handler is a name which is not really representative of what does the method. It could be used for any method.
JASS:
public static method add takes unit u, real amount, real time, string fx, string fxpoint returns nothing
Same for add. I think apply would make more sense.

It's not really necessary but..
JASS:
private static constant real TIMER_PERIOD=1./10.
^this should maybe be placed on top of library in globals, not in struct.

Also I believe more documentation for the library would be senseful.
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
This struct is only an API ; you do not allocate objects, so there is no reason not to extend array.

Incorrect, check again.

I personally think that using a list instead of an array for the enumeration of the shield would make more sense.

I use a list for when one unit has multiple instances, but for the rest of the script, a stack has better performance.

When the user add shields, you should handle all cases. For example :

JASS:
if time==-1. then
    set tempDat.useDuration=false endif
It should be changed to

JASS:
if time<0

I agree. Fixed.

And here you use "time" as real, whereas you use also "time" for your timer. It's a bit surprising.

You're right. Changed.

And what if the user wants a negative shield?
Will never be supported (doesn't even make sense)

JASS:
private static method handler takes nothing returns nothing
Handler is a name which is not really representative of what does the method. It could be used for any method.

It's a StructuredDD handler. (As documented)

Thanks for the review.
 
Level 3
Joined
Jun 25, 2011
Messages
36
Cokemonkey said:
Icemanbo said:
This struct is only an API ; you do not allocate objects, so there is no reason not to extend array.
Incorrect, check again.

Yes but shouldn't the method create be private then? Since this method should only be used by the system and not the user?
And what about the method destroy? Is it usable to stop a shield before its normal destruction (for a dispel for example)?
 

Cokemonkey11

Spell Reviewer
Level 29
Joined
May 9, 2006
Messages
3,534
Yes but shouldn't the method create be private then? Since this method should only be used by the system and not the user?
And what about the method destroy? Is it usable to stop a shield before its normal destruction (for a dispel for example)?

There are many useful things that Shield could do, but I haven't implemented them (because I haven't needed them :p)
 
Top