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!
That's pretty weird, you used the test scope with ADD_ALL_UNITS = true and didn't make another trigger for adding units? Regardless, if you didn't it would just display the same line twice. This looks more like something specific to holy light, maybe due to the allied/enemy duality involved with the spell.
Alright, this is just beautiful! I've got it to work on my map without any issues (I've haven't got any spells to test but I'll let you know how those turn out). I'm just wondering, will these events still fire if a UnitDamageTarget or similar event? and just out of curiosity, is there anyway to get the damage type of the damage dealt?
I think it's because they just call a "deal damage" function and lookup the damage using a function that determines how much needs to be done given the type.
That function would return 0, and hence, 0 damage is dealt, and it fires an event.
it is more efficient to have every unit's damage type preset into a variable and to trigger the rest of the damage, if you think about it most of the time there are several different groups of units. for example you can have 20 footmen or 10 archers there is no point getting every unit's damage type and even if you get the damage type of a group of units there is still no need to make a system which detects something as constant as damage type as for triggered damage, that can easily be detected what type it is by using integer comparisons so since there is no native to get the damage type you will have to set it manually, if there was a system it will only save you about 4-5 minutes of work
I've updated this system with a big improvement to the code including:
No more support for useless things like DEAD_UNITS_ARE_NULL. If you need that kind of optimization, you can remove dead units yourself in a separate trigger.
Improved and shortened the API substantially.
Improved code commenting with document style explanations for each function.
Fixed a massive bug in the add function.
Paved the way for a coming DamageType extension.
I am curious if anyone is interested in extended support for things like:
Deallocating handlers
Registering special buckets for units that are never removed (eg heroes)
Included "safe" damage target by code functions (will be included regardless in the DamageType extension)
Introducing priority handlers that will occur sooner than others
deallocating handlers sounds good, sometimes I want to detect something only for certain time but I still can lock the function with boolean(it will not prevent it from running tho)
priority handler hm...I dont care really but there are situations where you want something to happen before something else
the rest I dont really care, I wouldnt find use to those but maybe there are people that would
by the way, I have one question, since you mentioned it detects UnitDamageTarget, will this cause infinite loop?
JASS:
scope a initializer init
private function handler takes nothing returns nothing
call UnitDamageTarget(GetTriggerUnit(), GetEventDamageSource(), ...)
endfunction
private function init takes nothing returns nothing
call StructuredDD_ddbucket.addHandler(function handler) //or how :D(too lazy to chcek correct syntax)
endfunction
endscope
deallocating handlers sounds good, sometimes I want to detect something only for certain time but I still can lock the function with boolean(it will not prevent it from running tho)
priority handler hm...I dont care really but there are situations where you want something to happen before something else
the rest I dont really care, I wouldnt find use to those but maybe there are people that would
by the way, I have one question, since you mentioned it detects UnitDamageTarget, will this cause infinite loop?
JASS:
scope a initializer init
private function handler takes nothing returns nothing
call UnitDamageTarget(GetTriggerUnit(), GetEventDamageSource(), ...)
endfunction
private function init takes nothing returns nothing
call StructuredDD_ddbucket.addHandler(function handler) //or how :D(too lazy to chcek correct syntax)
endfunction
endscope
It could, but I'm guessing one of the two units will die before it does (you've called 'damaged' unit to deal damage to 'damage source', so the two would programmatically take some real number r of damage until one of them is dead. I'm guessing though that if r is 0, it would infinite loop.
Thanks for your thoughts on deallocation and priority handlers, I'll wait for some more responses for/against it.
Im not going to argue if you should add it or not(its up to you tho), but one real use of deallocation is when I have optional quests where I need to use stuff which uses Damage Detections(lets say the damage itself, or my GetKiller library), if I want to decide if the killer of quest-relevant unit was player x or player y and after the quest is done/failed, I no longer need to run that function
Im not going to argue if you should add it or not(its up to you tho), but one real use of deallocation is when I have optional quests where I need to use stuff which uses Damage Detections(lets say the damage itself, or my GetKiller library), if I want to decide if the killer of quest-relevant unit was player x or player y and after the quest is done/failed, I no longer need to run that function
I explained flaws of GetKillingUnit in my GetKiller library, GetKillingUnit returns unit doing killing blow(last hit), if we want to be more realistic the killer is technically the one that dealt most damage
By the way the system runs 0. damage events(dont know if its supposed to so Im letting you know even throught you didnt mention anything in documentation):
JASS:
call UnitDamageTarget(GetTriggerUnit(), GetEventDamageSource(), any number, true, false, ATTACK_TYPE_HERO, DAMAGE_TYPE_NORMAL, null)
crashes the game without error, but it crashes the game with DamageEvent too(not with 0. damage event) so its general problem with all systems
I explained flaws of GetKillingUnit in my GetKiller library, GetKillingUnit returns unit doing killing blow(last hit), if we want to be more realistic the killer is technically the one that dealt most damage
I think that's quite subject to opinion but I won't question you on that.
By the way the system runs 0. damage events(dont know if its supposed to so Im letting you know even throught you didnt mention anything in documentation):
JASS:
call UnitDamageTarget(GetTriggerUnit(), GetEventDamageSource(), any number, true, false, ATTACK_TYPE_HERO, DAMAGE_TYPE_NORMAL, null)
crashes the game without error, but it crashes the game with DamageEvent too(not with 0. damage event) so its general problem with all systems
I'm working on an update for StructuredDD which will include a removeHandler function among other things. Does anyone have any requests? API additions?
How do we feel about priority handlers? And long-term buckets (for heroes and the like)?
I've updated StructuredDD with an improved API and documentation.
I've not added support for removing handlers, removing units, priority handlers, self-optimizing cleanup periods, or long-term handlers, since I haven't received much interest in them.
According to this, UnitAlive() is significantly less computationally expensive than IsUnitType(u,UNIT_TYPE_DEAD)
Edit: The baseline code looks like this:
JASS:
scope baseline initializer i
globals
unit u
integer count=0
endglobals
private function p takes nothing returns nothing
local integer j=0
loop
exitwhen j>=count
set j=j+1
endloop
endfunction
private function c takes nothing returns boolean
set count=count+10
call DisplayTimedTextToPlayer(Player(0),0.,0.,1.,I2S(count))
return false
endfunction
private function i takes nothing returns nothing
local trigger t=CreateTrigger()
set u=CreateUnit(Player(1),'hfoo',0.,0.,0.)
call PanCameraToTimed(3960.,3960.,0.)
call SetTimeOfDayScale(0.)
call TriggerRegisterPlayerEvent(t,Player(0),EVENT_PLAYER_END_CINEMATIC)
call TriggerAddCondition(t,Condition(function c))
call TimerStart(CreateTimer(),1./10000.,true,function p)
set t=null
endfunction
endscope
Hm, interessting. I did some tests too, to check how the length of a functions name affects performance...
I took three different function names, here is the test trigger:
JASS:
globals
constant boolean LONG_FUNCTION_NAME = true
constant integer ITERATIONS_PER_PERIOD = 15000
constant real TIME_PERIOD = 0.01
endglobals
scope test initializer Init
private function aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa takes nothing returns nothing
endfunction
private function aaaaaaaaa takes nothing returns nothing
endfunction
private function callback takes nothing returns nothing
local integer i = 0
loop
exitwhen i > ITERATIONS_PER_PERIOD
static if LONG_FUNCTION_NAME then
call aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa()
else
call aaaaaaaaa()
endif
set i = i + 1
endloop
endfunction
private function Init takes nothing returns nothing
call TimerStart(CreateTimer(), TIME_PERIOD, true, function callback)
endfunction
endscope
There is a quite big difference, but you need an extrem amount of function calls to make that difference visible (I needed more than 1 million function calls per second).
We could probably extrapolate data to verify that the performance gain by using UnitAlive over IsUnitType is purely by call length.
Anyone have an easy method of doing this? I've been doing all my graphs with matplotlib, but I'm guessing numpy and matlab should be able to do this easily.
I don't know if you get the idea. The dummy trigger (where you will put all the handlers) will be global, just one. The triggers that are in the buckets, will only serve to listen to "unit takes damage" event (not to store code). When one of those (triggers in buckets) gets triggered you just simple execute all the conditions (or actions, whatever you have used to store code in the dummy trigger) of the global dummy trigger.
I don't know if you get the idea. The dummy trigger (where you will put all the handlers) will be global, just one. The triggers that are in the buckets, will only serve to listen to "unit takes damage" event (not to store code). When one of those (triggers in buckets) gets triggered you just simple execute all the conditions (or actions, whatever you have used to store code in the dummy trigger) of the global dummy trigger.
// global
trigger dummyTrigger = CreateTrigger()
// all triggers in the buckets must have this function
// to execute when their event is fired
function onDamage takes nothing returns nothing
call TriggerExecute(dummyTrigger)
endfunction
function addHandler takes code c returns nothing
call TriggerAddCondition(dummyTrigger, Condition(c))
endfunction
So, when you have to rebuild the trigger, only loop the events.
I see what you're saying now. But this will require each trigger in the buckets to call another trigger, which will in turn call the handlers. Currently the triggers in the buckets call the handlers directly.
I think this would perform worse, but the only way to know would be to test it.
You could do some tests and let me know what you find out if you like.
I see what you're saying now. But this will require each trigger in the buckets to call another trigger, which will in turn call the handlers. Currently the triggers in the buckets call the handlers directly.
I think this would perform worse, but the only way to know would be to test it.
You could do some tests and let me know what you find out if you like.
Well, I can write how I was suggesting, I will let the tests to you (I really don't know much about testing).
edit:
Sorry for the agressive modification
JASS:
//*
//* API:
//*
//* static method addHandler takes code func returns nothing
//* Registers a callback function to the generic
//* unit damage event. Example: call StructuredDD.addHandler(function h)
//*
//* static method add takes unit member returns nothing
//* Adds a unit to a bucket. If ADD_ALL_UNITS is enabled,
//* this method need not be used.
//*
library StructuredDD
globals
//* Set this to true if you want all units in your map to be
//* automatically added to StructuredDD. Otherwise you will have to
//* manually add them with StructuredDD.add(u).
private constant boolean ADD_ALL_UNITS=true
//* This is the amount of units that exist in each trigger bucket.
//* This number should be something between 5 and 30. A good starting
//* value will be an estimate of your map's average count of units,
//* divided by 10. When in doubt, just use 20.
private constant integer BUCKET_SIZE=1
//* This is how often StructuredDD will search for empty buckets. If
//* your map has units being created and dying often, a lower value
//* is better. Anything between 10 and 180 is good. When in doubt,
//* just use 60.
private constant real PER_CLEANUP_TIMEOUT=1.
endglobals
private struct Bucket
unit array members[BUCKET_SIZE]
integer size
/*
* Will be used to listen to
* the "a unit takes damage" event
*/
trigger trig
/*
* Check if the bucket is empty
*/
method isEmpty takes nothing returns boolean
local integer i = this.size
loop
/*
* Loop throw all the members
* of the bucket
*/
exitwhen i == 0
/*
* UniTypeId wil be 0 if
* the unit has been removed
*/
if (GetUnitTypeId(this.members[i]) != 0) then
return false
endif
set i = i - 1
endloop
return true
endmethod
/*
* Add a unit to the bucket
*/
method addMember takes unit member returns nothing
set this.size = this.size + 1
set this.members[this.size] = member
call TriggerRegisterUnitEvent(this.trig, member, EVENT_UNIT_DAMAGED)
endmethod
method destroy takes nothing returns nothing
set this.size = 0
call DestroyTrigger(this.trig)
call this.deallocate()
endmethod
static method create takes nothing returns thistype
local thistype this = thistype.allocate()
set this.size = 0
set this.trig = CreateTrigger()
call TriggerAddCondition(this.trig, StructuredDD.ON_DAMAGE_CODE)
return this
endmethod
endstruct
/*
* Do not instanciate it, we just
* use this for a pretty, Java-like API :3
*/
struct StructuredDD extends array
/*
* Cache
*/
readonly static unit source = null
readonly static unit target = null
readonly static real amount = 0.
readonly static boolexpr ON_DAMAGE_CODE
/*
* This trigger will save
* all the codes to run when
* the event is fired
*/
private static trigger dummyTrigger = CreateTrigger()
private static Bucket array buckets
private static Bucket activeBucket = 0
private static integer bucketsCounter = -1
/*
* Get a bucket that is not full or a new one
*/
private static method getBucket takes nothing returns Bucket
/*
* If there is no activeBucket or
* the size of it is >= BUCKET_SIZE
*/
if (activeBucket == 0 or activeBucket.size >= BUCKET_SIZE) then
set bucketsCounter = bucketsCounter + 1
set activeBucket = Bucket.create()
set buckets[bucketsCounter] = activeBucket
endif
return activeBucket
endmethod
/*
* Add a handler to the system. It will be triggered
* when the "a unit takes damage" event is fired
*/
public static method addHandler takes code func returns nothing
/*
* No need to have "return (bool)"
* from func thanks to this
*/
local boolexpr condition = Condition(func)
call TriggerAddCondition(dummyTrigger, condition)
endmethod
private static method onDamage takes nothing returns boolean
/*
* Previus values
*/
local unit prevSource = source
local unit prevTarget = target
local real prevAmount = amount
/*
* Updating cache
*/
set source = GetEventDamageSource()
set target = GetTriggerUnit()
set amount = GetEventDamage()
/*
* Run all the handles (codes)
*/
call TriggerEvaluate(dummyTrigger)
/*
* Prevent recursion problems
*/
set source = prevSource
set target = prevTarget
set amount = prevAmount
set prevSource = null
set prevTarget = null
return false
endmethod
/*
* Add a unit to the system
* If ADD_ALL_UNITS is true, there is no need to use this
*/
public static method add takes unit member returns nothing
call getBucket().addMember(member)
endmethod
/*
* Cleans up any empty bucket
*/
private static method cleanUpPeriodic takes nothing returns nothing
local integer i = bucketsCounter
local Bucket bucket
loop
/*
* Loop throw all buckets
*/
exitwhen i == 0
set bucket = buckets[i]
if (bucket != activeBucket and bucket.isEmpty()) then
call bucket.destroy()
/*
* Move the last bucket of
* buckets to the position
* of the destroyed one
*/
set buckets[i] = buckets[bucketsCounter]
set bucketsCounter = bucketsCounter - 1
/*
* ¿?
*/
if (buckets[bucketsCounter] == activeBucket) then
set activeBucket = buckets[i]
endif
/*
* We have moved the last bucket
* of buckets to a new position,
* so don't forgget to iterate it
*/
set i = i + 1
endif
set i = i - 1
endloop
endmethod
static if (ADD_ALL_UNITS) then
/*
* Auxiliary function
*/
private static method enteringUnitFilter takes nothing returns boolean
call add(GetFilterUnit())
return false
endmethod
/*
* Automatically will register all
* unit in the damage system
*/
private static method registerAllUnits takes nothing returns nothing
local region reg = CreateRegion()
local unit enumUnit
/*
* Add starting units
*/
call GroupEnumUnitsInRect(bj_lastCreatedGroup, bj_mapInitialPlayableArea, null)
loop
set enumUnit = FirstOfGroup(bj_lastCreatedGroup)
exitwhen enumUnit == null
call GroupRemoveUnit(bj_lastCreatedGroup, enumUnit)
call thistype.add(enumUnit)
endloop
/*
* Add entering units
*/
call RegionAddRect(reg, bj_mapInitialPlayableArea)
call TriggerRegisterEnterRegion(CreateTrigger(), reg, Filter(function thistype.enteringUnitFilter))
set reg = null
endmethod
endif
/*
* Initializer
*/
private static method onInit takes nothing returns nothing
set ON_DAMAGE_CODE = Condition(function thistype.onDamage)
/*
* Periodic clenup
*/
call TimerStart(CreateTimer(), PER_CLEANUP_TIMEOUT, true, function thistype.cleanUpPeriodic)
static if (ADD_ALL_UNITS) then
call registerAllUnits()
endif
endmethod
endstruct
endlibrary
A test (it's the same, just to show you that you can use cache/native):
JASS:
library Test initializer Init requires StructuredDD
private function onDamage takes nothing returns nothing
/*
* We can use the cache or
* simple the native functions
*/
//* Native
call BJDebugMsg("Source: " + GetUnitName(GetEventDamageSource()))
//* Cache
call BJDebugMsg("Target: " + GetUnitName(StructuredDD.target))
call BJDebugMsg("Amount: " + R2S(StructuredDD.amount))
endfunction
//===========================================================================
private function Init takes nothing returns nothing
/*
* Adding a handler
*/
call StructuredDD.addHandler(function onDamage)
endfunction
endlibrary
The only that I don't understand is:
JASS:
/*
* ¿?
*/
if (buckets[bucketsCounter] == activeBucket) then
set activeBucket = buckets[i]
endif
Can you explain me?
edit:
Uhmm, seems like this version is not better.
JASS:
scope stressTest initializer i
globals
private unit u
private integer kIPS=0
endglobals
private function onDamage takes nothing returns nothing
endfunction
private function p takes nothing returns nothing
local integer j=0
loop
exitwhen j>=kIPS
call UnitDamageTarget(u,u,0.,true,true,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_NORMAL,WEAPON_TYPE_WHOKNOWS)
set j=j+1
endloop
endfunction
private function c takes nothing returns boolean
set kIPS=kIPS+1
call DisplayTimedTextToPlayer(Player(0),0.,0.,1.,I2S(kIPS))
call StructuredDD.addHandler(function onDamage)
return false
endfunction
private function i takes nothing returns nothing
local trigger t=CreateTrigger()
set u=CreateUnit(Player(0),'hfoo',0.,0.,0.)
call TimerStart(CreateTimer(),1./1000.,true,function p)
call TriggerRegisterPlayerEvent(t,Player(0),EVENT_PLAYER_END_CINEMATIC)
call TriggerAddCondition(t,Condition(function c))
set t=null
endfunction
endscope
The system works by filling up one bucket at a time. If the current bucket which is *not* yet filled up was just moved to the middle of the stack, we have to update the value of the pointer which references the current bucket which is not yet filled.
That's a bit of a mouthful, but I hope it makes sense.
edit:
Uhmm, seems like this version is not better.
JASS:
scope stressTest initializer i
globals
private unit u
private integer kIPS=0
endglobals
private function onDamage takes nothing returns nothing
endfunction
private function p takes nothing returns nothing
local integer j=0
loop
exitwhen j>=kIPS
call UnitDamageTarget(u,u,0.,true,true,ATTACK_TYPE_NORMAL,DAMAGE_TYPE_NORMAL,WEAPON_TYPE_WHOKNOWS)
set j=j+1
endloop
endfunction
private function c takes nothing returns boolean
set kIPS=kIPS+1
call DisplayTimedTextToPlayer(Player(0),0.,0.,1.,I2S(kIPS))
call StructuredDD.addHandler(function onDamage)
return false
endfunction
private function i takes nothing returns nothing
local trigger t=CreateTrigger()
set u=CreateUnit(Player(0),'hfoo',0.,0.,0.)
call TimerStart(CreateTimer(),1./1000.,true,function p)
call TriggerRegisterPlayerEvent(t,Player(0),EVENT_PLAYER_END_CINEMATIC)
call TriggerAddCondition(t,Condition(function c))
set t=null
endfunction
endscope
The system works by filling up one bucket at a time. If the current bucket which is *not* yet filled up was just moved to the middle of the stack, we have to update the value of the pointer which references the current bucket which is not yet filled.
That's a bit of a mouthful, but I hope it makes sense.
Instead of "showing you lines" I prefered to just write some alternative for you, in case I had some time to spare.
When writing descriptions, especially here, you don't need to be verbose as much. Your current lib state reminds me more of a doc file instead of lib with inserted comments. In c++ I usually do:
JASS:
/**
< 1-3 lines description here > bla bla bla
@param:
@return:
*/
If you want to give us a tip in regard what something is here for, add a single line instead of block. This is nice:
JASS:
//* Our bucket struct which contains a trigger and its associated contents.
private struct bucket
And this is not:
JASS:
//* This method is for adding a handler to the system. Whenever a
//* handler is added, damage detection will immediately trigger that
//* handler. There is no way to deallocate a handler, so don't try to
//* do this dynamically (!) Support for handler deallocation is
//* feasible (please contact me)
public static method addHandler takes code func returns nothing
Whatmore, instead of:
JASS:
call myfunction1
// comment here related to myfunction2 or just algorithm step
call myfunction2
It's better to do:
JASS:
call myfunction1
//comment here related to myfunction2 or just algorithm step
call myfunction2
When explaining API, it's nice to give some breaks between configurables/methods especially if you want to write more that a couple of words. Blocks such as:
JASS:
//* boolean ADD_ALL_UNITS: If enabled, a trigger turns on which automatically
//* registers all units in the map.
//* integer BUCKET_SIZE: How many units to add to each 'bucket' - a larger
Could as well be splited to:
JASS:
* boolean ADD_ALL_UNITS
* - If enabled, a trigger turns on which automatically registers all units in the map.
*
* integer BUCKET_SIZE
Consider that this is part most users look at when reading your stuff. If those are unexperienced, scrolling down is even less possible. However, to not ruin what I've said previously: readability of system is still nice, even if just for reviewers.
Also, you have explained the meaning of each global at the top, only to repeat it down in the code - if description is good, it only needs to be written once.
In your script, I've noticed that you don't like spaces. Meaby it's just your writting-style. Spaces are good tho, especially when there are multiple operators such as ++, --, ==, =, >, < around. There were a lot of "thistypes". Since every field member in your struct is static, and considering that when working with "normal" scripting/programming language you usually don't prefix your members with anything its good to drop it.
You use bucket structure, and when "thistype" spam was present, multiple times I've noticed something as: field.field2.field3.invoke(). When such strings appear in groups, you should reduce them as much as possible -> we don't actually need to see whole road of invocation.
Remember it's my and just my opinion. Below, system code without 1-line comments above important parts (those are welcome) and with applied suggestions:
JASS:
/*****************************************************************************
*
* API:
*
* Configurables:
* --------------
*
* boolean ADD_ALL_UNITS
* - If enabled, a trigger turns on which automatically registers all units in the map.
*
* integer BUCKET_SIZE
* - How many units to add to each 'bucket' - a larger bucket will have their trigger refresh
* less frequently but will be more computationally expensive. A good starting value is about 20.
*
* real PER_CLEANUP_TIMEOUT
* - How many seconds to wait in between each scan for empty buckets. This value should be
* lower if units die often in your map. A good starting value is about 60.
*
* Methods:
* --------
*
* static method addHandler
* - Registers a callback function to the generic unit damage event.
* Example: call StructuredDD.addHandler(function h)
*
* static method add
* - Adds a unit to a bucket. If ADD_ALL_UNITS is enabled, this method need not be used.
*
*****************************************************************************/
library StructuredDD
//<< BEGIN SETTINGS SECTION
globals
private constant boolean ADD_ALL_UNITS = true
private constant integer BUCKET_SIZE = 20
private constant real PER_CLEANUP_TIMEOUT = 60.
endglobals
//>> END SETTINGS SECTION
private struct bucket
integer bucketIndex = 0
trigger trig = CreateTrigger()
unit array members[BUCKET_SIZE]
endstruct
struct StructuredDD extends array
private static boolexpr array conditions
private static bucket array bucketDB
private static integer conditionsIndex = -1
private static integer dbIndex = -1
private static integer maxDBIndex = -1
private static method getBucket takes nothing returns integer
local integer index = 0
local integer returner = -1
local bucket tempDat
if ( dbIndex != -1 and bucketDB[dbIndex].bucketIndex < BUCKET_SIZE ) then
return dbIndex
else
set maxDBIndex = maxDBIndex+1
set dbIndex = maxDBIndex
set tempDat = bucket.create()
set bucketDB[maxDBIndex] = tempDat
loop
exitwhen index > conditionsIndex
call TriggerAddCondition(tempDat.trig, conditions[index])
set index = index+1
endloop
return dbIndex
endif
return -1
endmethod
public static method addHandler takes code func returns nothing
local bucket tempDat
local integer index = 0
set conditionsIndex = conditionsIndex+1
set conditions[conditionsIndex] = Condition(func)
loop
exitwhen index > maxDBIndex
set tempDat = bucketDB[index]
call TriggerAddCondition(tempDat.trig, conditions[conditionsIndex])
set index = index+1
endloop
endmethod
public static method add takes unit member returns nothing
local bucket tempDat
local integer whichBucket = getBucket()
set tempDat = bucketDB[whichBucket]
set tempDat.bucketIndex = tempDat.bucketIndex+1
set tempDat.members[tempDat.bucketIndex] = member
call TriggerRegisterUnitEvent(tempDat.trig, member, EVENT_UNIT_DAMAGED)
endmethod
static if ADD_ALL_UNITS then
private static method autoAddC takes nothing returns boolean
call thistype.add(GetTriggerUnit())
return false
endmethod
endif
private static method bucketIsEmpty takes integer which returns boolean
local bucket tempDat = bucketDB[which]
local integer index = 0
loop
exitwhen index == BUCKET_SIZE
if GetUnitTypeId(tempDat.members[index]) != 0 then
return false
endif
set index = index+1
endloop
return true
endmethod
private static method perCleanup takes nothing returns nothing
local integer index = 0
loop
exitwhen index > maxDBIndex
if ( index != dbIndex and bucketIsEmpty(index) ) then
call DestroyTrigger(bucketDB[index].trig)
call bucketDB[index].destroy()
set bucketDB[index] = bucketDB[maxDBIndex]
set maxDBIndex = maxDBIndex-1
if ( maxDBIndex == dbIndex ) then
set dbIndex = index
endif
set index = index-1
endif
set index = index+1
endloop
endmethod
private static method onInit takes nothing returns nothing
local group grp
local region reg
local trigger autoAddUnits
local timer perCleanup
local unit FoG
static if ADD_ALL_UNITS then
set grp = CreateGroup()
call GroupEnumUnitsInRect(grp, bj_mapInitialPlayableArea, null)
loop
set FoG = FirstOfGroup(grp)
exitwhen FoG == null
call thistype.add(FoG)
call GroupRemoveUnit(grp, FoG)
endloop
set autoAddUnits = CreateTrigger()
set reg = CreateRegion()
call RegionAddRect(reg, bj_mapInitialPlayableArea)
call TriggerRegisterEnterRegion(autoAddUnits, reg, null)
call TriggerAddCondition(autoAddUnits, Condition(function thistype.autoAddC))
set autoAddUnits = null
set reg = null
endif
set perCleanup = CreateTimer()
call TimerStart(perCleanup, PER_CLEANUP_TIMEOUT, true, function thistype.perCleanup)
set perCleanup = null
endmethod
endstruct
endlibrary
When writing descriptions, especially here, you don't need to be verbose as much. Your current lib state reminds me more of a doc file instead of lib with inserted comments. In c++ I usually do:
JASS:
/**
< 1-3 lines description here > bla bla bla
@param:
@return:
*/
If you want to give us a tip in regard what something is here for, add a single line instead of block. This is nice:
JASS:
//* Our bucket struct which contains a trigger and its associated contents.
private struct bucket
And this is not:
JASS:
//* This method is for adding a handler to the system. Whenever a
//* handler is added, damage detection will immediately trigger that
//* handler. There is no way to deallocate a handler, so don't try to
//* do this dynamically (!) Support for handler deallocation is
//* feasible (please contact me)
public static method addHandler takes code func returns nothing
Whatmore, instead of:
JASS:
call myfunction1
// comment here related to myfunction2 or just algorithm step
call myfunction2
It's better to do:
JASS:
call myfunction1
//comment here related to myfunction2 or just algorithm step
call myfunction2
When explaining API, it's nice to give some breaks between configurables/methods especially if you want to write more that a couple of words. Blocks such as:
JASS:
//* boolean ADD_ALL_UNITS: If enabled, a trigger turns on which automatically
//* registers all units in the map.
//* integer BUCKET_SIZE: How many units to add to each 'bucket' - a larger
Could as well be splited to:
JASS:
* boolean ADD_ALL_UNITS
* - If enabled, a trigger turns on which automatically registers all units in the map.
*
* integer BUCKET_SIZE
Consider that this is part most users look at when reading your stuff. If those are unexperienced, scrolling down is even less possible. However, to not ruin what I've said previously: readability of system is still nice, even if just for reviewers.
Also, you have explained the meaning of each global at the top, only to repeat it down in the code - if description is good, it only needs to be written once.
In your script, I've noticed that you don't like spaces. Meaby it's just your writting-style. Spaces are good tho, especially when there are multiple operators such as ++, --, ==, =, >, < around.
There were a lot of "thistypes". Since every field member in your struct is static, and considering that when working with "normal" scripting/programming language you usually don't prefix your members with anything its good to drop it.
You use bucket structure, and when "thistype" spam was present, multiple times I've noticed something as: field.field2.field3.invoke(). When such strings appear in groups, you should reduce them as much as possible -> we don't actually need to see whole road of invocation.
Structured DD provides an interface to the EventUnitDamaged native; it does not try to correct the behavior of the native. I suggest printing GetEventDamage() , my intuition tells me you are registering some damage events with value 0.0 due to on hit effects like bash, or otherwise. Let me know how that goes.
Because Physical Damage Detection has this block of code:
JASS:
// Set damage variables
set rawAmount = GetEventDamage()
if rawAmount == 0.0 then
return
endif
set PDDS.source = GetEventDamageSource()
set PDDS.target = GetTriggerUnit()
well maybe it would be good to support it locally, but I think notice in the documetation(I went over it and there seems to be no hint to this) that it doesnt filter.
It is additional stuff, so I understand why it isnt there(implicit thought of not being there == not implemented).
This site uses cookies to help personalise content, tailor your experience and to keep you logged in if you register.
By continuing to use this site, you are consenting to our use of cookies.