[vJASS] Spell problem

[khamarr3524]

Hello everyone! I'm back from my short lived vacation to Michigan and on the way up and there I was working on a spell for a hero in my map and I'm not a spell creator so I would like some constructive feedback (what the hell is wrong with this) and suggestions for future spell creation (what did I do wrong, how can I fix it, whats the bottom line for practicing spell creation). All comments are welcome (no martians though...) so please feel free to critique what you wish. Please note that it's probably terrible, sorry.

library ArcDrain initializer init    
    globals
        private real array duration
        private real array drain
        private unit caster
        private unit target
        private constant string effectString = ""
        private constant integer SPELL = 'Arcd'
        private constant integer BUFF = 'Arcs'
    endglobals
    
    scope BuffTick
        private function FilterUnits takes nothing returns boolean
            return (GetUnitAbilityLevel(GetFilterUnit(), BUFF) > 0)
        endfunction
        
        private function BuffTick takes nothing returns nothing
            local group g = CreateGroup()
            local unit u
            local integer i
            call GroupEnumUnitsInRect(g, bj_mapInitialPlayableArea, Filter(function FilterUnits))
            loop
                set u = FirstOfGroup(g)
                exitwhen u == null
                set i = GetUnitTypeId(u)
                call GroupRemoveUnit(g, u)
                call SetUnitState(u, UNIT_STATE_MANA, (GetUnitState(u, UNIT_STATE_MANA) - drain[i]))
                set duration[i] = duration[i] - 1
                if duration[i] == 0 then
                    call UnitRemoveAbility(u, BUFF)
                endif
                set u = null
            endloop
            call DestroyGroup(g)
            set g = null
        endfunction
        function Ticker takes nothing returns nothing
            local trigger t = CreateTrigger()
            call TriggerRegisterTimerEvent(t, 1., true)
            call TriggerAddAction(t, function BuffTick)
            set t = null
        endfunction
    endscope            
    
    private struct spell extends array                
        
        private static method buffApply takes nothing returns nothing
            local unit dummy = CreateUnit(Player(12), 'dumy', GetUnitX(target), GetUnitY(target), 270.)
            call UnitAddAbility(dummy, BUFF)
            call IssueTargetOrder(dummy, "frostnova", target)
            call RemoveUnit(dummy)
            set dummy = null
        endmethod
        
       static method allocate takes nothing returns nothing
            local integer sLevel
            set caster = GetSpellAbilityUnit()
            set sLevel = GetUnitAbilityLevel(caster, SPELL)
            set target = GetSpellTargetUnit()
            set drain[GetUnitTypeId(target)] = ((sLevel * 2) + 5)
            set duration[GetUnitTypeId(target)] = ((sLevel * 2) + 5)
            call .buffApply()
        endmethod
       
    endstruct
    
    globals
        spell array Cast
    endglobals
    
    private function init takes nothing returns nothing
        local trigger t = CreateTrigger()
        local integer i = 0
        call Ticker()
        loop
            call TriggerRegisterPlayerUnitEvent(t, Player(i), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
            set i = i + 1
            exitwhen i > 11
        endloop
        call TriggerAddAction(t, function spell.allocate)
        set t = null
    endfunction
endlibrary

Edit: Before anyone says anything, I'm aware that the effect is "". This is on purpose.

 

[Starquizer]

I guess it has something to do with the method named allocate. Try different name.

 

[khamarr3524]

Updated. Still isn't working o.o It's not applying the buff.

 

[Starquizer]

There is no instance for the struct. Also buffApply is a static method how did you call it using call .buffApply !!

 

[Adiktuz]

1)you don't overwrite the allocate method... what you overwrite is the create method then add local thistype this = .allocate() as the first line and use "this" as the variable name for that instance

2)I don't see the point of making that scope inside the library

3)learn how to use structs (read the tut on the tut section and see spells which uses structs)

4)just make one globals block

Right now this isn't MUI even if it will work because caster is a single global variable, target is also a single global variable... so make sure you do number 3...

actually, I think .buffApply will work if he did number 1 because I think not adding a struct identifier before the dot turns it into this.something y default or so I read... but its very implicit and not a good practice to do so...

this spell should be reworked from scratch... seriously...

 

[khamarr3524]

There is no instance for the struct. Also buffApply is a static method how did you call it using call .buffApply !!

Explain the first one, and because it is static is acts as a function I believe.

 

[Adiktuz]

You never created a struct instance

static method create takes nothing returns thistype
    local thistype this = .allocate() //something like this
    return this //just so it won't return a compile error if you ever tried to CnP this method
endmethod

//can also be
call struct.create()

//but we prefer to overwrite the create method (first method above) for faster execution

if you don't even know how to create struct instances I seriously suggest that you read tutorials or look at vJASS spells here using structs

 

[Starquizer]

Explain the first one, and because it is static is acts as a function I believe.

Methods are always function the difference between static and non-static is that non-static don't take the integer argument this. Explain:

method testMethod takes unit u returns nothing
endmethod

will be written in the j file as

function s_structName__testMethod @takes integer this@, unit u returns nothing
endfunction

while static is the same but without the integer this.

 

[khamarr3524]

Well. I worked on this from 2 AM - 5 AM xD so I'll just scrap this and redo it.
And my vJass knowledge is limited. o.o

 

[Starquizer]

...
And my vJass knowledge is limited. o.o

Try to focus on learning and using structs and methods they are important, if you want to make spells.

 

[Adiktuz]

Methods are always function the difference between static and non-static is that non-static don't take the integer argument this

-->which leads to very different usage between the two

basically, we use non-static (or instanced methods) if we need the function to be related to its specific struct instance (like the destroy method) while we use static for general methods (like the create method)...

struct A

    method B takes nothing returns nothing
    endmethod
   
    static method C takes nothing returns nothing
    endmethod

    static method D takes nothing returns nothing
        local thistype this = .allocate()
        call this.B() //the variable this will tell method B which instance it's called for
        call this.C() //this will probably return a compile error since C is static
        call A.C()
    endmethod

endstruct

 

[khamarr3524]

See I never really used allocated structs o.o I only really used vJass once. Other than that I taught myself how to.

 

[Adiktuz]

I taught myself too... TRD forced me to learn it to approve my first spell/system here...

 

[khamarr3524]

Any specific tutorials to look at?

 

[Adiktuz]

the vJASS OOP tutorial I think is helpful... I'm not sure if there are newer tutorials though... for making spells, its better to look at existing vJASS spells out there and try to understand each part of the code

 

[khamarr3524]

I didn't understand the .allocate requirement. Thank you, I'll keep reading and see what else I can learn before I try to code this spell again!

 

[Adiktuz]

.allocate() allocates a new instance of the struct, this is automatically done by the .create() method if you don't overwrite the .create method by I suggest overwriting it to reduce function calls...

 

[Starquizer]

You are also making the struct extends array which means that the allocation and deallocation will not be done automatically. For now, use normal structs (i.e. remove extends array).