HomeUser Control Panel (unavailable in archive)ForumsTutorialsArt GalleryResourcesMaps

Boolexpr failing with function call.

03-09-2010, 02:00 AM#1
sPyRaLz
I'm sorry if this has been asked before.. But I've searched and cant find the answer I need.

I've got a function here which creates a dummy unit projectile when a spell is cast then uses a timer event to slide the unit. Kinda like Elunes arrow in dota..
This line of code is failing though

Collapse JASS:
        call GroupEnumUnitsInRange(g, x, y, 50, b) 

No units are selected even though they should be. I've traced the problem to boolexpr b, and thats as far as i could get.. I would like to ask for some advice on how I could fix this up so GroupEnumUnitsInRange() correctly checks for any units within range of the projectile.

Note: Theres a lot of redundant variables declared as I built this off a clone of another spell and removed alot of the unnecesary code to showcase this problem.

Collapse JASS:
//A Point/Unit Target Spell, Thus GetSpellTargetX/Y are used.
//Requires TimerUtils/GroupUtils
//Credits to . / .
scope Frost initializer Init
//===========================================================================
//=============================SETUP START===================================
//===========================================================================
    //Constants
    globals
        private constant integer SPELL_ID = 'A001'
        private constant integer PROJ_ID = 'h004'                       //id of the projectile
        private constant real PROJ_DURATION = 3.                        //Airtime of the projectile
        private constant real PROJ_COLLISION = 50.                      //Collision size of the spell
        private constant real PROJ_SPEED = 25.                          //the speed of the spell
        private constant attacktype ATT_TYPE = ATTACK_TYPE_NORMAL       //attack type of spell
        private constant damagetype DAM_TYPE = DAMAGE_TYPE_MAGIC        //damage type of spell   
        private constant integer DAM_BASE_STAT = bj_HEROSTAT_INT        //Base stat for damage calculation
    endglobals
    
    //Dynamics
    private function Area takes integer orbCharge returns real
    //orbCharge is the Charges of the AOE Orb
        return 200. + 75. * orbCharge
    endfunction

    private function Buff takes integer orbCharge returns real
    //orbCharge is the Charges of the Buff Orb
        return orbCharge * 75.
    endfunction

    private function Multicast takes integer orbCharge returns real
    //orbCharge is the Charges of the Multicast Orb
        return orbCharge * 50.
    endfunction
    
    private function Damage takes integer orbCharge, unit c returns real
    //orbCharge is the Charges of the Base Spell Item
        return I2R(GetHeroLevel(c) * GetHeroStatBJ(DAM_BASE_STAT, c, true) * orbCharge) 
    endfunction
    
//===========================================================================
//=============================SETUP END=====================================
//===========================================================================
    globals
        private group g             //Group
        private boolexpr b          //Boolexpr
        
        private unit projectile     //Projectile
        
        //Map Bounds
        private real mapXmin = GetRectMinX(bj_mapInitialPlayableArea)
        private real mapXmax = GetRectMaxX(bj_mapInitialPlayableArea)
        private real mapYmin = GetRectMinY(bj_mapInitialPlayableArea)
        private real mapYmax = GetRectMaxY(bj_mapInitialPlayableArea)  
    endglobals
 //===========================================================================       
    private struct FrostStruct
        //Spell Data
        unit caster         //Caster
        unit projectile     //Dummy Projectile
        integer level       //Spell Level
        real angle          //Cast Angle
        real time = 0       //Initialize Time
        
        timer t             //Initialize Timer    
    endstruct
//===========================================================================    
    private function Frost_Pick takes nothing returns boolean
        return IsPlayerEnemy(GetOwningPlayer(projectile), GetOwningPlayer(GetFilterUnit())) and GetWidgetLife(GetFilterUnit()) > 0.405 and not IsUnitType(GetFilterUnit(), UNIT_TYPE_FLYING)
    endfunction    
//===========================================================================  
    private function Conditions takes nothing returns boolean
    local integer i = 0
    local boolean b = false
    
    //Multiple Spells Trigger this spell, 
    //Their rawcodes are declared globally as other triggers also use them.
        loop    
            exitwhen i > 9
            if GetSpellAbilityId() == Frost[i] then
                set b = true
                set i = 9
            endif
            set i = i + 1
        endloop
        
    return b
    endfunction
//===========================================================================
    private function Frost_Move takes nothing returns nothing
    //Initialize
    local FrostStruct data = GetCSData(GetExpiredTimer())
    local integer n = 0                     //Unit Counter
    local real x = GetUnitX(data.projectile)
    local real y = GetUnitY(data.projectile)
    local unit f                            //First of Group
    
    set projectile = data.projectile
    
    //Import Player Hero Data
    //local PlayerStruct Pdata = PlayerData[GetOwningPlayer(caster)]  
    
        //Actions
        //call DisplayTimedTextToPlayer(Player(0),0,0,60,"FrostTest")
        
        if data.time < PROJ_DURATION then 
            call GroupEnumUnitsInRange(g, x, y, PROJ_COLLISION, b)  
            call SetUnitPosition(projectile, x + PROJ_SPEED * Cos(data.angle), y+ PROJ_SPEED * Sin(data.angle))
            
            set n =  CountUnitsInGroup(g)
            call DisplayTimedTextToPlayer(Player(0),0,0,60,"Count: " + I2S(n))

            if (n > 0) or ( data.time == PROJ_DURATION ) then
                call GroupEnumUnitsInRange(g, x, y, Area(1), b) 
                loop
                    set f = FirstOfGroup(g)
                    exitwhen f == null
                    call UnitDamageTarget(projectile, f, Damage(1, data.caster), true, false, ATT_TYPE, DAM_TYPE, null)
                    call DestroyEffect(AddSpecialEffect(GetAbilityEffectById(SPELL_ID, EFFECT_TYPE_MISSILE, 1), GetUnitX(f), GetUnitY(f)))
                    call GroupRemoveUnit(g, f)
                endloop
                set data.time = PROJ_DURATION
            else 
                //If not dealing damage
                set data.time = data.time + 0.05        
            endif 
            
        else
            //Remove Dummy Unit and Timer
            call UnitApplyTimedLife(projectile,'BTLF',0.2)
            call ReleaseGroup(g)
            call ReleaseTimer(data.t)
        endif
        
        //Destroy Imported Struct
        //call Pdata.destroy()  
        call data.destroy()
    endfunction
    
    private function Actions takes nothing returns nothing
        local FrostStruct data = FrostStruct.create()
        
        call DisplayTimedTextToPlayer(Player(0),0,0,60,"FrostTest")
        
        //Spell Data
        set data.caster = GetTriggerUnit()
        set data.level = GetUnitAbilityLevel(data.caster, SPELL_ID)
        set data.angle = Atan2(GetSpellTargetY()-GetUnitY(data.caster), GetSpellTargetX()-GetUnitX(data.caster))
        set data.projectile = CreateUnit(GetOwningPlayer(data.caster),PROJ_ID, GetUnitX(data.caster), GetUnitY(data.caster),0) 
        
        //Start Timer
        set data.t = NewTimer()
        call SetCSData(data.t, data)
        call TimerStart(data.t, 0.05, true, function Frost_Move)  
        
    endfunction
    
//===========================================================================
    private function Init takes nothing returns nothing
        local trigger FrostTrg = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(FrostTrg, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition(FrostTrg, Condition( function Conditions ) )
        call TriggerAddAction( FrostTrg, function Actions )

        //Setting globals
        set g = NewGroup()
        set b = Condition(function Frost_Pick)
        
        //Preload effects
        call Preload(GetAbilityEffectById(SPELL_ID, EFFECT_TYPE_MISSILE, 0))
        call Preload(GetAbilityEffectById(SPELL_ID, EFFECT_TYPE_MISSILE, 1))
        call Preload(GetAbilityEffectById(SPELL_ID, EFFECT_TYPE_MISSILE, 3))
        call Preload(GetAbilityEffectById(SPELL_ID, EFFECT_TYPE_MISSILE, 4))         
        
    endfunction
endscope

Edit: Cleaned up the code.. took away all the redundant stuff.

Edit 2: Converted code to an easier to read format.. Hopefully

Edit 3: Added struct to allow multi-instancing. Any feedback is welcome =)
03-09-2010, 02:04 AM#2
DioD
local group g = null
03-09-2010, 02:21 AM#3
Earth-Fury
Collapse JASS:
    call DestroyBoolExpr(b)

Don't destroy boolexprs. Ever.

Condition() and Filter() are optimized. They will return the same boolexpr when given the same function, making destroying any boolexpr you use more than once completely pointless. (And bug-causing, as if you have more than one boolexpr stored for a function, destroying one will destroy them all, even if each was created with a seperate invocation of Condition(). Also note that Condition() and Filter() are identical.)

If fixing that doesn't solve your bug, say so and I'll give your code a thorough look-over.
03-09-2010, 02:31 AM#4
sPyRaLz
Quote:
Originally Posted by DioD
local group g = null

Heh.. It took me 10 minutes of staring and experimentation to realize that null should have been CreateGroup().

Now to figure out why the projectile is detecting collision with its caster =/

Thanks DioD!

Edit: Alright Earth-Fury, Thanks for the tip. Will get back to you with the results as soon as I have a chance to try it out.

//------------------------------------------------------------------------------------------------------------------------

Edit # 2: I attempted to comment out the DestroyBoolExpr() , The result can be seen in screenshot # 1, Projectile passes through the targets although it shouldn't. This is he exact same thing which happened when i had group g set to null.

Screenshot #2 shows what happens when I don't comment it out. Its not very clear but the collision count is detecting 2 (Hero + Projectile) and the spell is triggering prematurely

I've edited my code in the opening post to reflect the change I made pos-DioD's advice.
Attached Images
File type: pngnoDestroy.png (1.3 MB)
File type: pngdestroy.png (1.3 MB)
03-09-2010, 03:06 AM#5
TheKid
From my understanding boolexpr handles don't need to be destroyed though, in which case there is no reason to. Obviously your code is going to act differently when you go from not destroying something to destroying something; divide 2 by 1 and then change the 1 to a 0, and all of a sudden your code won't work.

It would be nice if someone could shed some more light on the subject because at an immediate glance it seemed like if you did call DestroyBoolExpr(Filter(function filt)) it would not actually destroy it. When I set Filter(function filt) as a variable and then destroyed the variable, it ceased to work.
03-09-2010, 03:16 AM#6
Kueken
Well, as far as I know, I can tell the same. Usually you either use Condition() or Filter() in the code itself, and it will create a boolexpr at first call and return the same for the next call, or you create one global boolexpr, using Filter() one time, basically doing the same, but saving one function call in the code. Basically destroying a boolexpr provides no use of any way usually.

Btw are And() and Or() similar to Filter(), or do they create new boolexprs? This would be a case, where destroying them might be useful.
03-09-2010, 03:34 AM#7
sPyRaLz
Forgive me i'm still kinda new to JASS, There are certain operations I don't really understand. The way boolexpr functions is one of them. Much of what I know about JASS spellcrafting was learned from an old tutorial by Blade.DK. Unfortunately his code made use of H2I, so I had to convert the template to use structs instead of handles to get it working.

Point is, the only reason i've been destroying boolexpr is because his original stomp spell template does so. =/ I can't really answer why as I'm not Blade.DK

So back to the problem, I have tried not destroying boolexpr as suggested. But the result hasn't changed..
What could possibly be causing my projectile to act in the manner that can be seen in the screenshot in my last post? I believe its pretty clear no units are being added to group g at all upon the call of

GetEnumUnits(). . Therefore something about it isn't working right. My suspicions still point towards boolexpr b.

Oh Mystery..
03-09-2010, 03:36 AM#8
Earth-Fury
Quote:
Originally Posted by TheKid
From my understanding boolexpr handles don't need to be destroyed though, in which case there is no reason to. Obviously your code is going to act differently when you go from not destroying something to destroying something; divide 2 by 1 and then change the 1 to a 0, and all of a sudden your code won't work.

It would be nice if someone could shed some more light on the subject because at an immediate glance it seemed like if you did call DestroyBoolExpr(Filter(function filt)) it would not actually destroy it. When I set Filter(function filt) as a variable and then destroyed the variable, it ceased to work.

Read my post again ;) All the info you ever need to know about boolexprs (Except how to actually use them) is there. Minus the fact that boolexprs which return nothing will compile, but crash mac players. (PJASS (JassHelper/NewGen's syntax checker) reports this as a compile error, mind you. But it works in-game.)

------------------

On to your code.

#1: Use a library or scope. (Read the JASS Helper Manual if you don't know what this means or entails.)

#2:
Collapse JASS:
constant function Frost_SpellId takes nothing returns integer
    return GetSpellAbilityId()
endfunction

constant function Frost_Base_SpellId takes nothing returns integer
    return 'A001'
endfunction
You can just use constant globals. (You can declare globals anywhere when using vJass.)

#3: call Preload(GetAbilityEffectById(Frost_Base_SpellId(), EFFECT_TYPE_MISSILE, 0)) Cool, but you should also preload the ability itself. (The "Ability Preload" library in the scripts section (under the Code section) can do all the work for you.)

#4:
Collapse JASS:
    call PData.destroy(PData)
    call data.destroy(data)
it's just .destroy() (No arguments) unless you use the static version, which is StructName.destroy(someVariableOrOtherStructTypeExpression)

#5: I see you're destroying groups. Don't do that. Use the group recycling mechanism in GroupUtils (again in the scripts section), or use a single global group which you reuse as needed.

#6: set gg_trg_Frost = CreateTrigger( )
Don't use world-editor generated global variables. Ever. (Barring in really, really map-specific code.)
You can declare your own globals, so do it. (Though in this case, using a local variable for the trigger is fine, considering you''ll never want to turn it off or destroy it.)

#7: local timer t = CreateTimer()
Use TimerUtils. (Again, scripts section.)

#8:
Collapse JASS:
    local unit dummy
    local unit c = GetTriggerUnit()
    local real x = GetSpellTargetX()
    local real y = GetSpellTargetY()
    local real a = GetUnitFacing(c)
    local player p = GetOwningPlayer(c)
    local boolexpr b = Condition(function Frost_Filter)
    local group g = CreateGroup()
    local group n
    local unit f
    
    local integer SpellID = Frost_SpellId()
    local integer i = GetUnitAbilityLevel(c, SpellID)
    
    //PlayerData
    local PlayerStruct PData = PlayerData[p]
    local integer TempInt = GetSpellIndex(Frost_Base_SpellId())
    local integer customValue = PData.SpellValue[TempInt]    
    local integer AoeCharge = PData.SpellAOE[TempInt]
    local integer BuffCharge = PData.SpellBuff[TempInt]
    local integer MCCharge = PData.SpellMC[TempInt]    
    local integer itemCharges = PData.SpellLVL[TempInt]    
    
    //MiscVars
    local integer heroLvl = GetHeroLevel(c)
    local integer heroStr = GetHeroStatBJ(bj_HEROSTAT_STR, c, true)
    local integer isUltimate = 0
    local integer aoeValue = 0
    local integer index = 0
    local integer count = 0

I have never seen so many local variables in my life... I don't even think grim001's thousand line physics simulation function had that many local variables.

#9: I still have NFI what's actually buggy in your code. It just melts my brain trying to read it. Hope this helps you write less brain-melting code in the future :)

Quote:
Originally Posted by Kueken
Btw are And() and Or() similar to Filter(), or do they create new boolexprs? This would be a case, where destroying them might be useful.
And() and Or() do create new boolexprs. They are also almost completely useless. (And by "almost completely useless", I mean I will bow before you if you find an actually good use for them that has no sane alternative.)

-----

Edit: Damn you for posting right before me...

Quote:
Originally Posted by sPyRaLz
Forgive me i'm still kinda new to JASS,
You are not only forgiven, but welcome to stay and learn :)

Quote:
Originally Posted by sPyRaLz
Much of what I know about JASS spellcrafting was learned from an old tutorial by Blade.DK. Unfortunately his code made use of H2I, so I had to convert the template to use structs instead of handles to get it working.

Point is, the only reason i've been destroying boolexpr is because his original stomp spell template does so. =/ I can't really answer why as I'm not Blade.DK

Well, Blade hasn't been around in forever plus a day, so we can't even yell at him for being outdated on all the stuff we've learned :<

-----------------

I have almost no life, so don't be afraid to ask any questions relating to WC3 modding. (There are no stupid questions, only people made stupid by not asking questions.)
03-09-2010, 04:04 AM#9
sPyRaLz
Whoops.. Didn't mean to melt your brain.

Its going to take awhile for me to recode it with all the Utils you recommended. =/ In the mean time, I've cleaned up the script in the opening post. Taken away all the unused functions and variables. It should be significantly easier to read now, and hopefully makes it clearer what the bug might be. I would be really grateful if you could take another look before I do a complete overhaul of my script.

I'll adapt to Utils, but i'll take some time.
03-09-2010, 04:52 AM#10
Earth-Fury
Quote:
Originally Posted by sPyRaLz
Whoops.. Didn't mean to melt your brain.

Its going to take awhile for me to recode it with all the Utils you recommended. =/ In the mean time, I've cleaned up the script in the opening post. Taken away all the unused functions and variables. It should be significantly easier to read now, and hopefully makes it clearer what the bug might be. I would be really grateful if you could take another look before I do a complete overhaul of my script.

I'll adapt to Utils, but i'll take some time.

Actualy, GroupUtils and TimerUtils are trivial to use.

NewTimer() ReleaseTimer() instead of Create/DestroyTimer().

Same for groups, but replace Timer with Group.

With TimerUtils, you also have "custom value"/"user data" for timers with:
SetTimerData(t, integer)->nothing
GetTimerData(t)->integer

I suggest you read about libraries before you get to using the ones I mentioned. (Libraries are really, really simple.)

The JASS Helper manual is both in your Jass NewGen folder, and (an online copy) is in the JASS Helper thread in the editing tools forum.

---------

On a completely unrelated note, when you're done with this specific spell, you should look at the following libraries:

Insanely useful: AutoIndex for giving units a unique ID which can be used as an array index. (Thus you can associate an arbitrary amount and type of data with any unit you want.)

ARGB which allows you to define colours in the form: 0xAARRGGBB (Hexidecimal notation (like HTML and WC3 string colours (|cAARRGGBBText Here|r) use) with alpha (opacity)) and easily work with them.

Table can be useful sometimes.

SimError for simulating error messages. (The kind that pop up when you, say, try to cast a spell on an invalid target.)

--------------

If you want a lesson on how to apply some of this stuff, search around for tutorials, or ask me in IRC (We have a Java applet client if you don't know how to work an IRC client) or in this thread.

Of course, actually writing some code is more help in understanding all this than all the walls of text in the world.
03-09-2010, 04:57 AM#11
Fledermaus
Collapse JASS:
function Frost_Filter takes nothing returns boolean
    return IsPlayerEnemy(GetOwningPlayer(GetTriggerUnit()), GetOwningPlayer(GetFilterUnit())) and GetWidgetLife(GetFilterUnit()) > 0.405 and not IsUnitType(GetFilterUnit(), UNIT_TYPE_FLYING)
endfunction

It's failing because GetTriggerUnit() is null when you don't use it in an event response (you're using it in a periodic timer's callback).
03-09-2010, 05:20 AM#12
TheKid
Quote:
Originally Posted by Earth-Fury
Read my post again ;) All the info you ever need to know about boolexprs (Except how to actually use them) is there. Minus the fact that boolexprs which return nothing will compile, but crash mac players. (PJASS (JassHelper/NewGen's syntax checker) reports this as a compile error, mind you. But it works in-game.)

I just saw your post and I was like "Oh its just that Earth-Fury crack-head" so I just ignored what you wrote (apparently). Thanks for the info.

Aw, I tried to give you some rep (because you're running low) but it told me to spread the love. Obviously this site doesn't know just how much love I spread.
03-09-2010, 04:21 PM#13
Captain Griffen
Quote:
Originally Posted by Earth-Fury
Collapse JASS:
    call DestroyBoolExpr(b)

Don't destroy boolexprs. Ever.

Quoted randomly just because it's so important. Can cause bugs in a distant and largely unrelated part of the code.

Way to fail Blizzard.
03-10-2010, 01:59 PM#14
sPyRaLz
Quote:
Originally Posted by Fledermaus
Collapse JASS:
function Frost_Filter takes nothing returns boolean
    return IsPlayerEnemy(GetOwningPlayer(GetTriggerUnit()), GetOwningPlayer(GetFilterUnit())) and GetWidgetLife(GetFilterUnit()) > 0.405 and not IsUnitType(GetFilterUnit(), UNIT_TYPE_FLYING)
endfunction

It's failing because GetTriggerUnit() is null when you don't use it in an event response (you're using it in a periodic timer's callback).

Yeap.. That was the problem. As a temporary fix i created a global variable and stored my projectile into it, then called it back in place of GetTriggerUnit() Thats gona create further problems though so im gona have to figure out how to use em scopes.
03-10-2010, 09:18 PM#15
Fledermaus
Make a TempInt (or TempPlayer) global and set it to GetPlayerId(GetOwningPlayer(data.dummy)) or GetOwningPlayer(data.dummy) before you do the GroupEnum and check that in the boolexpr