HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps

What's wrong with this?

07-13-2009, 10:29 PM#1
Cheezeman
After having it explained to me that UnitDamageTarget() is instant by HINDYhat, I've decided to post my spell's code and ask for your help with it, since I'm very confused with why this doesn't work.
(by the way, this spell is a <target enemy unit> kind of spell)
When casting the spell it kills the first unit, but not more (it's supposed to kill/damage units around it aswell). Please help me, I'm very scared and alone.

Please note that I'm not a very advanced vJass coder. I'm basically using normal Jass, but with enhancements pf scopes/libraries/private, free global decleration and extending array limits. That's basically it, I don't know much more. Improvements are appreciated, but asking me to do sructs of the whole thing isn't exactly helpful
Collapse JASS:
scope spell initializer Initialze
//===========================================================================
//=============================== SETUP =====================================
//===========================================================================

    globals
        private constant integer SPELL_ID = 'A000'
        private constant integer DUMMY_ID = 'h000'
        
        private constant attacktype ATTACK_TYPE = ATTACK_TYPE_MAGIC
        private constant damagetype DAMAGE_TYPE = DAMAGE_TYPE_NORMAL
        private constant string EXPLOSION_EFFECT = "Abilities\\Spells\\Other\\Incinerate\\FireLordDeathExplode.mdl"
        private constant real INTERVAL = .5
    endglobals
        
    private constant function MainTargetDamage takes integer level returns real
        return 800.
    endfunction
    
    private constant function SecondaryTargetDamage takes integer level returns real
        return 800.
    endfunction
    
    private function ExplosionRange takes integer level returns real
        return 100.
    endfunction
    
    private function AllowedTargets takes unit target, unit caster returns boolean
        return true
    endfunction
    
//===========================================================================
//============================= END SETUP ===================================
//===== Don't mess with the code below this line, unless you really know ====
//========================== what you're doing ==============================
//===========================================================================
    globals
        private group array Group1[40800]
        private group array Group2[40800]
        private unit array Caster[40800]
        private boolean array AbortTimer[40800]
        private integer array Counter[40800]
        
        private unit TemporareCaster
        private boolexpr b
    endglobals
//=========================== Misc functions ================================
    private function Spell_IdCheck takes nothing returns boolean
        return GetSpellAbilityId() == SPELL_ID
    endfunction
    
    private function TargetsCaller takes nothing returns boolean
        return AllowedTargets( GetFilterUnit(), TemporareCaster )
    endfunction
    
    private function H2I takes handle h returns integer
        return h
        return 0
    endfunction

    private function GetUnitId takes unit u returns integer
        return H2I( u ) - 0x100000
    endfunction
    
    private function IHateBlizzard takes nothing returns boolean
        return true
    endfunction
    
    private function IsIntegerUneven takes integer i returns boolean
        //Checking if it's uneven doesn't make much sence, but I couldn't design it any other way
        return ModuloInteger(i, 2) != 0
    endfunction
//============================ Main script ==================================
    private function GroupActions takes nothing returns nothing
        local timer t = GetExpiredTimer()
        local unit f
        local integer caster_id = GetTimerData( t )
        local integer level = GetUnitAbilityLevel( Caster[caster_id], SPELL_ID )
        local real fX
        local real fY
        local boolean destroy_timer = true
        if IsIntegerUneven( Counter[caster_id] ) then
            set f = FirstOfGroup( Group1[caster_id] )
            set fX = GetUnitX( f )
            set fY = GetUnitY( f )
            loop
                exitwhen f == null
                call UnitDamageTarget(  Caster[caster_id], f, SecondaryTargetDamage( level ), false, true, ATTACK_TYPE, DAMAGE_TYPE, null )
                if GetWidgetLife(f) < 0.405 then
                    call GroupEnumUnitsInRange( Group2[caster_id], fX, fY, ExplosionRange( level ), null )
                    set destroy_timer = false
                endif
                call GroupRemoveUnit( Group1[caster_id], f )
                set f = FirstOfGroup( Group1[caster_id] )
                set fX = GetUnitX( f )
                set fY = GetUnitY( f )
            endloop
        else
            set f = FirstOfGroup( Group2[caster_id] )
            set fX = GetUnitX( f )
            set fY = GetUnitY( f )
            loop
                exitwhen f == null
                call UnitDamageTarget(  Caster[caster_id], f, SecondaryTargetDamage( level ), false, true, ATTACK_TYPE, DAMAGE_TYPE, null )
                if GetWidgetLife(f) < 0.405 then
                    call GroupEnumUnitsInRange( Group1[caster_id], fX, fY, ExplosionRange( level ), null )
                    set destroy_timer = false
                endif
                call GroupRemoveUnit( Group2[caster_id], f )
                set f = FirstOfGroup( Group2[caster_id] )
                set fX = GetUnitX( f )
                set fY = GetUnitY( f )
            endloop
        endif
        set Counter[caster_id] = Counter[caster_id] + 1 //Makes it even/uneven
        if destroy_timer then
            call BJDebugMsg( "DestroyTimer == true" )
            call ReleaseTimer(t)
        else
            call BJDebugMsg( "DestroyTimer == false" )
        endif
        set t = null
    endfunction

    private function Spell_Actions takes nothing returns nothing    
        local unit caster = GetTriggerUnit()
        local unit target = GetSpellTargetUnit()
        local integer caster_id = GetUnitId(caster)
        local integer level = GetUnitAbilityLevel( caster, SPELL_ID )
        local real targetX = GetUnitX( target )
        local real targetY = GetUnitY( target )
        local timer t = NewTimer()
        local real target_life = GetWidgetLife( target )
        set Group1[caster_id] = CreateGroup()
        set Group2[caster_id] = CreateGroup()
        set Counter[caster_id] = 1
        set Caster[caster_id] = caster
        
        call UnitDamageTarget(  caster, target, MainTargetDamage( level ), false, true, ATTACK_TYPE, DAMAGE_TYPE, null )
        //Checkmark: Life = 0 hp
        if GetWidgetLife(target) < 0.405 then
            call GroupEnumUnitsInRange( Group1[caster_id], targetX, targetY, ExplosionRange( level ), null )
            call SetTimerData( t, caster_id )
            call TimerStart( t, INTERVAL, true, function GroupActions )
        endif
        
        set caster = null
        set target = null
        set t = null
    endfunction
//============================= Initialze ===================================
    private function Initialze takes nothing returns nothing
        local trigger cast = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ( cast, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition( cast, Condition( function Spell_IdCheck ) )
        call TriggerAddAction( cast, function Spell_Actions )
        
        //Preload
        set b = Condition( function TargetsCaller )
        call Preload( EXPLOSION_EFFECT )
        set bj_lastCreatedUnit = CreateUnit(Player( PLAYER_NEUTRAL_PASSIVE ), DUMMY_ID, 0, 0, 0)
        call UnitAddAbility( bj_lastCreatedUnit, SPELL_ID )
        call KillUnit( bj_lastCreatedUnit )
    endfunction
endscope

Changelog:
- initalized both groups (Michael Peppers)
- exchanged event code for BJ function (Antarif)
- exchanged the trigger event looping with TriggerRegisterAnyUnitEventBJ
07-13-2009, 10:58 PM#2
Michael Peppers
Try writing set YourGroup = CreateGroup() within the first lines of your "Spell_Actions" function for every group you're using in the triggers.

If it doesn't work, localize the issue adding a lot of Debug messages, then the problem will be clearer...

Anyway, I'd write the "IsIntegerUnever" this way:
Collapse JASS:
private function IsIntegerUneven takes integer i returns boolean
    return ModuloInteger(i, 2) != 0
endfunction
It's faster :)
07-13-2009, 11:09 PM#3
Cheezeman
Tried initializing group1 and group2 in actions; still doesn't work

Oh and I should've seen that dumb function, silly me :)
I know it's faster, just missed it somehow (planned on extracing the (evil...) ModuloInteger to the function itself later, and I think that's why I never did that)
07-13-2009, 11:33 PM#4
Anitarf
What's the point of the IHateBlizzard function?

"still doesn't work" is a rather vague statement.

Try using this function to fix your code.

You should look into using GroupUtils.
07-14-2009, 12:13 AM#5
Cheezeman
Quote:
Originally Posted by Anitarf
What's the point of the IHateBlizzard function?
It's used as a dummy filter in the initializer... I've been told that TriggerRegisterAnyUnitEventBJ leaks 32 times, and that using this event is better.
Maybe you've got a better way to improve it?

Quote:
Originally Posted by Anitarf
"still doesn't work" is a rather vague statement.
Sorry for my grammar?

Quote:
Originally Posted by Anitarf
Try using this function to fix your code.
Yes I will (tomorrow)
Yeah, I admit, I've been rather lazy in my two previous threads. I've done some checkings with it, but not much.
Quote:
Originally Posted by Anitarf
You should look into using GroupUtils.
Thank you very much! I've been searching for it for a while now.
This might improve my other spell aswell...
07-14-2009, 12:17 AM#6
Michael Peppers
Found the issue, both set f = FirstOfGroup( Group2[caster_id] ) and set f = FirstOfGroup( Group1[caster_id] ) have null values, checked also to see if the groups themselves were null, and they aren't... then... I don't know what else to do :D

EDIT: In my tests (with different number of enemies) Group 1 had always 1 unit, while Group 2 had 0 units... o_O

EDIT 2: 50 range? the minimum for melee damage is 90/120... that could explain it...

EDIT 3: Ok, complete solution: Set the Enum function in the Spell_Actions trigger this way and it will work call GroupEnumUnitsInRange( Group1[caster_id], targetX, targetY, ExplosionRange( level ), null ), set the explosion range to a number higher than 50, between 160/200 should work fine...

A boolexpr correctly coded should work too... probably it ought to be filled with "IsUnitType" stuff...

Also, if you don't need boolexpr filters (as acknowledged with "IHateBlizzard" :D), set them to null (like I did in the function above) and the related functions will always fire :)

And yes... I was bored, that's why I've tested the spell to fix it xD
07-14-2009, 04:31 AM#7
darkwulfv
Quote:
I've been told that TriggerRegisterAnyUnitEventBJ leaks 32 times
Um, what? By who? I've never ever ever heard this, and if it's true I'm sure someone would've made a big announcement about it and it would've been avoided by everyone.

I don't see how this leaks at all, let alone 32 times:
Collapse JASS:
    local integer index

    set index = 0
    loop
        call TriggerRegisterPlayerUnitEvent(trig, Player(index), whichEvent, null)

        set index = index + 1
        exitwhen index == bj_MAX_PLAYER_SLOTS
    endloop

I've heard null boolexprs leak but I thought that was only in groupenum situations. But even then, that isn't 32 leaks.
07-14-2009, 02:46 PM#8
Anitarf
Quote:
Originally Posted by Cheezeman
It's used as a dummy filter in the initializer... I've been told that TriggerRegisterAnyUnitEventBJ leaks 32 times, and that using this event is better.
First of all, that makes no sense, you're not using a different event, it's the same event, you're just manually typing the code that TriggerRegisterAnyUnitEventBJ otherwise does for you.

Second, define what you mean by leak. When I tested TriggerRegisterAnyUnitEventBJ with HandleCounter, the count went up by 16 every time I called the function, which is expected behaviour given that it creates 16 events. Your code fares no better. Perhaps using a null filter does leak in some other way, but that has only been confirmed for the GroupEnum functions and if it does, it brings me to my third point:

Nobody cares. It's a function you call once in the entire game. It's not worth the ten extra lines of code you put into it.
07-15-2009, 03:08 PM#9
Cheezeman
Ok I've done some testing with BJDebugMsg() and I've got some points:
1: As you know, this is some kind of chain-reaction spell. I've got the first units around the target to die (I replaced the filter with null) correctly. However, no more units die after that, which I'm not sure why.
2: When I do the BJDebugMsg() in GroupActions, it fills the screen with messages when it should only be displayed once every 0.5 second.

Code is updated in header. Help's appreciated
07-15-2009, 04:27 PM#10
darkwulfv
A null boolexpr in a groupenum WILL leak.
07-15-2009, 04:41 PM#11
Michael Peppers
Quote:
Originally Posted by Cheezeman
1: As you know, this is some kind of chain-reaction spell. I've got the first units around the target to die (I replaced the filter with null) correctly. However, no more units die after that, which I'm not sure why.

To me, it's the explosion range... 100 is the melee attack range, so the units have to be very close to each other in order to continue the "chain"... toy around with that value a bit.

Quote:
Originally Posted by darkwulfv
A null boolexpr in a groupenum WILL leak.

As for this, well, you could create a single boolexpr used by your groupenums doing something like return IsUnitType(UNIT_TYPE_GROUND) //I don't remember what is its name, though :P
Also, now that you use the BJ, you don't need the bj_MAX_PLAYERS loop anymore
07-15-2009, 04:50 PM#12
Cheezeman
Quote:
Originally Posted by darkwulfv
A null boolexpr in a groupenum WILL leak.
Though this piece of information is educative, it doesn't really help me out.
My problem isn't leaks, but rather extreme looping of a timer.

Oh and Michael, the units I'm testing on are within melee range of each other.
07-16-2009, 03:13 PM#13
Sophismata
You need to update the initialisation trigger now that you've changed the RegisterEvent call.
Expand JASS:


Secondly, you're making a lot of function calls that are not necessary when you're using scopes and globals. But worry about that later - for now, add the following:
Expand JASS:
07-16-2009, 04:32 PM#14
Cheezeman
Initialization isn't a problem, I'll get to optimizing the code later.
And how would those messages help me? As I said, I got the GroupActions to run, once. The problem lies in the timer (I think?), not that it isn't optimized for the best.
07-16-2009, 06:23 PM#15
Sophismata
Peppers said you had null groups. Also, you're adding the trigger event multiple times - the loop was incorrect to begin with, you've used the iterator incorrectly, but now you're actually adding the triggering event several times over.


After you've added the above debug code, add this:

Expand JASS:


I do not understand what you're trying to acomplish with the two groups, however.