Feedback on lightning script + test map

07-04-2009, 10:31 AM

This script's primary feature is to handle timed lightning effects, but it also supports additional features. The most noticeable one is connecting lightning effects to units. The script will only get the position of a unit once, even if multiple lightning effects is connected to the same unit. This is done by using an internal struct called LightningLoc, which has an x, y and z member. If a LightningLoc is attached to a unit, the x, y and z will be updated periodically. Then, the lightning effects will be moved to the appropriate coordinates. Since the LightningLocs have to be updated before the lightnings are moved, this script can't use TimedLoop, however in return, the update period can be specified.

To iterate trough all structs to update them, this script uses ListModule by grim001. This improves speed, and only one function call per struct type has to be done on every update period. This time, the script supports fading out lightnings. This is done by another struct called Fader. It has most of the members of the Lightning struct, and some additional ones.

The script needs some form of unit indexing, it uses the function GetUnitId(), so both UnitIndexingUtill and AutoIndex will work (PUI will also work if you configure the system to do that). You will however have to make sure the unit is indexed before it is passed to this system.

This is the code used to test this in the sample map:

JASS:

private function Actions takes nothing returns nothing
 local lightning l = AddLightning("DRAL", false, 0., 0., 0., 0.)
 // It doesn't matter where the lightning is created, it will be moved to the correct place by the system.
 local Lightning L = Lightning.create(l, 5., Lightning.atUnit(gg_unit_hfoo_0001), Lightning.atUnit(gg_unit_hfoo_0002))
 // Lightning.atUnit returns a lightning loc, Lightning.atCoords can also be used.
    call L.setFadeTime(0.5)
    // Sets the fade time of the lightning. Note that the fading starts AFTER the time specified in create.
    // In this case, the fading will start after 5 seconds, and it will be faded out a half second after that?
 set l = null
 // No destruction of either Lightning or lightning is needed. In fact, doing so would break the system.
endfunction

I know the API isn't perfect, how can I improve it?
Here is the script itself:

JASS:

library LightningUtils initializer init requires Indexing, ListModule

// Remove the /* and the */ if you are using PUI.
/* 
private function GetUnitId takes unit u returns integer
    return GetUnitIndex(u)
endfunction
*/

globals
    public constant real PERIOD = 0.040     // The rate at which the lightnings updates.

// ===================== End of Configuration Zone =====================

    private location loc = Location(0., 0.) // For getting location height
    private timer T = CreateTimer()         // The timer doing all the work
    private code TimerExpire                // This has to be here, else .evaluate() would have to be here.
    private integer TimerUsageCount = 0     // If this reaches 0, the timer will pause.
    
    private keyword LightningLoc
    private LightningLoc array UnitLightningLoc // Stores all LightningLocs attached to units
endglobals

// After this code is executed, the instance will be updated every time the timer expires.
//! textmacro LightningUtils__AddToTimer
    call .addList()
    set TimerUsageCount = TimerUsageCount + 1
    if TimerUsageCount == 1 then
        call TimerStart(T, PERIOD, true, TimerExpire)
    endif
//! endtextmacro

//! textmacro LightningUtils__RemoveFromQueue
    if .removeList() then
        set TimerUsageCount = TimerUsageCount - 1
    endif
//! endtextmacro

// =======================================================
// Code to decide where to move lightnings:
private struct LightningLoc
    implement List
    
    integer refCount = 0
    // This counts how many Lightning instances uses this instance,
    // so it will be destroyed when, and only when, it is not needed any more.
    
    //! textmacro CheckRefCount takes object
        if $object$.refCount == 1 then
            call $object$.destroy()
        else
            set $object$.refCount = $object$.refCount - 1
        endif
    //! endtextmacro
    
    private unit u = null
    
    real x = 0.
    real y = 0.
    real z = 0.
    
    method removeUnit takes nothing returns nothing
    // Removes the unit from the LightingLoc, the coordinates won't be changed any more.
        if .u != null then
            set UnitLightningLoc[GetUnitId(.u)] = 0
            set .u = null
            //! runtextmacro LightningUtils__RemoveFromQueue()
        endif
    endmethod
    
    method onDestroy takes nothing returns nothing
        call .removeUnit() // .removeUnit() is sometimes called outside destruction.
    endmethod
    
    static method fromCoords takes real x, real y, real z returns LightningLoc
     local LightningLoc this = LightningLoc.allocate()
        set this.x = x
        set this.y = y
        set this.z = z
     return this
    endmethod
    
    static method fromUnit takes unit u returns LightningLoc
     local integer id = GetUnitId(u)
     local LightningLoc this
        if u == null then // There can't be any lightning attached to null.
            debug call BJDebugMsg(SCOPE_PREFIX + "Error: Lightning.atUnit was called on a null unit.")
            return LightningLoc.fromCoords(0., 0., 0.)
        endif
        if UnitLightningLoc[id] != 0 then     // If the unit already has a LightningLoc, return it.
            return UnitLightningLoc[id]
        else
            set this = LightningLoc.allocate() // Else, create a new one.
            set UnitLightningLoc[id] = this
            set this.u = u
            //! runtextmacro LightningUtils__AddToTimer()
            return this
        endif
    endmethod
    
    static method update takes nothing returns nothing
     local LightningLoc this = .getFirst()
     local unit u
        loop
            exitwhen this == 0

                set u = .u
                set .x = GetUnitX(u)
                set .y = GetUnitY(u)
                call MoveLocation(loc, .x, .y) // Move the location to where the height should be read
                set .z = GetLocationZ(loc) + GetUnitFlyHeight(u) 
                // Fly height + Terrain height to get the unit's actual height
                
            set this = .getNext()
        endloop
     set u = null
    endmethod
endstruct

//======================================================================
// Struct for fading lightnings.

private struct Fader
    implement List
    private lightning l
    private integer i // See comments in the Lightning struct.
    
    private real r
    private real g
    private real b
    private real a
    private real da // d is supposed to mean delta.
    
    private LightningLoc start
    private LightningLoc end
    
    static method create takes lightning l, LightningLoc start, LightningLoc end, integer i returns Fader
     local Fader this = Fader.allocate()
        set .r = GetLightningColorR(l)
        set .g = GetLightningColorG(l)
        set .b = GetLightningColorB(l)
        set .a = GetLightningColorA(l)
        // Is saving the color faster than using GetLightningColor every time?
        set .i = i
        set .da = -.a/i
        set .l = l
        set .start = start
        set .end = end
        //! runtextmacro LightningUtils__AddToTimer()
     return this
    endmethod
    
    method onDestroy takes nothing returns nothing // This is almost the same as Lightning's onDestroy()
        call DestroyLightning(.l)
        //! runtextmacro CheckRefCount(".start")
        //! runtextmacro CheckRefCount(".end")
        //! runtextmacro LightningUtils__RemoveFromQueue()
    endmethod
    
    static method update takes nothing returns nothing
     local Fader this = .getFirst()
     local Fader prev
     local LightningLoc start
     local LightningLoc end
        loop
            exitwhen this == 0
            if .i > 0 then
                set .i = .i - 1
                set start = .start
                set end = .end
                set .a = .a + .da
                call SetLightningColor(.l, .r, .g, .b, .a)
                call MoveLightningEx(.l, false, start.x, start.y, start.z, end.x, end.y, end.z)
                set this = .getNext()
            else
                set prev = this
                set this = .getNext()
                call prev.destroy()
            endif
        endloop
    endmethod
    
endstruct

// =======================================================
// The struct that does the stuff with the lightnings.
struct Lightning
    implement List
    
    private lightning l         // The lightning itself.
    private integer i
    private LightningLoc start  // Where the lighting should start...
    private LightningLoc end    // ... and where it should end.
    private integer fadeIterations = 0
    
    static method create takes lightning l, real time, LightningLoc start, LightningLoc end returns Lightning
     local Lightning this = Lightning.allocate()
        if l == null then   // Return the error code -1 if the lightning wasn't created properly.
            return -1
        endif
        set .l = l
        set .start = start
        set .end = end
        set start.refCount = start.refCount + 1
        set end.refCount = end.refCount + 1
        set .i = R2I(time/PERIOD) // Calculates how many PERIOD the lightning should stay.
        //! runtextmacro LightningUtils__AddToTimer()
     return this
    endmethod
    
    method onDestroy takes nothing returns nothing
        if .fadeIterations == 0. then // Destroy the lightning if it is not going to fade.
            call DestroyLightning(.l)
            
            //! runtextmacro CheckRefCount(".start")
            //! runtextmacro CheckRefCount(".end")
            
        else // If it is going to fade, pass everything to the fader struct.    
            call Fader.create(.l, .start, .end, .fadeIterations)
        endif
        //! runtextmacro LightningUtils__RemoveFromQueue()
    endmethod
    
    static method update takes nothing returns nothing
     local Lightning this = .getFirst()
     local Lightning prev
     local LightningLoc start
     local LightningLoc end
        loop
            exitwhen this == 0
            if .i > 0 then
                set .i = .i - 1
                set start = .start
                set end = .end
                call MoveLightningEx(.l, false, start.x, start.y, start.z, end.x, end.y, end.z)
                set this = .getNext()
            else
                set prev = this
                set this = .getNext()
                call prev.destroy()
            endif
        endloop
    endmethod
    
    method setFadeTime takes real time returns nothing
        set .fadeIterations = R2I(time/PERIOD)
    endmethod
    
    static method atUnit takes unit u returns LightningLoc // Grants acsess to the LightningLoc struct.
        return LightningLoc.fromUnit(u) // Inline friendly
    endmethod
    
    static method atCoords takes real x, real y, real z returns LightningLoc
        return LightningLoc.fromCoords(x, y, z) // Inline friendly
    endmethod
endstruct

private function UnitLeave takes unit u returns nothing
    call UnitLightningLoc[GetUnitId(u)].removeUnit()
endfunction

hook RemoveUnit UnitLeave // Unit death isn't handled yet...

private function OnTimerExpire takes nothing returns nothing
    if TimerUsageCount == 0 then // If there is nothing to update...
        call PauseTimer(T)       // ...pause the timer...
        return                   // ...and don't do anything else.
    endif
    call LightningLoc.update() // First the locations of the lightnings have to be updated
    call Lightning.update()    // And then the lightnings themselves have to be updated
    call Fader.update()
endfunction

private function init takes nothing returns nothing
    set TimerExpire = function OnTimerExpire
endfunction

endlibrary

The scirpt should be more readable this time, since I have removed some unnessecary textmacros, and replaced some code with other textmacros.

I am planning to add something to handle lightnings that doesn't move, to improve speed. That will require TimerUtils. A question: How should this script handle unit death?

Here is the test map: LightningSystem.w3m
Any feedback is appreciated, I will give +rep for any useful feedback. Please note, I will not be able to post for about 5 weeks, but I will still read all feedback, and I will give rep as soon as I can.

Attached Files

LightningSystem.w3m (23.8 KB)

07-04-2009, 11:31 AM

Blubb-Tec

ok, this is what i've found from looking over the script and testing your map so far:
return 0 // This line will never be executed. why don't you just leave this lilne out if will never be executed? shouldn't be a problem to pjass afaik

Another thing you might want to change is that currently the user has to create a lightning himself, and pass that one to your Lightning struct's create method. you might simply add a string to that create method which then takes the "DRAL" or w/e lightning the user wishes to use, so he doesn't have to clean up etc. else it also looks as if the user has to destroy this thing again.

then, you do a check whether the unit is null in the fromUnit method, however you only do that check in debug mode.
you should probably change this:

JASS:

        debug if u == null then // There can't be any lightning attached to null.
            debug call BJDebugMsg(SCOPE_PREFIX + "Error: Lightning.atUnit was called on a null unit.")
            debug return LightningLoc.fromCoords(0., 0., 0.)
        debug endif

into this:

JASS:

        if u == null then // There can't be any lightning attached to null.
            debug call BJDebugMsg(SCOPE_PREFIX + "Error: Lightning.atUnit was called on a null unit.")
            return LightningLoc.fromCoords(0., 0., 0.)
        endif

then, when looking at the test map, i saw that the fromUnit method you're using there actually creates very ugly lightnings, since they start at the feet of the soldier, and end at the feet of the other soldier. ugh. probably you should add something like 50. to the .z coord, or find another way to do that. also you should a possibility for the user to add an effect to the start and end unit, like this green or red glow which is also used by most standard spells, since else the beginning/end of each lightning look like they're "cut off".

Despite from that, I like this system very much, since it provides a good way to attach a lightning to a unit. very cool

i also like the fader very much, since it looks very good ingame(why didn't you use vex' ARGB for that, btw?)

all in all, nice system

07-18-2009, 01:57 PM

0zyx0

Thank you for the feedback. I will update the script in the first post with the small changes. About the passing of a lightning to the struct, I did it that way so the color of the lightning would be configurable. Sacrificing that benefit would also improve the speed slightly, but I will probably make a more complicated (or advanced) way of changing the color, possibly using ARGB.

To fix the problem with lightning attached to the origin of units, I will probably add two Z-offset constants, one for buildings and one for normal units.

Finally, I actually made a way of attaching special effects at the end of the lightnings, but that turned out to complicate things a lot. A better solution would be to add special effects to units as usual. The problem then shifts to the destruction, which can be fixed by passing a function interface argument to the struct, which is evaluated when the lightning is destroyed.