Storm the Building (my first JASS ability)

[Siefer]

Not sure if any of you guys remember me, but I used to hang around here a lot. I pretty much only used GUI though except for the occasional leak correction. But I've decided to switch to full-on JASS now. So I've been going through all the tutorials and everything trying to get the hang of it.

Here's my first JASS spell. It seems to be working correctly and everything, but I was hoping that you guys who are more experienced with it could help me out anyways.
Just because code is functional, doesn't mean it is the most efficient or leak-proof way of doing it.
Also (the main reason i'm switching to JASS) I was hoping you could tell me whether this is multi-unit instanceable, and if not, how I could make it so (I think it is...)
Finally, there is one thing that's been especially annoying me. I used the Rescue sound when this code was in GUI, but the Syntax Checker doesn't seem to like it. I'm guessing because it's a global sound variable, but I'm not exactly positive how to make and use sounds as local variables.

So please: Tell me how it is, how to make it better, etc. I mostly looking for help with JASS and coding, but if you have any comments about the spell itself, that's cool too. It's all appreciated.

function Trig_Storm_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A005' ) ) then
        return false
    endif
    return true
endfunction

function Trig_Storm_Func005001001 takes unit storm_target returns boolean
    return ( GetUnitStateSwap(UNIT_STATE_LIFE, storm_target) <= 8.00 )
endfunction

function Trig_Storm_Func005001002 takes unit storm_target returns boolean
    return ( GetUnitStateSwap(UNIT_STATE_LIFE, storm_target) >= 1.00 )
endfunction

function Trig_Storm_Func005001 takes unit storm_target returns boolean
    return GetBooleanAnd( Trig_Storm_Func005001001(storm_target), Trig_Storm_Func005001002(storm_target) )
endfunction

function alreadyOwnTarget takes unit storm_target, unit storm_caster returns boolean
    return IsUnitInGroup(storm_target, GetUnitsOfPlayerAll(GetOwningPlayer(storm_caster)))
endfunction

function Trig_Storm_Actions takes nothing returns nothing
    local unit storm_caster = GetSpellAbilityUnit()
    local unit storm_target = GetSpellTargetUnit()
    call ShowUnitHide( storm_caster )
    call SetUnitInvulnerable( storm_caster, true )
    loop
        exitwhen ( GetBooleanOr( GetBooleanOr( Trig_Storm_Func005001(storm_target), (IsUnitDeadBJ(storm_target)) ), alreadyOwnTarget(storm_target, storm_caster) ))
        call TriggerSleepAction(RMaxBJ(bj_WAIT_FOR_COND_MIN_INTERVAL, 0.50))
    endloop
    call SetUnitOwner( storm_target, GetOwningPlayer(storm_caster), true )
    call ShowUnitShow( storm_caster )
    call SetUnitInvulnerable( storm_caster, false )
    call SetUnitLifePercentBJ( storm_target, 5.00 )
    call PlaySoundBJ( gg_snd_Rescue )
    call SetUnitLifeBJ( storm_caster, ( GetUnitStateSwap(UNIT_STATE_MAX_LIFE, storm_caster) - ( 0.05 * GetUnitStateSwap(UNIT_STATE_MAX_LIFE, storm_target) ) ) )
    set storm_caster = null
    set storm_target = null
endfunction

//===========================================================================
function InitTrig_Storm_Building takes nothing returns nothing
    local trigger Storm_Building
    set Storm_Building = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( Storm_Building, EVENT_PLAYER_UNIT_SPELL_CHANNEL )
    call TriggerAddCondition( Storm_Building, Condition( function Trig_Storm_Conditions ) )
    call TriggerAddAction( Storm_Building, function Trig_Storm_Actions )
endfunction

I made this from 60-70% of converting GUI into custom text and working my through it that way, and the other 40-30% has been looking at basic tutorials, JASS manual, and the NewGen function list.



Storm!:
The unit brings the battle to the inside of the selected enemy or nuetral building, raiding its supplies and slaughtering its denizens as it goes. As it begins to take it over from the inside, the building will lose health. If the building lowers to a certain amount, then the unit will exit the building with some damage taken and the building will be yours. If the building is destroyed through some other means that is not this ability, then the unit will exit and you will not gain control of the building (considering making it so the unit dies, as the building collapses on it while it is inside).

Sorry for all the reading; thanks for the help in advance!

tl;dr : Help me make the code/spell better.



PS. The NewGenWorldEditor has been nice and all (a definite improvement), but I was wondering if people can educate me, or direct me to the place where I can be educated on, some features I'm confused on (the advantages of free global declarations, libraries and structs)
EDIT: Btw, the spell itself, if anyone wants to try it out, is just some modified damaging channeling spell. I used Drain Life.

 

[BlackRose]

function Trig_Storm_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A005' ) ) then
        return false
    endif
    return true
endfunction

function Trig_Storm_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A005'
endfunction

function Trig_Storm_Func005001001 takes unit storm_target returns boolean
    return ( GetUnitStateSwap(UNIT_STATE_LIFE, storm_target) <= 8.00 )
endfunction

function Trig_Storm_Func005001002 takes unit storm_target returns boolean
    return ( GetUnitStateSwap(UNIT_STATE_LIFE, storm_target) >= 1.00 )
endfunction

function Trig_Storm_Func005001 takes unit storm_target returns boolean
    return GetBooleanAnd( Trig_Storm_Func005001001(storm_target), Trig_Storm_Func005001002(storm_target) )
endfunction

function alreadyOwnTarget takes unit storm_target, unit storm_caster returns boolean
    return IsUnitInGroup(storm_target, GetUnitsOfPlayerAll(GetOwningPlayer(storm_caster)))
endfunction

All this can be put into an and conjuction thingy...

Basically, the code has a lot of things you can improve.... also you forgot to null the trigger in the init one.

also...

local trigger t = CreateTrigger()

not

local trigger t
set t = CreateTrigger()

    call TriggerRegisterAnyUnitEventBJ( Storm_Building, EVENT_PLAYER_UNIT_SPELL_CHANNEL )

turns into

EVENT_PLAYER_UNIT_SPELL_EFFECT

 

[Tukki]

You are using NewGen! Make global blocks instead of those old constant functions!

Example:

globals
     private constant integer AID_MY_SPELL_NAME = 'A000'
endglobals

And you should begin to scope your spells. Put the 'scope' as first line in your code, and after that the name of the trigger.
Example:

scope <yourtriggername> initializer <myinitializer>

But remember to use 'endscope' as the last line in your code, or else you'll have syntax error(s).

you forgot to null the trigger in the init one.

Triggers used througout the game does not need, or require, to be nulled as they are used all the game - not momentarily.

Another good step is to use the native functions instead of the BJ ones.
Below is a small table over some of the functions you use-

BJ-function            Native
--------------------------------------------
ShowUnitHide()        | ShowUnit(<unit>, false)
GetUnitStateSwap()    | GetUnitState(<unit>, <state>) (browse in the function browser for UNIT_STATE)
ShowUnitShow()        | ShowUnit(<unit>, true)        
SetUnitLifePercentBJ()| SetUnitState(<unit>, UNIT_STATE_LIFE, GetUnitState(UNIT_STATE_MAX_LIFE)*0.05)

As for the call PlaySoundBJ( gg_snd_Rescue ), I'm note sure if the function calls a native
or a bunch of them.

function Trig_Storm_Func005001001 takes unit storm_target returns boolean
    return ( GetUnitStateSwap(UNIT_STATE_LIFE, storm_target) <= 8.00 )
endfunction

function Trig_Storm_Func005001002 takes unit storm_target returns boolean
    return ( GetUnitStateSwap(UNIT_STATE_LIFE, storm_target) >= 1.00 )
endfunction

function Trig_Storm_Func005001 takes unit storm_target returns boolean
    return GetBooleanAnd( Trig_Storm_Func005001001(storm_target), Trig_Storm_Func005001002(storm_target) )
endfunction

These can, as BlackRose stated, be merged to something like this:

private function Conditions takes unit who returns boolean
    return (GetUnitState(who, UNIT_STATE_LIFE)>0.405) and (GetUnitState(who, UNIT_STATE_LIFE)<=8) // why 8? And I take it you are checking if the target is dead.

Then here you should change some things too

    loop
        exitwhen ( GetBooleanOr( GetBooleanOr( Trig_Storm_Func005001(storm_target), (IsUnitDeadBJ(storm_target)) ), alreadyOwnTarget(storm_target, storm_caster) ))
        call TriggerSleepAction(RMaxBJ(bj_WAIT_FOR_COND_MIN_INTERVAL, 0.50))
    endloop

    // to

    loop
       exitwhen Conditions(storm_target) or IsUnitInGroup(storm_target, GetUnitOfPlayerAll(GetOwningPlayer(storm_caster))
       // bla bla
    endloop

And finally, add some 'private' prefix to your functions and rename them to something descriptive

----------------------------------------
Figured I should post a different version.

scope Storm initializer init

    globals
        private constant integer AID_STORM = 'A005'
    endglobals

    private function Check takes unit who returns boolean
        return (GetUnitState(who, UNIT_STATE_LIFE)>=1.0) and (GetUnitState(who, UNIT_STATE_LIFE)<=8.0)
    endfunction

    private function CastCheck takes nothing returns boolean
        return GetSpellAbilityId()==AID_STORM
    endfunction

    
    private function Actions takes nothing returns nothing
        local unit caster = GetTriggerUnit() // works same as GetSpellAbilityUnit()
        local unit target = GetSpellTargetUnit()

        call ShowUnit(caster, false) // hide the caster
        call SetUnitInvulnerable(caster, true)
 
        loop
            exitwhen (Check(target) or GetWidgetLife(target)<=0.405 or GetOwningPlayer(target)==GetOwningPlayer(caster))
            call TriggerSleepAction(0) // wait lowest amount possible
        endloop
        
        if (GetOwningPlayer(caster)!=GetOwningPlayer(target)) then
            call SetUnitOwner(target)=GetOwningPlayer(caster)
        endif
        call ShowUnit(caster, true)
        call SetUnitInvulnerable(caster, false)
        call SetUnitState(target, UNIT_STATE_LIFE, GetUnitState(target, UNIT_STATE_MAX_LIFE)*0.05)
        call SetUnitState(caster, GetUnitState(caster, UNIT_STATE_MAX_LIFE)-(0.05*GetUnitState(target, UNIT_STATE_MAX_LIFE))
	// this is pretty strange; the caster will be healed pretty much if not:
        //  1: the caster has nearly full health;
        //  2: the target has a very high amount of health.

	// play sound ?
        
        
        set caster = null
        set target = null
    endfunction

    private function init takes nothing returns nothing
        local trigger t = CreateTrigger()

        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(t, Condition(function CastCheck))
        call TriggerAddAction(t, function Actions)
    endfunction
endscope

Please note that this was written in Notepad, and may contain errors

 

[Siefer]

Thanks for all the help so far guys! I appreciate it.

A couple questions, if you don't mind (obviously this is all a learning experience for me):
1) What's the difference between EVENT_PLAYER_UNIT_CHANNEL and EVENT_PLAYER_UNIT_SPELL_EFFECT?
2) call TriggerSleepAction(0) ... you can wait 0 seconds? I thought there was a minimum limit.
3) You're right, it is kinda strange. You think there's a way I can do the damage-the-unit-thing better ? (flavorfully, the unit is having battles inside the building so I wanted to represent it somehow)
4) What are global blocks and how are they better than the constant functions I was using?
5) What are scopes?
6) How do I find the native equivalent of BJ functions?

Oh, and no one's answered some of the equestions I gave in the beginning, like about the MUI thing.
I'm gonna go dissect the code you guys gave me, see if I can understand it. I'll be back if I have more to report (questions and stuff).

If anyone has more advice n' stuff, pls give!

EDIT: FIXED

 

[Naga'sShadow]

I'll try to answer what I can but keep in mind I don't have We on this computer.

1.) Not much overall, its similar to the Gui Casting unit as opposed to Triggering unit. They will accomplish the same thing but one casting referrers to triggering so save yourself the time of running an unnecessary step and just use triggering. Same idea, EVENT_PLAYER_UNIT_CHANNEL should work the same in this instance but will run faster as EVEN_PLAYER_UNIT_SPELL_EFFECT. At least that' what I've heard, the change shouldn't be noticeable.

2.)You can wait zero seconds, but not using TriggerSleepActions(). That has a minimum wait time of .02 seconds.

3.)You can set the units health to a percentage compared to the building, so if the unit breaks off the attack just before the building collapses he'd get out barely alive. Another thing, is this a hero spell? Cause you lose control of hidden units, I think, and it be useful to reselect the unit when it comes out if its a hero.

Something like

local real state
set state = GetUnitState(target, UNIT_STATE_MAX_LIFE) / GetUnitState(target, UNIT_STATE_CURRENT_LIFE)

You can use something like that to get the percentage life of the building.

4.) Anything declared in the globals block is moved to the top of the map, like a global variable. Except that these are only for scope they are in, most of the time. Its main use is ease, you can set all your variables that won't change, like damage and special effects at the top of your code then you, or anyone looking at your code won't have to go digging if they want to change something about the way the code works. Say the base spell changes, well the spell rawcode is now in big bold letters at the top, rather than buried in your condition function.

5.) Scopes are containers for holding code. If you look at the code of you map, this trigger and everything in it will be named Storm__<Name>. Biggest benefit of this is that you can name multiple functions similarly. As in all of the init functions can be named init and each still knows which spell its for. In normal Jass every function needs a unique name, so you get stuff like InitTrig_<name>, and <name>_Actions.

6.)There's a function search ability in New Gen, or an external program like JassCraft. Search for the function, especially if you don't know how it works. You'll be able to see the function and all the functions it calls. Natives don't call anything else, while most functions with the BJ ending call Natives.

You asked a question earlier about sound. Was your problem the game wasn't letting you use whatever sound you wanted? Or was it a trigger issue? If it was the first, disable your trigger before selecting it in the editor and using it as sound. The game game hates it if you declare it as gg_snd whatever before actually setting it.

If it's the latter, set it as sound and then intentionally create a save error, a typo works. Save it and you'll get an error, and the ability to read the map code. The part were sound is initialized it at the top. That will show you how blizzard sets up sound. Hope that helps.

 

[BlackRose]

1) What's the difference between EVENT_PLAYER_UNIT_CHANNEL and EVEN_PLAYER_UNIT_SPELL_EFFECT?

Press the spell, then press stop....

2) call TriggerSleepAction(0) ... you can wait 0 seconds? I thought there was a minimum limit.

Already answered.

3) You're right, it is kinda strange. You think there's a way I can do the damage the unit (flavorfully, the unit is having battles inside the building so I wanted to represent it somehow) thing better?

HUH?

4) What are global blocks and how are they better than the constant functions I was using?

It is easier to configure, that's all... (I think)... and I don't see no constant functions in your trigger.

5) What are scopes?

6) How do I find the native equivalent of BJ functions?

You can also CTRL + CLICK a BJ.... (Usually written in Red).

 

[Siefer]

Thanks guys! I'll test it later today.

You asked a question earlier about sound. Was your problem the game wasn't letting you use whatever sound you wanted? Or was it a trigger issue? If it was the first, disable your trigger before selecting it in the editor and using it as sound. The game game hates it if you declare it as gg_snd whatever before actually setting it.

If it's the latter, set it as sound and then intentionally create a save error, a typo works. Save it and you'll get an error, and the ability to read the map code. The part were sound is initialized it at the top. That will show you how blizzard sets up sound. Hope that helps.

Well, the game let me use the sound I think (I'll check again and tell you if I'm mistaken), however the syntax checker inside the trigger editor is always pointing out an error on the line where I play the sound, telling me that the variable hasn't been declared. The ability seems to work fine enough when I test it, though. I was just hoping I could get rid of the error since there is probably some good reason the syntax checker is mad at me.
Nevertheless I'm going to try both solutions and get back to you on that.


To BlackRose, sorry, I thought your choices for functions were constant, private, and global. Since I wasn't using private or globals I assumed they were constant. My research continues.
Also, I rephrased the original question number 3. Hope it helps. It's basically in response to a comment in Tukki's revised code that he gave me.

This has all been kind of a lot of information to dissect at once-figuring out the reason for all the things you guys suggested I made and trying to keep it in mind for future situations- but I appreciate it all.

 

[Tukki]

3) You're right, it is kinda strange. You think there's a way I can do the damage the unit (flavorfully, the unit is having battles inside the building so I wanted to represent it somehow) thing better?

You mean this?

        call SetUnitState(caster, GetUnitState(caster, UNIT_STATE_MAX_LIFE)-(0.05*GetUnitState(target, UNIT_STATE_MAX_LIFE))
        // this is pretty strange; the caster will be healed pretty much if not:
        //  1: the caster has nearly full health;
        //  2: the target has a very high amount of health.

You could change it to:

        call SetUnitState(caster, GetUnitState(caster, UNIT_STATE_LIFE)-(0.05*GetUnitState(target, UNIT_STATE_MAX_LIFE))
        // changed UNIT_STATE_MAX_LIFE to UNIT_STATE_LIFE:
        //  now the damage will be dealt to the storming unit's current health,
        //  instead of the new hp being based of his maximum health. This way he wont be healed.

2) call TriggerSleepAction(0) ... you can wait 0 seconds? I thought there was a minimum limit.

You can always use a timer with 0 as interval, then it will be instant. But TriggerSleepAction has a delay, and the delay gets increased when playing in multiplayer.
So by using (0) instead of some 0.2 you are assured that it's the lowest amount of time possible. Though, you can bypass this by using a variable and setting it to negative number, then passing it to the function instead of 0, but I'm not sure how small the wait will be. Don't know where I got it from, though.

Example:

local integer wait
// rest of locals...

//Bla, bla, some code
set wait = -1000000
call TriggerSleepAction(wait)

-----------------------------------------------
EDIT: Added some code as an example of the MUI version of the spell:

//========================================================================================
//                  Storm - sample code for Siefer.
//========================================================================================
//
//  This code contains some examples and comments on how to make your spell MUI.
//  
//
//  It works by storing each spell instance in a struct, and the struct in an array.
//  Then every INTERVAl seconds, a timer is run and executes the update method, which
//  checks if the spell is completed and updates array if so.
//
//  This is one of the ways I'd make the spell MUI, if I ever should code it.
//  
//  You must change the target options of your spell in the Object Edtior to make it
//  only target enemy buildings.
//
//
//  Note: This is not tested, but is at least compilable.
//========================================================================================

scope Storm initializer init

    //------------------------------------------------
    // global block, used to contain global variables
    globals    // <-- start a block
    
        private constant integer AID_STORM  = 'A005'  // rawcode of the spell
        private constant string  EID_STORM  = ""      // put your awsome effect here, use 2 \\
        private constant real    PERCENTAGE = 0.33    // % of the target's hp, dealt as damage 
                                                      // to the caster. Deals damage depending 
                                                      // on the target's hp!
        private constant real    INTERVAL   = 0.05    // timer interval, good enough i suppose
        private constant string  ATTACH     = "chest" // attachment pos of the effect
        
    endglobals // <-- end a block
    
    //----------------------------------
    // a private, and constant function!
    private constant function GetTakeOverTime takes integer level returns real
        return 30.0 - (level-1)*5.0
    endfunction // returns 30 on level 1, 25 on level 2, 20 on level 3, etc..
    
    private constant function GetStructureHP takes integer level, real current returns real
        return current*(0.10+(level-1)*0.05)
    endfunction // this will return 10% of current hp, 15%, 20% etc..
    
    //-------------------------------------
    // not constant, but private
    private function GetEnding takes unit caster, unit target returns boolean
        return GetUnitCurrentOrder(caster)==OrderId("your spells orderstring") and GetWidgetLife(target)>0.405
    endfunction // will return true if the caster is casting the spell and the target is alive
    
    //-------------------------------------------------------\\
    // if you want to submit spells, it's a good idea to state
    // where the user may edit code, and where not. Like this:
    
    //---------< SPELL START >--------------------------------\\
    //---------< NO TOUCHING >--------------------------------\\
    
    
    private keyword data // <-- keyword so that I can use the struct on functions over it
    globals // scripters often use a private globals block, to prevent some issues with users..
        private timer Timer = null  // <-- initialize this in the init function
        private data array DATAS    // array which contains all spell instances
        private integer Counter = 0 // counter, number of spell instances
    endglobals
    
    private struct data // <-- struct! good thing, not only for storing things in
        unit caster     // the storming unit
        unit target     // the targeted unit
        integer lvl     // spell level
        
        real takeTime   // how much time left until the structure is taken over
        effect effect   // love vJass, you can actually name things with "type names"
        
        static method create takes nothing returns data // creation method, must return <structname>
            local data this = data.allocate() // use <structname>.allocate to allocate a struct instance
                                              // the "this" keyword enables you to use "." as name
                                              
            set .caster = GetSpellAbilityUnit() // set the caster
            set .target = GetSpellTargetUnit()  // set the target
            set .lvl    = GetUnitAbilityLevel(.caster, AID_STORM) // set level
            call SetUnitInvulnerable(.caster, true) // make caster invulnerable
            
            set .takeTime = GetTakeOverTime(.lvl) // set time remaining until taken over
            if (EID_STORM!="") then
                set .effect = AddSpecialEffectTarget(EID_STORM, .target, ATTACH)
            endif // add special effect if EID_STORM contains anything
            
            
            set DATAS[Counter] = this // add the struct to the array
            set Counter = Counter + 1 // update Counter
            
            if (Counter==1) then
                call TimerStart(Timer, INTERVAL, true, function data.update)
            endif
            
            return this // return the struct
        endmethod
        
        static method update takes nothing returns nothing
            local integer i = Counter - 1 // -1 because we add 1 after the last struct
            local data this               // my spell struct
            
            loop
                exitwhen i<0
                set this = DATAS<i>       // retrieve struct from array
                if (GetEnding(.caster, .target)) then // conditions passed, now update
                    set .takeTime = .takeTime - INTERVAL
                    if (.takeTime&lt;=0) then // if the caster has finished the spell:
                        call SetUnitOwner(.target, GetOwningPlayer(.caster), true)
                        call SetUnitInvulnerable(.caster, false)
                        call SetUnitState(.caster, UNIT_STATE_LIFE, GetWidgetLife(.target)-(PERCENTAGE*GetWidgetLife(.target)))
                        call SetUnitState(.target, UNIT_STATE_LIFE, GetStructureHP(.lvl, GetWidgetLife(.target)))
                        call .destroy() // destroy struct instance
                    endif
                else
                    call .destroy()     // destroy struct instance
                endif
                
                if (integer(this)==0) then       // struct indexes range from 1 to 8190, 0 being non-existant
                    set Counter = Counter - 1
                    if (Counter&lt;0) then          // pause timer when no spells are running
                        call PauseTimer(Timer)
                        set Counter = 0
                    else
                        set DATAS<i> = DATAS[Counter] // else, update array
                    endif
                endif
                set i = i - 1
            endloop
        endmethod
        
        method onDestroy takes nothing returns nothing
            if (.effect!=null) then // effects must be destroyed!
                call DestroyEffect(.effect)
            endif
        endmethod
    endstruct
    
    private function Conditions takes nothing returns boolean
        if (GetSpellAbilityId()==AID_STORM) then
            call data.create() // call data.create method directly, saving one call
        endif
        return false // there is no action added, so why return true?
    endfunction
    
    private function init takes nothing returns nothing
        local trigger t = CreateTrigger()
        
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_CHANNEL)
        call TriggerAddCondition(t, Condition(function Conditions))
        
        set Timer = CreateTimer() // I hope that this does not kill the universe..
                                  // or as Vexorian stated: &quot;.. trigger the apocallypse.&quot;
    endfunction
endscope // &lt;-- end this scope</i></i>

 

[Siefer]

Thanks!

Also, another question, I'm guessing that private functions can't be accessed somehow? Are functions assumed public if I don't put private? What's the advantages of private functions?

Oh, and cause Naga'sShadow asked, no, this isn't a hero spell. I want to be able to give this to multiple small infantry units.

 

[saw792]

Within a scope or library (you know what those are, right?) there are three access levels you can give to a function/global variable/struct:

No prefix: Any function in the map script can call this function (or variable or whatever) (unless it is in a scope and you try to call it from a function that appears above it in the map script). Using no prefix in scopes (not libraries necessarily) is not advisable for that reason.

'public': Any function within the scope/library that contains the function (variable, whatever) can access this function as if it had no prefix. Outside of the declaring scope you can call the function like this:

call SCOPENAME_FUNCTIONNAME

'private': Any function within the scope/library that contains the function (variable, whatever) can access this function as if it had no prefix. Private functions/variables are not accessible outside of the declaring scope/library.

The whole point of using 'public' and 'private' is to prevent people from using your code incorrectly (calling Init functions away from map init or something), as well as to prevent errors that occur when functions have identical names ('public' and 'private' both add a prefix when parsed by JASSHelper depending on the scope/library name). Combining scopes/libraries with public/private functions helps keep your code separated from other unrelated code ('encapsulation').

 

[Siefer]

----------------------------------------
Figured I should post a different version.

scope Storm initializer init

    globals
        private constant integer AID_STORM = 'A005'
    endglobals

    private function Check takes unit who returns boolean
        return (GetUnitState(who, UNIT_STATE_LIFE)>=1.0) and (GetUnitState(who, UNIT_STATE_LIFE)<=8.0)
    endfunction

    private function CastCheck takes nothing returns boolean
        return GetSpellAbilityId()==AID_STORM
    endfunction

    
    private function Actions takes nothing returns nothing
        local unit caster = GetTriggerUnit() // works same as GetSpellAbilityUnit()
        local unit target = GetSpellTargetUnit()

        call ShowUnit(caster, false) // hide the caster
        call SetUnitInvulnerable(caster, true)
 
        loop
            exitwhen (Check(target) or GetWidgetLife(target)<=0.405 or GetOwningPlayer(target)==GetOwningPlayer(caster))
            call TriggerSleepAction(0) // wait lowest amount possible
        endloop
        
        if (GetOwningPlayer(caster)!=GetOwningPlayer(target)) then
            call SetUnitOwner(target)=GetOwningPlayer(caster)
        endif
        call ShowUnit(caster, true)
        call SetUnitInvulnerable(caster, false)
        call SetUnitState(target, UNIT_STATE_LIFE, GetUnitState(target, UNIT_STATE_MAX_LIFE)*0.05)
        call SetUnitState(caster, GetUnitState(caster, UNIT_STATE_MAX_LIFE)-(0.05*GetUnitState(target, UNIT_STATE_MAX_LIFE))
	// this is pretty strange; the caster will be healed pretty much if not:
        //  1: the caster has nearly full health;
        //  2: the target has a very high amount of health.

	// play sound ?
        
        
        set caster = null
        set target = null
    endfunction

    private function init takes nothing returns nothing
        local trigger t = CreateTrigger()

        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(t, Condition(function CastCheck))
        call TriggerAddAction(t, function Actions)
    endfunction
endscope

Please note that this was written in Notepad, and may contain errors

Hey guys! I'm back!
So i've been trying to do this version of this spell, I figured I'd move on to the MUI version after, since it looks a bit more advanced.
Yet, it's not working, no matter how much I've tried. Can anyone suggest a reason why it wouldn't work? None of the triggers will activate when I use the spell.

 

[Tukki]

Hey guys! I'm back!
So i've been trying to do this version of this spell, I figured I'd move on to the MUI version after, since it looks a bit more advanced.
Yet, it's not working, no matter how much I've tried. Can anyone suggest a reason why it wouldn't work? None of the triggers will activate when I use the spell.

Please specify what code you're using, or post your own

//========================================================================================
//                  Storm - sample code for Siefer.
//========================================================================================
//
//  This code contains some examples and comments on how to make your spell MUI.
//  
//
//  It works by storing each spell instance in a struct, and the struct in an array.
//  Then every INTERVAl seconds, a timer is run and executes the update method, which
//  checks if the spell is completed and updates array if so.
//
//  This is one of the ways I'd make the spell MUI, if I ever should code it.
//  
//  You must change the target options of your spell in the Object Edtior to make it
//  only target enemy buildings.
//
//
//  Note: This is not tested, but is at least compilable.
//========================================================================================

scope Storm initializer init

    //------------------------------------------------
    // global block, used to contain global variables
    globals    // <-- start a block
    
        private constant integer AID_STORM  = 'A005'  // rawcode of the spell
        private constant string  EID_STORM  = ""      // put your awsome effect here, use 2 \\
        private constant real    PERCENTAGE = 0.33    // % of the target's hp, dealt as damage 
                                                      // to the caster. Deals damage depending 
                                                      // on the target's hp!
        private constant real    INTERVAL   = 0.05    // timer interval, good enough i suppose
        private constant string  ATTACH     = "chest" // attachment pos of the effect
        
    endglobals // <-- end a block
    
    //----------------------------------
    // a private, and constant function!
    private constant function GetTakeOverTime takes integer level returns real
        return 30.0 - (level-1)*5.0
    endfunction // returns 30 on level 1, 25 on level 2, 20 on level 3, etc..
    
    private constant function GetStructureHP takes integer level, real current returns real
        return current*(0.10+(level-1)*0.05)
    endfunction // this will return 10% of current hp, 15%, 20% etc..
    
    //-------------------------------------
    // not constant, but private
    private function GetEnding takes unit caster, unit target returns boolean
        return GetUnitCurrentOrder(caster)==OrderId("your spells orderstring") and GetWidgetLife(target)>0.405
    endfunction // will return true if the caster is casting the spell and the target is alive
    
    //-------------------------------------------------------\\
    // if you want to submit spells, it's a good idea to state
    // where the user may edit code, and where not. Like this:
    
    //---------< SPELL START >--------------------------------\\
    //---------< NO TOUCHING >--------------------------------\\
    
    
    private keyword data // <-- keyword so that I can use the struct on functions over it
    globals // scripters often use a private globals block, to prevent some issues with users..
        private timer Timer = null  // <-- initialize this in the init function
        private data array DATAS    // array which contains all spell instances
        private integer Counter = 0 // counter, number of spell instances
    endglobals
    
    private struct data // <-- struct! good thing, not only for storing things in
        unit caster     // the storming unit
        unit target     // the targeted unit
        integer lvl     // spell level
        
        real takeTime   // how much time left until the structure is taken over
        effect effect   // love vJass, you can actually name things with "type names"
        
        static method create takes nothing returns data // creation method, must return <structname>
            local data this = data.allocate() // use <structname>.allocate to allocate a struct instance
                                              // the "this" keyword enables you to use "." as name
                                              
            set .caster = GetSpellAbilityUnit() // set the caster
            set .target = GetSpellTargetUnit()  // set the target
            set .lvl    = GetUnitAbilityLevel(.caster, AID_STORM) // set level
            call SetUnitInvulnerable(.caster, true) // make caster invulnerable
            
            set .takeTime = GetTakeOverTime(.lvl) // set time remaining until taken over
            if (EID_STORM!="") then
                set .effect = AddSpecialEffectTarget(EID_STORM, .target, ATTACH)
            endif // add special effect if EID_STORM contains anything
            
            
            set DATAS[Counter] = this // add the struct to the array
            set Counter = Counter + 1 // update Counter
            
            if (Counter==1) then
                call TimerStart(Timer, INTERVAL, true, function data.update)
            endif
            
            return this // return the struct
        endmethod
        
        static method update takes nothing returns nothing
            local integer i = Counter - 1 // -1 because we add 1 after the last struct
            local data this               // my spell struct
            
            loop
                exitwhen i<0
                set this = DATAS<i>       // retrieve struct from array
                if (GetEnding(.caster, .target)) then // conditions passed, now update
                    set .takeTime = .takeTime - INTERVAL
                    if (.takeTime&lt;=0) then // if the caster has finished the spell:
                        call SetUnitOwner(.target, GetOwningPlayer(.caster), true)
                        call SetUnitInvulnerable(.caster, false)
                        call SetUnitState(.caster, UNIT_STATE_LIFE, GetWidgetLife(.target)-(PERCENTAGE*GetWidgetLife(.target)))
                        call SetUnitState(.target, UNIT_STATE_LIFE, GetStructureHP(.lvl, GetWidgetLife(.target)))
                        call .destroy() // destroy struct instance
                    endif
                else
                    call .destroy()     // destroy struct instance
                endif
                
                if (integer(this)==0) then       // struct indexes range from 1 to 8190, 0 being non-existant
                    set Counter = Counter - 1
                    if (Counter&lt;0) then          // pause timer when no spells are running
                        call PauseTimer(Timer)
                        set Counter = 0
                    else
                        set DATAS<i> = DATAS[Counter] // else, update array
                    endif
                endif
                set i = i - 1
            endloop
        endmethod
        
        method onDestroy takes nothing returns nothing
            if (.effect!=null) then // effects must be destroyed!
                call DestroyEffect(.effect)
            endif
        endmethod
    endstruct
    
    private function Conditions takes nothing returns boolean
        if (GetSpellAbilityId()==AID_STORM) then
            call data.create() // call data.create method directly, saving one call
        endif
        return false // there is no action added, so why return true?
    endfunction
    
    private function init takes nothing returns nothing
        local trigger t = CreateTrigger()
        
        call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_CHANNEL)
        call TriggerAddCondition(t, Condition(function Conditions))
        
        set Timer = CreateTimer() // I hope that this does not kill the universe..
                                  // or as Vexorian stated: &quot;.. trigger the apocallypse.&quot;
    endfunction
endscope // &lt;-- end this scope</i></i>

If you are using this, there are some things than may be the problem;

• AID_STORM points to the wrong rawcode
• GetUnitCurrentOrder(caster)==OrderId("your spells orderstring") - orderstring not added
• The spell is not channeled

 

[Siefer]

I did specify what code I was using. Its pretty much identical to the code I posted before I started talking. I changed a few numbers that shouldn't matter, but otherwise its pretty much the same. Here it is specifically:

scope Storm initializer init

    globals
        private constant integer AID_STORM = 'A005'
    endglobals

    private function Check takes unit who returns boolean
        return (GetUnitState(who, UNIT_STATE_LIFE)>=1.0) and (GetUnitState(who, UNIT_STATE_LIFE)<=10.0)
    endfunction

    private function CastCheck takes nothing returns boolean
        return GetSpellAbilityId() == AID_STORM
    endfunction

    function Actions takes nothing returns nothing
        local unit caster = GetTriggerUnit()
        local unit target = GetTriggerUnit()
        
        call ShowUnit(caster, false)
        call SetUnitInvulnerable(caster, true)
        
        loop
            exitwhen (Check(target) or GetWidgetLife(target)<=0.405 or GetOwningPlayer(target)==GetOwningPlayer(caster))
            call TriggerSleepAction(0)
        endloop
        
        if (GetOwningPlayer(caster) != GetOwningPlayer(target)) then
            call SetUnitOwner(target, GetOwningPlayer(caster), true)
        endif
        call ShowUnit(caster, true)
        call SetUnitInvulnerable(caster, false)
        call SetUnitState(caster, UNIT_STATE_LIFE, GetUnitState(caster,UNIT_STATE_LIFE)-(0.10*GetUnitState(target, UNIT_STATE_MAX_LIFE)))
        call PlaySoundBJ( gg_snd_Rescue )
        set caster = null
        set target = null
    endfunction

//===========================================================================
    private function init takes nothing returns nothing
        local trigger t = CreateTrigger()
        
        call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_EFFECT )
        call TriggerAddCondition( t, Condition( function CastCheck ) )
        call TriggerAddAction( t, function Actions )
    endfunction
endscope

I decided I'd try to get the spell you posted after I could get this version working, since the other version seemed more complicated. Maybe I should just move on immediately to that version first? I don't understand why this isn't working.
I checked the rawcode, and the spell is channeled. Any other suggestions?
I appreciate any help you can offer.

 

[Viikuna]

You need to learn how to debug your code.

Fill it with some BJDebugMsgs and see where they stop displaying.

 

[Tukki]

Have you checked that the trigger is named Storm? Because I'm using scopes, so it should be named 'Storm' only, without the ' that is.

But as Viikuna said, you'll need to but some BJDebugMsg("") in some places where the spell may bug.

Also using above code (the one you posted) will not work due to the 'SPELL_EFFECT' event, you'll need to change it to 'SPELL_CHANNEL' to at least make it trigger.

On a side note:

'private': Any function within the scope/library that contains the function (variable, whatever) can access this function as if it had no prefix. Private functions/variables are not accessible outside of the declaring scope/library.

True, but private/public prefixes only adds a _ between <triggername><functionname>. Public uses 1 _ (mytrigger_myfunction(..)) while private uses 2 _ (mytrigger__myfunction(..)). So, as saw792 said, the prefixes are commonly used to prevent unauthorized access to your script's functions.

 

[Siefer]

Yay! I fixed it! Before I double-check, can someone tell me what the BJDebugMsg method do?


Next up, I'll try the MUI version. Thanks for the help, as always guys. This is going to be an awesome spell, I can tell, and I've learned a lot from just trying this.
I suppose sometimes the best way to learn stuff is to learn by doing. And, of course, having other experts always helps.

 

[Viikuna]

BJDebugMsg( "lol" ) just displays text message lol. People add them to their not working spells and see which messages get displayed to find out where the code fails.

Its easier than trying to only read the code and spot errors from it.