Which is more efficient?

[WereElf]

I have few questions:

1) Is it better to do multiple "if" checks:

if udg_int == 'A000' then
    call UnitAddAbility(u, 'A02B')
elseif udg_int == 'A001' then
    call UnitAddAbility(u, 'A02G')
elseif udg_int == 'A004' then
    call UnitAddAbility(u, 'A01Q')
elseif udg_int == 'A00A' then
    call UnitAddAbility(u, 'A031')
endif

Or would it be better if I use a hashtable:
call UnitAddAbility(u, LoadInteger(Table, 'abil', udg_int))


2) Is it better to start a seperate (not-repeating) timer for every unit:

function OnExpire takes nothign returns nothing
    local timer t = GetExpiredTimer()
    local unit u = LoadUnitHandle(Table, GetHandleId(t), 'unit')
    call UnitRemoveAbility(u, 'A021')
    call FlushChildHashtable(Table, GetHandleId(t))
    call DestroyTimer(t)
    set t = null
    set u = null
endfunction

function First takes nothin returns nothing
    // some stuff
    call UnitAddAbility(u, 'A021')
    call SaveUnitHandle(Table, GetHandleId(t), 'unit', u)
    call TimerStart(t, 5, false, function OnExpire)
    // clearing leaks
enefunction

Or would it be better to use 1 looping timer for all the units?:

function OnExpire takes nothing returns nothing
    local integer i = 1
    loop
        exitwhen i > udg_max
        set udg_time[i] = udg_time[i] - 0.03125
        if udg_time[i] <= 0 then
            call UnitRemoveAbility(udg_unit[i], 'A021')
            set udg_time[i] = udg_time[udg_max]
            set udg_unit[i] = udg_unit[udg_max]
            set udg_max = udg_max - 1
        else
            set i = i + 1
        endif
    endloop
    if udg_max == 0 then
        call DestroyTimer(GetExpiredTimer())
    endif
endfunction

function First takes nothing returns nothing
    //some actions
    if udg_max == 0 then
        set t = CreateTimer()
        call TimerStart(t, 0.03125, true, function OnExpire)
    endif
    set udg_max = udg_max + 1
    set udg_unit[udg_max] = u
    set udg_time[udg_max] = 5
    //leak clearing
endfunction

And also, I feel that both of the ways can be efficient, but 1 is used for some cases, and the other - for other cases.
When do I want to use the 1-st and when do I want to use the 2-nd method?

 

[Ardenian]

1) If you only have these abilities listed there, then a hashtable would be too much. You could also use an array variable for it.

2) A seperate timer is good if you don't need the actions all the time, since a periodic timer will run all the time, even when deactivating it via trigger.

 

[SAUS]

1) If you only have these abilities listed there, then a hashtable would be too much. You could also use an array variable for it.

2) A seperate timer is good if you don't need the actions all the time, since a periodic timer will run all the time, even when deactivating it via trigger.

He is destroying the timer if it is not supposed to be running. I do this in a bunch of spots in my triggers as well.

The first one definitely is overkill with a hashtable with that much data, but a hashtable will be better if you plan to add a lot of these checks. It will scale nicely as you add more - the time to retrieve the data remains constant. Using a big set of if statements will be ugly to maintain and will become slower as you add more and more of these (if you even plan to do that - it also will probably remain unnoticeable for any amount that a human would add unless it is running very often).

 

[Dr Super Good]

Hashtable is best approach and you can use it for similar tasks elsewhere.

One timer looping for all instances is the most efficient as the thread creation on timer expiry is a resource intensive operation. Do be aware that periodic timers currently will load from a saved game as "one shot" (similar to pausing and resuming them) and that destroying a timer does not deschedule its function from running on expiry (you need to pause the timer before destruction).

Paused timers and periodic triggers have practically no overhead. They are descheduled from running and do not advance in time.

 

[Anitarf]

Hashtable is best approach and you can use it for similar tasks elsewhere.

Agreed.

One timer looping for all instances is the most efficient as the thread creation on timer expiry is a resource intensive operation.

This is true for frequent periodic timers, however in his example he just needs one update after a longer duration, in which case a timer per instance is the better approach. I would recommend using TimerUtils for it instead of creating and destroying the timer, though.

 

[Dr Super Good]

This is true for frequent periodic timers, however in his example he just needs one update after a longer duration, in which case a timer per instance is the better approach. I would recommend using TimerUtils for it instead of creating and destroying the timer, though.

Did not notice his code is not functionally equivalent. One cannot be replaced with the other really.

Separate timers are most efficient for specific timeouts and are certainly more efficient than decrementing a counter. They are not efficient for highly periodic effects due to the thread creation overhead. Decrementing/incrementing counters can be convenient to do when you need to perform other actions at the same time since it save you having to deal with managing the timer.

 

[WereElf]

Alright, thanks for the info, but now I have another question "Which is more efficient?"

3) To run everything in the same thread:

// some code overhead (in this case left[] and right[] are local unit arrays)
            if left_c > right_c then
                loop
                    exitwhen right_c == -1
                    if count <= targets/2 then
                        set dice = GetRandomInt(0, right_c)
                        call IssueTargetOrder(dummy, "acidbomb", right[dice])
                        call GroupAddUnit(damage.shotGroup, right[dice])
                        set count = count + 1
                        set right[dice] = right[right_c]
                    endif
                    set right[right_c] = null
                    set right_c = right_c - 1
                endloop
                loop
                    exitwhen left_c == -1
                    if count < targets then
                        set dice = GetRandomInt(0, left_c)
                        call IssueTargetOrder(dummy, "acidbomb", left[dice])
                        call GroupAddUnit(damage.shotGroup, left[dice])
                        set count = count + 1
                        set left[dice] = left[left_c]
                    endif
                    set left[left_c] = null
                    set left_c = left_c - 1
                endloop
            else
                loop
                    exitwhen left_c == -1
                    if count <= targets/2 then
                        set dice = GetRandomInt(0, left_c)
                        call IssueTargetOrder(dummy, "acidbomb", left[dice])
                        call GroupAddUnit(damage.shotGroup, left[dice])
                        set count = count + 1
                        set left[dice] = left[left_c]
                    endif
                    set left[left_c] = null
                    set left_c = left_c - 1
                endloop
                loop
                    exitwhen right_c == -1
                    if count < targets then
                        set dice = GetRandomInt(0, right_c)
                        call IssueTargetOrder(dummy, "acidbomb", right[dice])
                        call GroupAddUnit(damage.shotGroup, right[dice])
                        set count = count + 1
                        set right[dice] = right[right_c]
                    endif
                    set right[right_c] = null
                    set right_c = right_c - 1
                endloop
            endif

Or should I make seperate functions for the actions, since they are doing the same thing (just have different order)?

// some code overhead (in this case left[] and right[] are GLOBAL unit arrays)
function RightLoop takes integer c, integer max, unit d, DamageProperties damage returns integer
    local integer dice
    loop
        exitwhen right_c == -1
        if c <= max then
            set dice = GetRandomInt(0, right_c)
            call IssueTargetOrder(d, "acidbomb", right[dice])
            call GroupAddUnit(damage.shotGroup, right[dice])
            set c = c + 1
            set right[dice] = right[right_c]
        endif
        set right[right_c] = null
        set right_c = right_c - 1
    endloop
    return c
endfunction

function LeftLoop takes integer c, integer max, unit d, DamageProperties damage returns integer
    local integer dice
    loop
        exitwhen left_c == -1
        if c <= max then
            set dice = GetRandomInt(0, left_c)
            call IssueTargetOrder(d, "acidbomb", left[dice])
            call GroupAddUnit(damage.shotGroup, left[dice])
            set c = c + 1
            set left[dice] = left[right_c]
        endif
        set left[left_c] = null
        set left_c = left_c - 1
    endloop
    return c
endfunction

// some actions, in another function
            if left_c > right_c then
                set count = RightLoop(count, targets/2, dummy, damage)
                set count = LeftLoop(count, targets, dummy, damage)
            else
                set count = LeftLoop(count, targets/2, dummy, damage)
                set count = RightLoop(count, targets, dummy, damage)
            endif

And by the way, can I use a textmacro inside a textmacro?

 

[Anitarf]

It is not good practice to copy code like that. You should find a way to reuse the same code for both left and right loop, and no I don't mean using a textmacro.

 

[WereElf]

I'm open to suggestions how to do it with just 1 loop.
I can think of 1-2 ways, but they will end up being heavier than the code is now...
The idea of this code is to hit half of the units from the "left", and the other half from the "right".
If there aren't enough enemies - in "right" - it starts the loop from there to hit all of them, and get the rest from "left".

EDIT: Perhaps the easiest way to do it would be:

    loop
        exitwhen right_c + left_c == -2
        if right_c > -1 then
            if count <= targets then
                set dice = GetRandomInt(0, right_c)
                call IssueTargetOrder(d, "acidbomb", right[dice])
                call GroupAddUnit(damage.shotGroup, right[dice])
                set count = count + 1
                set right[dice] = right[right_c]
            endif
            set right[right_c] = null
            set right_c = right_c - 1
        endif
        if left_c > -1 then
            if count <= targets then
                set dice = GetRandomInt(0, left_c)
                call IssueTargetOrder(d, "acidbomb", left[dice])
                call GroupAddUnit(damage.shotGroup, left[dice])
                set count = count + 1
                set left[dice] = left[left_c]
            endif
            set left[right_c] = null
            set left_c = left_c - 1
        endif
    endloop

 

[Anitarf]

I can't make any specific suggestions because I don't know what the rest of the code looks like, but you really should use a non-code-copy approach, to avoid errors like this: set left[dice] = left[right_c]

 

[WereElf]

This code is part of my Custom Multishot.
And yeah, in Version 1.6.2, I had a bug caused by such mistake xP (current version is 1.8.4.1).
Should I make it use the code I posted in the "edit" of my previous comment?
(just have in mind that my Multishot has a lot of code, but not nearly half of it executes at the same time. It has several branches, seperated by ifs.)

 

[Anitarf]

Hmm... well, this would be my approach, it does seem like it would be a more complex algorithm but it would minimize code duplication:

Since you already know the maximum number of units that will be picked when you start populating the arrays, you could take this into account then, instead of first putting all the units into the arrays and then picking randomly from them later.

I would use a single array to hold the chosen units, with a "fixed" size. Add the units on the left to the start of the array and units one the right to the end. Insert each new unit in a random spot from 1 to the number of units looped and move the unit that was already in that spot to the last spot. That way, even units that get added last have a chance to make the cut. Now for the trick: when storing a new or a moved unit to a spot, only store them if either you haven't reached the middle of the array yet or the spot is empty. That way, if there are too few units on the right, the left units will be allowed to populate more than half of the array, but if the loop then encounters more units on the right those units will be allowed to overwrite the left units until the right units also fill half of the array. After that, the only new units added to the array would be the ones lucky enough to get randomly assigned into a spot lower than half the size of the array.

                        if temp_ang >= 0 and temp_ang < bj_PI then
                            set left_c = left_c+1 // starts at 0 in my example, it's easier for me to think if the array starts at 1
                            set tempint = GetRandomInt(1, left_c)
                            if tempint<=SIZE and ( tempint<=(SIZE+1)/2 or units[left_c]==null ) then
                                if left_c<=SIZE and ( left_c<=(SIZE+1)/2 or units[left_c]==null ) then
                                    set units[left_c] = units[tempint]
                                endif
                                set units[tempint] = secondaryTarget
                            endif
                        else
                            set right_c = right_c+1 //same as left_c, starts at 0
                            set tempint = GetRandomInt(1, right_c)
                            if tempint<=SIZE and ( tempint<=(SIZE+1)/2 or units[SIZE+1-right_c]==null ) then
                                if right_c<=SIZE and ( right_c<=(SIZE+1)/2 or units[SIZE+1-right_c]==null ) then
                                    set units[SIZE+1-right_c] = units[tempint]
                                endif
                                set units[SIZE+1-tempint] = secondaryTarget
                            endif
                        endif

I realize this still has some code duplication here, but it gives you a finalised units array with randomly chosen targets stored under indexes from 1 to SIZE, so there's no more code duplication after this, just a single loop to apply the spell effect on all the targets.

 

[Dr Super Good]

It is not good practice to copy code like that. You should find a way to reuse the same code for both left and right loop, and no I don't mean using a textmacro.

It is called procedural coupling, something you want to minimize when writing code but is often maximized in actual produced machine code. In languages like Java or C/C++ you usually get rid of it with functions/methods which you pass the appropriate arguments and then let the compiler inline them. In JASS the compiler does no optimizations as far as I am aware and due to the runtime resolution of JASS it is often more efficient to use procedural coupling to do something rather than have the extra overhead of function calls or variable resolution.

As such for non-critical code I would recommend reducing procedural coupling as much as possible but for critical code increasing it by explicit in-lining and loop unwinding.

Here is a rather rough example for what you can do.

function SetSide takes boolean right, integer pos, unit u returns nothing
    if right then
        set right[pos] = u
    else
        set left[pos] = u
    endif
endif

function GetSide takes boolean right, integer pos returns unit
    if right then
        return right[pos]
    endif
    return left[pos]
endif

function DoLoop takes boolean right, integer c, integer max, unit d, DamageProperties damage returns integer
    local integer dice
    local integer side_c
    local unit u

    // fetch side based c
    if right then
        set side_c = right_c
    else
        set side_c = left_c
    endif

    loop
        exitwhen side_c == -1

        if c <= max then
            // resolve random side unit
            set u = GetSide(right, dice)

            set dice = GetRandomInt(0, side_c)
            call IssueTargetOrder(d, "acidbomb", u)
            call GroupAddUnit(damage.shotGroup, u)
            set c = c + 1

            // remove random side unit from list
            call SetSide(right, dice, GetSide(right, side_c))
        endif

        // cleanup top side unit
        call SetSide(right, side_c, null)

        set side_c = side_c - 1
    endloop

    // update side based c
    if right then
        set right_c = side_c
    else
        set left_c = side_c
    endif

    // bugfix LDLHVRCL
    set u = null

    return c
endfunction

// some actions, in another function
    local boolean right = left_c > right_c
    set count = DoLoop(right, count, targets/2, dummy, damage)
    set count = DoLoop(not right, count, targets, dummy, damage)

Due to the lack of references there are a number of conditional resolutions required which will obviously slow the code down. In theory a good optimizer could spot this and inline/expand the calls and conditions to produce the efficient code one would expect.

 

[WereElf]

Hmm... well, this would be my approach, it does seem like it would be a more complex algorithm but it would minimize code duplication:

Since you already know the maximum number of units that will be picked when you start populating the arrays, you could take this into account then, instead of first putting all the units into the arrays and then picking randomly from them later.

I would use a single array to hold the chosen units, with a "fixed" size. Add the units on the left to the start of the array and units one the right to the end. Insert each new unit in a random spot from 1 to the number of units looped and move the unit that was already in that spot to the last spot. That way, even units that get added last have a chance to make the cut. Now for the trick: when storing a new or a moved unit to a spot, only store them if either you haven't reached the middle of the array yet or the spot is empty. That way, if there are too few units on the right, the left units will be allowed to populate more than half of the array, but if the loop then encounters more units on the right those units will be allowed to overwrite the left units until the right units also fill half of the array. After that, the only new units added to the array would be the ones lucky enough to get randomly assigned into a spot lower than half the size of the array.

                        if temp_ang >= 0 and temp_ang < bj_PI then
                            set left_c = left_c+1 // starts at 0 in my example, it's easier for me to think if the array starts at 1
                            set tempint = GetRandomInt(1, left_c)
                            if tempint<=SIZE and ( tempint<=(SIZE+1)/2 or units[left_c]==null ) then
                                if left_c<=SIZE and ( left_c<=(SIZE+1)/2 or units[left_c]==null ) then
                                    set units[left_c] = units[tempint]
                                endif
                                set units[tempint] = secondaryTarget
                            endif
                        else
                            set right_c = right_c+1 //same as left_c, starts at 0
                            set tempint = GetRandomInt(1, right_c)
                            if tempint<=SIZE and ( tempint<=(SIZE+1)/2 or units[SIZE+1-right_c]==null ) then
                                if right_c<=SIZE and ( right_c<=(SIZE+1)/2 or units[SIZE+1-right_c]==null ) then
                                    set units[SIZE+1-right_c] = units[tempint]
                                endif
                                set units[SIZE+1-tempint] = secondaryTarget
                            endif
                        endif

I realize this still has some code duplication here, but it gives you a finalised units array with randomly chosen targets stored under indexes from 1 to SIZE, so there's no more code duplication after this, just a single loop to apply the spell effect on all the targets.

That's an elegant aproach. However, how will it impact the performance?
Will it be heavier or lighter than my code as it is now (or will it be pretty much the same)?

EDIT: probobly your way is more efficient xP

EDIT 2: Okay I did this:

                        if temp_ang >= 0 and temp_ang < bj_PI then
                            set left_c = left_c + 1
                            set dice = GetRandomInt(1, left_c)
                            if dice <= (targets - count) and ( dice <= ((targets - count) + 1)/2 or units[left_c] == null ) then
                                if left_c <= (targets - count) and ( left_c <= ((targets - count)+ 1)/2 or units[left_c] == null ) then
                                    set units[left_c] = units[dice]
                                    debug set s[left_c] = s[dice]
                                endif
                                set units[dice] = secondaryTarget
                                debug set s[dice] = "left"
                            endif
                        else
                            set right_c = right_c + 1
                            set dice = GetRandomInt(1, right_c)
                            if dice <= (targets - count) and ( dice <= ((targets - count) + 1)/2 or units[(targets - count) + 1 - right_c] == null ) then
                                if right_c <= (targets - count) and ( right_c <= ((targets - count) + 1)/2 or units[(targets - count) + 1 - right_c] == null ) then
                                    set units[(targets - count) + 1 - right_c] = units[dice]
                                    debug set s[targets - count + 1 - right_c] = s[dice]
                                endif
                                set units[(targets - count) + 1 - dice] = secondaryTarget
                                debug set s[targets - count + 1 - dice] = "right"
                            endif
                        endif
                    endif
                endif
                call GroupRemoveUnit(g, secondaryTarget)
            endloop
            set dice = 1
            loop
                exitwhen dice > targets
                if units[dice] != null then
                    set xTarget = GetUnitX(units[dice])
                    set yTarget = GetUnitY(units[dice])
                    call $PART_ONE$(dummy, $PART_TWO$, units[dice] $PART_THREE$)
                    call GroupAddUnit(damage.shotGroup, units[dice])
                    debug call BJDebugMsg(s[dice])
                    set count = count + 1
                    set units[dice] = null
                endif
                set dice = dice + 1
            endloop

But now I have a minor "problem" when I have targets set to 3, and count starts with 1 (instead of 0) - it may shoot the main target + 2 targets form the left, or 2 targets from the right, while its supposed to hit 1 from the left and 1 from the right (it is doing that too, sometimes).
Why would that be happening? xP

EDIT 3: Okay, I fixed the problem. Had to do:
if dice <= (targets - count) and ( dice <= ((targets - count) + 1)/2 or (units[left_c] == null and left_c <= (targets - count))) then
and
if dice <= (targets - count) and ( dice <= ((targets - count) + 1)/2 or (units[(targets - count) + 1 - right_c] == null and right_c <= (targets - count))) then

EDIT 4: Okay, now units[] contains the same unit several times (sometimes)...

EDIT 5: Alright, fixed that too.. it was supposed to be set units[(targets - count) + 1 - right_c] = units[(targets - count) + 1 - dice]
and in your version - set units[SIZE+1-right_c] = units[SIZE+1-tempint]

 

[Anitarf]

See, even I made an error when copying code.

I'm not sure what your "targets" and "count" values are and whether they are the correct replacement for my "SIZE" value, but I guess it wouldn't work correctly otherwise? Basically, SIZE was meant to be the number of additional targets (beyond the first, central target) of the multishot.

In terms of efficiency, I'm not sure, you were getting random values for each targeted unit while I'm doing it for each filtered unit so It's possible that my approach is slightly more costly, although as the number of units goes up, my approach will be doing fewer and fewer operations per unit as most of them will start quitting on the first condition of the first if statement. You were only doing an array write and a variable increment for each unit, so it's possible that even my ideal case of one GetRandomInt plus one comparison would be slower.

It really doesn't matter, though, in general coding correctly is more important than coding efficiently (not to mention that if you're coding correctly, you won't be too inefficient). The time computers spend processing your code is measured in milliseconds while the time you spend maintaining it is measured in hours.

If you want to optimize it further, you could pre-calculate the "targets-count" value, if that is the correct value to use there.

 

[WereElf]

Yeah, I actually did that with the last update (just now) (I am setting targets to (targets - count) if count is different than 0, before the loop)
I kind a needed to, cuz I was getting some annoying issues (you can imagine why, when you see the code below).
I even changed the loop to skip empty spaces if too high number of allowed units is set

            loop
                exitwhen dice > targets
                if units[dice] != null then
                    set xTarget = GetUnitX(units[dice])
                    set yTarget = GetUnitY(units[dice])
                    call $FUNC$, units[dice])
                    call GroupAddUnit(damage.shotGroup, units[dice])
                    set count = count + 1
                    set units[dice] = null
                elseif dice == left_c + 1 and targets - right_c > dice then
                    set dice = targets - right_c - 1
                endif
                set dice = dice + 1
            endloop

Before if someone had called my multishot with 3000 targets allowed - it would've done the full loop from 1 to 3000. Now it loops from 1 to left_c + 1, and then goes straight to targets - right_c.

 

[Anitarf]

You should also include a safety check that sets the targets allowed to 8190 if the user specifies a higher number.

 

[WereElf]

Okay, another questions about "which is more efficient":
4) Using Atan2, and then Sin and Cos

set a = Atan2(y2 - y1, x2 - x1) // or alternatively: set a = Atan((y2 - y1)/(x2 - x1))
set x1 = x1 + dist*Cos(a)
set y1 = y1 + dist*Sin(a)

Or using SquareRoot:

set w = x2 - x1
set h = y2 - y1
set hipo = SquareRoot(w*w + h*h)
set x1 = x1 + dist*(w/hipo)
set y1 = y1 + dist*(h/hipo)

EDIT:
Do have in mind, that (w*w)/(hipo*hipo) is NOT equal to w/hipo, so I can't do it without the square root.

 

[Dr Super Good]

Do have in mind, that (w*w)/(hipo*hipo) is NOT equal to w/hipo, so I can't do it without the square root.

It is however equal to (w/hipo)^2. Not that I see why you raised that...

First "Atan2" approach you are using is the polar offset. Second "SquareRoot" approach is using line segments.

Speed wise there is probably very little between them. One will have to profile it to find out which is faster. It is only worth doing this if you really need speed for that part of the code.

 

[WereElf]

It is however equal to (w/hipo)^2. Not that I see why you raised that...

I mentioned it, cuz I don't need (w/hipo)^2, I need (w/hipo), and i can't get it without using SquareRoot.

Speed wise there is probably very little between them. One will have to profile it to find out which is faster. It is only worth doing this if you really need speed for that part of the code.

How does one "profile it"? Can it be benchmarked or something?

 

[Dr Super Good]

How does one "profile it"? Can it be benchmarked or something?

You run the code repeatedly in such a way as to make the game drop frames significantly. For example in a loop for 20,000 or 200,000 times. Best is to unwind the loop as much as possible to reduce loop overhead.

You then time how long WC3 "freezes" while it runs the loop. Repeat until consistent results are obtained. The one with the shortest freeze time would be the fastest, and by comparing times you can get some figure of how much slower the other approach is (careful of loop overhead).

 

[WereElf]

And how do I measure the freeze time?

 

[Dr Super Good]

With a stop watch? I mean you should be measuring long periods of time like 30 seconds to a minute.

 

[WereElf]

I tried to do this:

native StopWatchCreate takes nothing returns integer

native StopWatchMark takes integer id returns integer

native StopWatchDestroy takes integer id returns nothing

native StopWatchTicks takes integer id returns integer

function Trig_Test_Actions takes nothing returns nothing
    local real sq
    local real at
    local real x1
    local real y1
    local real x2
    local real y2
    local real h
    local real w
    local real a
    local integer i = 0
    local integer s = StopWatchCreate()
    set x1 = 105.0
    set y1 = -36.7
    loop
        exitwhen i >= 2500
        set x2 = 243.4
        set y2 = 112.3
        set h = y2 - y1
        set w = x2 - x1
        set a = Atan2(h, w)
        set x1 = x1 + Cos(a)
        set y1 = y1 + Sin(a)
        set i = i + 1
    endloop
    set sq = StopWatchMark(s)
    set i = 0
    call StopWatchDestroy(s)
    set s = StopWatchCreate()
    set x1 = 105.0
    set y1 = -36.7
    loop
        exitwhen i >= 2500
        set x2 = 243.4
        set y2 = 112.3
        set h = y2 - y1
        set w = x2 - x1
        set a = SquareRoot(h*h + w*w)
        set x1 = x1 + (w/a)
        set y1 = y1 + (h/a)
        set i = i + 1
    endloop
    set at = StopWatchMark(s)
    call StopWatchDestroy(s)
    call BJDebugMsg(R2SW(sq,10,10)+" sq")
    call BJDebugMsg(R2SW(at,10,10)+" at")
endfunction

//===========================================================================
function InitTrig_Test takes nothing returns nothing
    set gg_trg_Test = CreateTrigger(  )
    call TriggerRegisterPlayerEventEndCinematic( gg_trg_Test, Player(0) )
    call TriggerAddAction( gg_trg_Test, function Trig_Test_Actions )
endfunction

But it throws me to the start menu when I try to launch the map...

EDIT: Hmm, it's possible that this happens cuz my SharpCraft is old version... will update it now and see if it works then.
I hope the newest version of SharpCraft works with the old patch.... xP

EDIT 2: Okay, it doesn't let me downalod it... says it may contain a virus or a spyware. I turned off my anti-virus, but still the same thing....

Could you test it for me, please? xP

 

[Dr Super Good]

Have they even released a SharpCraft version with 1.27 support yet? Until then it is in the same place as Reinventing The Craft of legacy tools that no longer work.

 

[WereElf]

The newest version of SharpCraft DOES support patch 1.27a, however I'm still using 1.26.

I downloaded the previous version of SharpCraft, which supports 1.26, but I have no idea how to install it.
When I try to open the Launcher.exe - I get "The launcher has not been implemented."
When I try to open the EasyHook - I get some error message.
When I try to run SharpCraft trough JNGP - nothing happens.

The only thing(s) working are the 4 .bat files, but even using the "Start game windowed.bat" - I still can't run the map.....

So could you PLEASE test this for me? xP

 

[Dr Super Good]

So could you PLEASE test this for me? xP

I do not have SharpCraft installed.

 

[Bribe]

WereElf, wouldn't it be easier for you to just update your WC3?

 

[WereElf]

WereElf, wouldn't it be easier for you to just update your WC3?

I don't think that this will allow me to use SharpCraft. I got the SC version, which supports patch 1.26, so my WC version isn't the problem.
And my net is too slow for me to bother even trying to update it now (usually I update/download stuff when I'm not home, and got faster net).

Anyways... I hit google, and people there claim that SquareRoot is faster than Atan2. But they were claiming that about other languages, not JASS (and they did mention, that it may varry in other languages)...

Is it possible to benchmark without using StopWatch??
Like, if I do something like this:

scope Test initializer Init
    globals
        timer StopWatch = CreateTimer()
        real Begining
    endglobals

    function Start takes nothing returns nothing
        set Begining = TimerGetElapsed(StopWatch)
    endfunction

    function Mark takes nothing returns real
        return TimerGetElapsed(StopWatch) - Begining
    endfunction

    function Trig_Test_Actions takes nothing returns boolean
        local real sq
        local real at
        local real x1
        local real y1
        local real x2
        local real y2
        local real h
        local real w
        local real a
        local integer i = 0
        call Start()
        set x1 = 105.0
        set y1 = -36.7
        set x2 = 243.4
        set y2 = 112.3
        loop
            exitwhen i >= 2500
            set x2 = x2 + 1
            set y2 = y2 + 1.5
            set h = y2 - y1
            set w = x2 - x1
            set a = Atan2(h, w)
            set x1 = x1 + Cos(a)
            set y1 = y1 + Sin(a)
            set i = i + 1
        endloop
        set sq = Mark()
        set i = 0
        call Start()
        set x1 = 105.0
        set y1 = -36.7
        set x2 = 243.4
        set y2 = 112.3
        loop
            exitwhen i >= 2500
            set x2 = x2 + 1
            set y2 = y2 + 1.5
            set h = y2 - y1
            set w = x2 - x1
            set a = SquareRoot(h*h + w*w)
            set x1 = x1 + (w/a)
            set y1 = y1 + (h/a)
            set i = i + 1
        endloop
        set at = Mark()
        call BJDebugMsg(R2SW(sq,10,10)+" sq")
        call BJDebugMsg(R2SW(at,10,10)+" at")
        return false
    endfunction

    function Init takes nothing returns nothing
        set gg_trg_Test = CreateTrigger(  )
        call TriggerRegisterPlayerEventEndCinematic( gg_trg_Test, Player(0) )
        call TriggerAddCondition( gg_trg_Test, Condition(function Trig_Test_Actions ))
        call TimerStart(StopWatch, 999999.00, true, null)
    endfunction
endscope

EDIT: Nope... Shows 0.000000000 for both xP

 

[Bribe]

How to benchmark: FPS tests. Spam function calls in a timed loop until it gets to (for example) 45 FPS. See how much of the other one you need to spam to get it to 45 FPS. Say it takes 100,000 calls to hit 45 FPS with function A, how many calls to B are needed to hit the same FPS?

 

[WereElf]

Okay, Atan2 + Sin + Cos is better than SquareRoot:

scope Test

globals
    integer SQ_Loops = 1000
    integer AT_Loops = 1000
    timer Refresher = CreateTimer()
    boolean On = false
endglobals

function SQ_Loop takes nothing returns nothing
    local integer i = 0
    local real x1 = 125.8
    local real y1 = -242.6
    local real x2 = 193.5
    local real y2 = 278.3
    local real h
    local real w
    local real a
    loop
        exitwhen i == SQ_Loops
        set x2 = x2 + 1
        set y2 = y2 - 0.4
        set h = y2 - y1
        set w = x2 - x1
        set a = SquareRoot(w*w + h*h)
        set x1 = x1 + (w/a)
        set y1 = y1 + (h/a)
        set i = i + 1
    endloop
endfunction

function AT_Loop takes nothing returns nothing
    local integer i = 0
    local real x1 = 125.8
    local real y1 = -242.6
    local real x2 = 193.5
    local real y2 = 278.3
    local real h
    local real w
    local real a
    loop
        exitwhen i == AT_Loops
        set x2 = x2 + 1
        set y2 = y2 - 0.4
        set h = y2 - y1
        set w = x2 - x1
        set a = Atan2(h, w)
        set x1 = x1 + Cos(a)
        set y1 = y1 + Sin(a)
        set i = i + 1
    endloop
endfunction

function SQ_Actions takes nothing returns boolean
    if not On then
        call TimerStart(Refresher, 0.01, true, function SQ_Loop)
        set On = true
    else
        set SQ_Loops = SQ_Loops + 100
    endif
    call BJDebugMsg(I2S(SQ_Loops)+" SQ")
    return false
endfunction

function AT_Actions takes nothing returns boolean
    if not On then
        call TimerStart(Refresher, 0.01, true, function AT_Loop)
        set On = true
    else
        set AT_Loops = AT_Loops + 100
    endif
    call BJDebugMsg(I2S(AT_Loops)+" AT")
    return false
endfunction

function Stop takes nothing returns boolean
    call PauseTimer(Refresher)
    call BJDebugMsg(I2S(SQ_Loops)+" SQ")
    call BJDebugMsg(I2S(AT_Loops)+" AT")
    set On = false
    return false
endfunction

//===========================================================================
function InitTrig_Test takes nothing returns nothing
    local trigger tr = CreateTrigger(  )
    set gg_trg_Test = CreateTrigger(  )
    call TriggerRegisterPlayerKeyEventBJ( gg_trg_Test, Player(0), bj_KEYEVENTTYPE_DEPRESS, bj_KEYEVENTKEY_UP )
    call TriggerAddCondition( gg_trg_Test, Condition(function SQ_Actions) )
    call TriggerRegisterPlayerKeyEventBJ( tr, Player(0), bj_KEYEVENTTYPE_DEPRESS, bj_KEYEVENTKEY_DOWN )
    call TriggerAddCondition( tr, Condition(function AT_Actions) )
    set tr = CreateTrigger(  )
    call TriggerRegisterPlayerEventEndCinematic( tr, Player(0) )
    call TriggerAddCondition( tr, Condition(function Stop) )
    set tr = null
endfunction
endscope

They both reached 45 FPS at 2000 loops every 0.01 seconds.
But at 3000 loops every 0.01 seconds Atan2 had 5-6.5 FPS, while SquareRoot dropped under 2 FPS

 

[WereElf]

Well, I have another "interesting" (as Bribe calls them) question:
Which is more effecient:
5) To put an 'if' and prevent the action from happening if the conditions aren't met.

//function 2
if my_condition then
    call FlushChildHashtable(Table, id)
endif

// function 1
if my_condition then
    call SaveInteger(Table, id, 0, struct)
endif

Or simply do the action every time, regardless if the condition is true or false, even if it's not needed when the condition isn't met

//function 2
call FlushChildHashtable(Table, id)

//function 1
call SaveInteger(Table, id, 0, struct)

Note: "my_condition" is a CONSTANT boolean. So it will either be passing the check every time, either failing it every time. However, in my opinion it will rarely be set to fail the condition

 

[Bribe]

That's why static if's exist in vJass.

 

[WereElf]

How exactly does "static if" work?
It compiles the code under "then" only if the condition is true?
And can it really be used with constant booleans?
Until now I've only used it for stuff like static if LIBRARY_blablabla then

EDIT: apparently it works (I tried it), but still, care to explain how exactly does it work?

 

[Anitarf]

Does it really need an explanation? It's not rocket science. The compiler just evaluates the if statement and parses the code if the expression evaluates to true.

 

[Bribe]

static if's are unique to JassHelper and don't even allow their evaluations nor contents to be parsed if the result is false. You may want to consult the JassHelper manual

 

[Dr Super Good]

Static ifs are a hacky solution to a lack of a proper optimization stage in JassHelper.

With the following declaration...

globals
    constant boolean RUNIT = true
    constant boolean NORUN = false
endglobals

If an optimizer sees this...

if RUNIT then
    call dosomething()
endif

if NORUN then
    call dosomething()
endif

It should produce this out...

//if true then
    call dosomething()
//endif

//if false then
//    call dosomething()
//endif

This is what a good C/C++ compiler like GCC does for the C/C++ language.

 

[Anitarf]

I disagree, I think it's good that you have to explicitly state that you want the if to be static, sure it wouldn't matter to a smarter compiler but it matters to the people reading the code. In fact, I'd have liked it better if it were a //! if... preprocessor command so it would stand out more from the rest of the code.