[JASS] Help at a easy vJass Spell.

[redscores]

This Spell bugs sometimes, the laser never appears and the distance (if i put it out) is always 1500. Dunno whats the problem on it.

scope Laser1

public struct Laser1
    unit caster
    player casterowner
    real X
    real Y
    real X2
    real Y2
    real destX
    real destY
    unit dummy
    real dist
    real face
    group Damaged
    unit p
    real damage
endstruct

globals
    private Laser1 array Br
    private integer Total = 0
    private timer t = CreateTimer()
    private integer SPELL_ID = 'A000' //change this to your lasers spells ID
endglobals

function Trig_Laser_1_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == SPELL_ID
endfunction

function Trig_Laser1 takes nothing returns nothing
    local Laser1 kb
    local integer i = 0
    loop
        exitwhen i >= Total
        set kb = Br[i]
        set kb.dist = kb.dist + 6
        set kb.X2 = kb.X + kb.dist * Cos(kb.face)
        set kb.Y2 = kb.Y + kb.dist * Sin(kb.face)
        call SetUnitPosition(kb.dummy, kb.X2, kb.Y2)
        set kb.Damaged = CreateGroup()
        call GroupEnumUnitsInRange(kb.Damaged, kb.X2, kb.Y2, 250.00, null)
        loop
            set kb.p = FirstOfGroup(kb.Damaged)
            exitwhen kb.p == null
            call GroupRemoveUnit(kb.Damaged, kb.p)
            if not IsUnitType(kb.p,UNIT_TYPE_MAGIC_IMMUNE) and not IsUnitType(kb.p,UNIT_TYPE_FLYING) and not IsUnitAlly(kb.p, kb.casterowner) and not (GetUnitState(kb.p, UNIT_STATE_LIFE) <= 0.405) then
                call UnitDamageTarget(kb.caster, kb.p, (kb.damage*0.03)*GetUnitAbilityLevel(kb.caster, SPELL_ID), true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_NORMAL, WEAPON_TYPE_WHOKNOWS)
            endif
        endloop
        call DestroyGroup(kb.Damaged)
        if kb.dist >= 1500 then
            set kb.caster = null
            call UnitApplyTimedLife(kb.dummy, 'BTLF', 0.02)
            set Br[i] = Br[Total - 1]            
            set Total = Total - 1
            call kb.destroy()
        endif
        set i = i + 1
    endloop
    if Total == 0 then
        call PauseTimer(t)
    endif
endfunction

function Trig_Laser_1_Init takes nothing returns nothing
    local Laser1 kb = Laser1.create()
    local location l = GetSpellTargetLoc()
    set kb.caster = GetTriggerUnit()
    set kb.casterowner = GetOwningPlayer(kb.caster)
    set kb.destX = GetLocationX(l)
    set kb.destY = GetLocationY(l)
    call RemoveLocation(l)
    set kb.X = GetUnitX(kb.caster)
    set kb.Y = GetUnitY(kb.caster)
    set kb.face = Atan2(kb.destY - kb.Y, kb.destX - kb.X)
    set kb.X2 = kb.X + 100 * Cos(kb.face)
    set kb.Y2 = kb.Y + 100 * Sin(kb.face)
    set kb.dummy = CreateUnit(kb.casterowner, 'h000', kb.X, kb.Y, kb.face*bj_RADTODEG)
    set kb.damage = GetHeroInt(kb.caster, true)*10
    call SetUnitTimeScale(kb.dummy, 1000 * 0.01)
    if Total == 0 then
        call TimerStart(t, 0.03, true, function Trig_Laser1)
    endif
    set Total = Total + 1
    set Br[Total-1] = kb
endfunction

//===========================================================================
function InitTrig_Laser1 takes nothing returns nothing
    local integer index = 0
    set gg_trg_Laser1 = CreateTrigger(  )
    loop
        call TriggerRegisterPlayerUnitEvent(gg_trg_Laser1, Player(index), EVENT_PLAYER_UNIT_SPELL_EFFECT, null)
        set index = index + 1
        exitwhen index == bj_MAX_PLAYER_SLOTS
    endloop
    call TriggerAddCondition( gg_trg_Laser1, Condition( function Trig_Laser_1_Conditions ) )
    call TriggerAddAction( gg_trg_Laser1, function Trig_Laser_1_Init )
endfunction
endscope

 

[DiscipleOfLife]

Change ur struct block to this one:

public struct Laser1
    unit caster
    player casterowner
    real X
    real Y
    real X2
    real Y2
    real destX
    real destY
    unit dummy
    real dist = 0 // fixed
    real face
    group Damaged
    unit p
    real damage
endstruct

Btw why do you do:

    set Total = Total + 1
    set Br[Total-1] = kb

instead of:

    set Br[Total] = kb
    set Total = Total + 1

?

 

[Drone]

Change ur struct block to this one:

public struct Laser1
    unit caster
    player casterowner
    real X
    real Y
    real X2
    real Y2
    real destX
    real destY
    unit dummy
    real dist = 0 // fixed
    real face
    group Damaged
    unit p
    real damage
endstruct

Btw why do you do:

    set Total = Total + 1
    set Br[Total-1] = kb

instead of:

    set Br[Total] = kb
    set Total = Total + 1

?

Either way isn't recommendable because you can end up with either an uninitialized struct being processed or removing a struct that hasn't been processed yet.

 

[redscores]

How could it be improved?

 

[PurplePoot]

Either way isn't recommendable because you can end up with either an uninitialized struct being processed or removing a struct that hasn't been processed yet.

And what makes you think that can happen?

 

[Drone]

And what makes you think that can happen?

If you would've bothered to think this through, you would know why...

 

[PurplePoot]

If you would've bothered to think this through, you would know why...

I have bothered to think this through, and I see nothing wrong with that method.

 

[Drone]

I have bothered to think this through, and I see nothing wrong with that method.

I do see a problem. Never use globals in a MUI environment if you're going to use it like this. It could very well be that it works fine. But it can also cause very rare failures (I'm not talking about it not working at all) where you can spend hours of troubleshooting what is wrong. Using it this way, you have no way to be 100% sure that the global data isn't modified when it shouldn't be, when one instance is running.

 

[PurplePoot]

I do see a problem. Never use globals in a MUI environment if you're going to use it like this. It could very well be that it works fine. But it can also cause very rare failures (I'm not talking about it not working at all) where you can spend hours of troubleshooting what is wrong. Using it this way, you have no way to be 100% sure that the global data isn't modified when it shouldn't be, when one instance is running.

Except for the fact that it's private?

And a stack works fine in an MUI environment, that's what a struct is.

EDIT: And if someone is that determined to screw with your code in their map, they could just edit it anyways.

 

[Drone]

Except for the fact that it's private?

What are you talking about? I'm not referring to attempted modifications outside this scope. That would be impossible.

And a stack works fine in an MUI environment, that's what a struct is.

EDIT: And if someone is that determined to screw with your code in their map, they could just edit it anyways.

It can fail, and that's my point. It's not a 100% safe system to use. If you're determined and 100% sure that this always works, then please prove to me how you can 100% prevent the actions trigger (within this scope!) from messing with the global Laser 1 struct array and its index, before the timer callback is done with it, by using this method?

Can you explain how a struct = stack?
The members of a struct are permanent. You can't add or remove members from a struct when it's been declared. I assume you were referring to the stack like array of structs?

 

[DiscipleOfLife]

It can fail, and that's my point. It's not a 100% safe system to use. If you're determined and 100% sure that this always works, then please prove to me how you can 100% prevent the actions trigger (within this scope!) from messing with the global Laser 1 struct array and its index, before the timer callback is done with it, by using this method?

No one has ever proven othewise nor have I ever had any problems doing stuff this way. Also Vexorian wrote a vJass spell making tutorial on wc3c that does this exact same thing. That is enough proof for me personally.

Triggers that have some specific events registered to them can fire in the middle of other scripts when those scripts do something that fires the event. For example a trigger set to fire whenever any unit enters the map, will trigger right after any CreateUnit calls and before anything else after it. The same goes for issuing orders to units in some situations. In this case however there is no way that a trigger or timer callback etc. would be run in between the set Br[Total] = kb and set Total = Total + 1 lines.

Also if what you are saying is true then structs' create and destroy methods have the problem you're talking about. You can see what they become after being compiled by declaring a struct in a map and then making a syntax error somewhere so PJASS shows the map's script compiled up to that error. From what I have understood they use a linked list to store recycled structs and that list is done using a global integer array.

 

[Drone]

No one has ever proven othewise nor have I ever had any problems doing stuff this way. Also Vexorian wrote a vJass spell making tutorial on wc3c that does this exact same thing. That is enough proof for me personally.

Triggers that have some specific events registered to them can fire in the middle of other scripts when those scripts do something that fires the event. For example a trigger set to fire whenever any unit enters the map, will trigger right after any CreateUnit calls and before anything else after it. The same goes for issuing orders to units in some situations. In this case however there is no way that a trigger or timer callback etc. would be run in between the set Br[Total] = kb and set Total = Total + 1 lines.

Also if what you are saying is true then structs' create and destroy methods have the problem you're talking about. You can see what they become after being compiled by declaring a struct in a map and then making a syntax error somewhere so PJASS shows the map's script compiled up to that error. From what I have understood they use a linked list to store recycled structs and that list is done using a global integer array.

I assume you're referring to http://wc3campaigns.net/showthread.php?t=91491

It isn't as clear cut that it should fail by default. I'm not talking about easy to spot failures in terms of crashes, but simply missed instances, where the effect isn't properly shown, or trying to process an uninitialized instance (depends on the order of modifying the global data). I don't see Vexorian mentioning anything there about different instances and thread safety. So he and others must know something I don't know. So I ask again, how can you be totally sure that missed instances or trying to process uninitialized instances won't happen? It's easy to overlook one missed instance where many others are processed just fine. As far as what I've learned, actions run threaded. Timer callbacks must be run from a thread as well, seems logical. The way I see it is, that if this will work then it will depend on how these threads run. A quote from wiki http://en.wikipedia.org/wiki/Thread_(computer_science)

Threads in the same process share the same address space. This allows concurrently-running code to couple tightly and conveniently exchange data without the overhead or complexity of an IPC. When shared between threads, however, even simple data structures become prone to race hazards if they require more than one CPU instruction to update: two threads may end up attempting to update the data structure at the same time and find it unexpectedly changing underfoot. Bugs caused by race hazards can be very difficult to reproduce and isolate.

Normally threads are managed in a preemptive manner. This means that you have no way of controlling if a thread is done with changing global data, before another context change occurs. Because this is all done by the OS.

If WC3 uses something like fiber threading then it might work flawlessly. It then would depend on how WC3 manages these cooperative threads. A function runs fully before yielding to allow another fiber to run (keeping in mind the max opcodes limit of course). Then it would work just fine. I couldn't find any confirmation on this. Using a single timer is great performance wise, no doubt about it, but what I'd really like to know is how safe it is thread wise.

 

[DiscipleOfLife]

I'm not entirely sure, but this thing PitzerMike once said might have something to do with this issue:

- - JASS is deterministic in that a JASS statement executes in 0 time per definition. - -

(http://www.wc3jass.com/viewtopic.php?t=2676)

Perhaps you know better what he ment with that but from what I can tell that pretty much means that using globals this way is safe. Though this was the only thing I could find right now that confirms this (assuming that PitzerMike's word is confirmation enough, and that I translated it correctly).

 

[Drone]

I'm not entirely sure, but this thing PitzerMike once said might have something to do with this issue:
(http://www.wc3jass.com/viewtopic.php?t=2676)

Perhaps you know better what he ment with that but from what I can tell that pretty much means that using globals this way is safe. Though this was the only thing I could find right now that confirms this (assuming that PitzerMike's word is confirmation enough, and that I translated it correctly).

Thanks for the link. I think he means that event threads are cooperative. A thread has to either yield, get suspended due to hitting the maximum allowed statements, or do its stuff within the maximum allowed statements. So normally thread code gets executed completely before it changes context again.

A good example b.t.w. why trigger waits are not wise to use.