Function Call and Recursion Problem

[chopinski]

I'm trying to code the following item effect:



and here is the code that i wrote:

scope SphereOfNatureOvergrowth initializer Init

private function OvergrowthStart takes nothing returns nothing
    local timer t = GetExpiredTimer()
    local unit source
    local unit target
    local unit u
    local unit v
    local real duration
    local effect e
    local effect f
    local group g
    local group aux
 
    set source = LoadUnitHandle(udg_HeroHash, GetHandleId(t), 1)
    set target = LoadUnitHandle(udg_HeroHash, GetHandleId(t), 2)
    set duration = LoadReal(udg_HeroHash, GetHandleId(t), 3)
    set e = LoadEffectHandle(udg_HeroHash, GetHandleId(t), 4)
 
    call UnitDamageTarget(source, target, 500.0, true, false, ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC, null)
    if(UnitAlive(target) == false) then
        set g = CreateGroup()
        set aux = CreateGroup()
        call GroupEnumUnitsInRange(aux, GetUnitX(target), GetUnitY(target), 600.0, null)
        loop
            set u = FirstOfGroup(aux)
            exitwhen u == null
                if(IsUnitEnemy(u, GetOwningPlayer(source)) and UnitAlive(u) and u != target and IsUnitType(u, UNIT_TYPE_STRUCTURE) == false) then
                    call GroupAddUnit(g, u)
                endif
                call GroupRemoveUnit(aux, u)
        endloop
        set u = GetClosestUnitGroup(GetUnitX(target), GetUnitY(target), g)
        call GroupRemoveUnit(g, u)
        if(IsUnitType(u, UNIT_TYPE_HERO)) then
            set f = AddSpecialEffectTarget("OvergrowthTarget.mdx", u, "origin")
            call PauseUnit(u, true)
            call Overgrowth(source, u, 3.0, f)
        else
            set f = AddSpecialEffectTarget("OvergrowthTarget.mdx", u, "origin")
            call PauseUnit(u, true)
            call Overgrowth(source, u, 10.0, f)
        endif
        set v = GetClosestUnitGroup(GetUnitX(target), GetUnitY(target), g)
        call GroupRemoveUnit(g, v)
        if(IsUnitType(v, UNIT_TYPE_HERO)) then
            set f = AddSpecialEffectTarget("OvergrowthTarget.mdx", v, "origin")
            call PauseUnit(v, true)
            call Overgrowth(source, v, 3.0, f)
        else
            set f = AddSpecialEffectTarget("OvergrowthTarget.mdx", v, "origin")
            call PauseUnit(v, true)
            call Overgrowth(source, v, 10.0, f)
        endif
        call DestroyGroup(g)
        call DestroyGroup(aux)
        call DestroyEffect(e)
        call FlushChildHashtable(udg_HeroHash, GetHandleId(t))
        call PauseTimer(t)
        call DestroyTimer(t)
    else
        if(duration > 0) then
            call SaveReal(udg_HeroHash, GetHandleId(t), 3, duration - 1)
        else
            call PauseUnit(target, false)
            call DestroyEffect(e)
            call FlushChildHashtable(udg_HeroHash, GetHandleId(t))
            call PauseTimer(t)
            call DestroyTimer(t)
        endif
    endif
 
    set source = null
    set target = null
    set e = null
    set f = null
    set u = null
    set v = null
    set g = null
    set t = null
    set aux = null
endfunction

private function Overgrowth takes unit source, unit target, real duration, effect e returns nothing
    local timer t = CreateTimer()
 
    call SaveUnitHandle(udg_HeroHash, GetHandleId(t), 1, source)
    call SaveUnitHandle(udg_HeroHash, GetHandleId(t), 2, target)
    call SaveReal(udg_HeroHash, GetHandleId(t), 3, duration)
    call SaveEffectHandle(udg_HeroHash, GetHandleId(t), 4, e)
    call TimerStart(t, 1.00, true, function OvergrowthStart)
 
    set t = null
endfunction

private function Actions takes nothing returns nothing
    local effect e
 
    if(GetRandomInt(1,100) <= 20 and UnitAlive(udg_PDD_target) and IsUnitEnemy(udg_PDD_target, GetOwningPlayer(udg_PDD_source)) and IsUnitType(udg_PDD_target, UNIT_TYPE_STRUCTURE) == false) then
        set e = AddSpecialEffectTarget("OvergrowthTarget.mdx", udg_PDD_target, "origin")
        call PauseUnit(udg_PDD_target, true)
        if(IsUnitType(udg_PDD_target, UNIT_TYPE_HERO)) then
            call Overgrowth(udg_PDD_source, udg_PDD_target, 3.0, e)
        else
            call Overgrowth(udg_PDD_source, udg_PDD_target, 10.0, e)
        endif
    endif
 
    set e = null
endfunction

private function Conditions takes nothing returns boolean
    if (UnitHasItemOfType(udg_PDD_source, 'I04N') > 0 and udg_PDD_damageType == udg_PDD_PHYSICAL) then
        call Actions()
    endif
    return false
endfunction
//===========================================================================
private function Init takes nothing returns nothing
    set gg_trg_SphereOfNatureOvergrowth = CreateTrigger()
    call DisableTrigger(gg_trg_SphereOfNatureOvergrowth)
    call TriggerRegisterVariableEvent( gg_trg_SphereOfNatureOvergrowth, "udg_PDD_damageEventTrigger", EQUAL, 1.00 )
    call TriggerAddCondition( gg_trg_SphereOfNatureOvergrowth, Condition(function Conditions ))
endfunction

endscope

as you can see, i'm calling the function Overgrowth from within the OvergrowthStart if the unit with the effect dies, but thats not allowed because jass read from bottom to top and it gives me an error saying that the Overgrowth function is not declared. I'm a little short of ideas on how to solve this, and i do not want to use the entangle roots ability because i want the source unit to do the damage, not a dummy. Any ideas on how to do this? Thx.

 

[IcemanBo]

Just have read the text description, but you might try to look at keyword feature.

 

[chopinski]

Just have read the text description, but you might try to look at keyword feature.

Will do!

 

[LeP]

You can use vjass .evaluate.

 

[chopinski]

Just have read the text description, but you might try to look at keyword feature.

So, i tried that, and i get the error "Unexpected return value from a function that returns nothing". i even tried to use the example in the JassHelper and get the "Unexpected return value" error.

 

[chopinski]

You can use vjass .evaluate.

My functions return nothing, how can i modify them to fit this solution, considering that the first function is called from within a TimerStart function call?

 

[LeP]

Just do call Overgroth.evaluate(source, v, 10, f)

 

[Overfrost]

There are other solutions which I think is better than .evaluate:

// One
globals
	trigger Trig
endglobals
function OnExpire takes nothing returns nothing
	call TriggerEvaluate(Trig)
endfunction

function Overgrowth takes ... returns nothing
	// ...
	// use OnExpire as the timer's function
endfunction
function OvergrowthStart takes nothing returns nothing
	// ...
endfunction

function Init takes nothing returns nothing
	// ...
	// add this:
	call TriggerAddCondition(Trig, Condition(function OvergrowthStart))
endfunction


// Two
function OvergrowthPrep takes ... returns timer
	// ...
	return t
endfunction
// then every time you'd call Overgrowth, call this instead:
	call TimerStart(OvergrowthPrep(...), 1, true, function OvergrowthStart)
// should work when it's called inside OvergrowthStart


// Three
// run this textmacro instead of calling Overgrowth
//! textmacro OVERGROWTH takes SOURCE, TARGET, DUR, E
	// place what's inside Overgrowth here, replace the local statement with a set statement
//! endtextmacro
// the function Overgrowth itself can be rewritten to:
function Overgrowth takes unit source, unit target, real dur, effect e returns nothing
	local timer t = CreateTimer()
//! runtextmacro OVERGROWTH("source", "target", "dur", "e")
	set t = null
endfunction

And I think this is how the .evaluate version goes:

private keyword Overgrowth
private function OvergrowthStart //...
private function Overgrowth //...

 

[MyPad]

Try creating a code variable, (must be global) and initialize its' value in the initialization function (assigned to the callback function). The function Overgrowth would be placed on top of OvergrowthStart and Overgrowth will refer to the code variable instead of the function object.

globals
    code overGrowth = null

function OverGrowth()
    ....
    call TimerStart(timer, real, real, overGrowth)

// Can come from any initializer
function onInit()
    overGrowth = function OverGrowthStart

 

[Overfrost]

Try creating a code variable, (must to global) and initialize its' value in the initialization function (assigned to the callback function). The function Overgrowth would be placed on top of OvergrowthStart and Overgrowth will refer to the code variable instead of the function object.

Didn't think about the code variable. This should be the best solution.

If anyone is curious, I actually did a little fun experiment with it:

Spoiler

scope Tester initializer Init

globals
	code Code
	integer Count
endglobals

// Auxiliaries
//! textmacro DESTROY_EXPIRED_TIMER
		call PauseTimer(GetExpiredTimer())
		call DestroyTimer(GetExpiredTimer())
//! endtextmacro
function Print takes string s returns nothing
	call DisplayTextToPlayer(Player(0), 0, 0, s)
endfunction

function Start takes nothing returns nothing
	local real r = GetRandomReal(0.5, 1.5)
	set Count = GetRandomInt(2, 5)

	call Print("Starting a |cffffcc00" + I2S(Count) + "x|r countdown with |cffc3dbff" /*
		*/ + R2S(r) + "s|r interval.")
	call TimerStart(CreateTimer(), r, true, Code)

//! runtextmacro DESTROY_EXPIRED_TIMER()
endfunction

function OnExpire takes nothing returns nothing
	if (Count == 0) then
		call Start()
	//! runtextmacro DESTROY_EXPIRED_TIMER()
	else
		call Print("- |cffff0000" + I2S(Count) + "!|r")
		set Count = Count - 1
	endif
endfunction

function Init takes nothing returns nothing
	set Code = function OnExpire
	call TimerStart(CreateTimer(), 1, false, function Start)
endfunction

endscope

 

[LeP]

There are other solutions which I think is better than .evaluate:

// One
globals
    trigger Trig
endglobals
function OnExpire takes nothing returns nothing
    call TriggerEvaluate(Trig)
endfunction

function Overgrowth takes ... returns nothing
    // ...
    // use OnExpire as the timer's function
endfunction
function OvergrowthStart takes nothing returns nothing
    // ...
endfunction

function Init takes nothing returns nothing
    // ...
    // add this:
    call TriggerAddCondition(Trig, Condition(function OvergrowthStart))
endfunction


// Two
function OvergrowthPrep takes ... returns timer
    // ...
    return t
endfunction
// then every time you'd call Overgrowth, call this instead:
    call TimerStart(OvergrowthPrep(...), 1, true, function OvergrowthStart)
// should work when it's called inside OvergrowthStart


// Three
// run this textmacro instead of calling Overgrowth
//! textmacro OVERGROWTH takes SOURCE, TARGET, DUR, E
    // place what's inside Overgrowth here, replace the local statement with a set statement
//! endtextmacro
// the function Overgrowth itself can be rewritten to:
function Overgrowth takes unit source, unit target, real dur, effect e returns nothing
    local timer t = CreateTimer()
//! runtextmacro OVERGROWTH("source", "target", "dur", "e")
    set t = null
endfunction

And I think this is how the .evaluate version goes:

private keyword Overgrowth
private function OvergrowthStart //...
private function Overgrowth //...

And it's only 6x the amount of code in comparison to .evaluate; what a bargain.

Try creating a code variable, (must to global) and initialize its' value in the initialization function (assigned to the callback function). The function Overgrowth would be placed on top of OvergrowthStart and Overgrowth will refer to the code variable instead of the function object.

globals
    code overGrowth = null

function OverGrowth()
    ....
    call TimerStart(timer, real, real, overGrowth)

// Can come from any initializer
function onInit()
    overGrowth = function OverGrowthStart

for this specific case where you actually use a code variable for TimerStart this seems like a nice solution.

 

[Overfrost]

And it's only 6x the amount of code in comparison to .evaluate; what a bargain.

What matters is the end result of the code mate, aiming performance over file size and laziness. See what your luv .evaluate bloody compiles to.

From:

scope Test

function A takes nothing returns integer
	return B.evaluate(3)
endfunction

function B takes integer i returns integer
	return i*i + i
endfunction

endscope

Into:

//JASSHelper struct globals:
trigger array st___prototype3
integer f__result_integer
integer f__arg_integer1

endglobals

function sc___prototype3_execute takes integer i,integer a1 returns nothing
    set f__arg_integer1=a1

    call TriggerExecute(st___prototype3[i])
endfunction
function sc___prototype3_evaluate takes integer i,integer a1 returns integer
    set f__arg_integer1=a1

    call TriggerEvaluate(st___prototype3[i])
 return f__result_integer
endfunction


// scope Test begins

function A takes nothing returns integer
	return sc___prototype3_evaluate(1,3)
endfunction

function B takes integer i returns integer
	return i * i + i
endfunction

// scope Test ends


//Struct method generated initializers/callers:
function sa___prototype3_B takes nothing returns boolean
 local integer i=f__arg_integer1

    set f__result_integer= i * i + i
    return true
endfunction

function jasshelper__initstructs21135171 takes nothing returns nothing
    set st___prototype3[1]=CreateTrigger()
    call TriggerAddAction(st___prototype3[1],function sa___prototype3_B)
    call TriggerAddCondition(st___prototype3[1],Condition(function sa___prototype3_B))

endfunction

Now this is actually more than 6x the code, including extra unnecessary things. And look at how unoptimized it is. A bargain that I'd always decline.

 

[LeP]

What matters is the end result of the code mate, aiming performance over file size and laziness. S

Wrong. readable code is way more important than the resulting compiled code.
a duplicated function and virtually the same performance is something i take gladly for some high level features. and very hypothetically jasshelper could always get updated to compile it better which would mean no rewrite. Also the war3map.j is one of the best compressable file in the whole map archive.

 

[Overfrost]

Wrong. readable code is way more important than the resulting compiled code.

The players never need to read any code behind the game, yet they can suffer from misoptimizations of the programmer's code. One small performance loss is fine, but when there are a lot of them they pile up and can cause a noticeable effect on the game.

I've been playing games on a low-tier platform, and even in wc3 maps there can be a huge performance difference. That's why I value the end result more than readability.

Readability is important, yes. It allows the programmer to manage their code efficiently. But using .evaluate is just too much mate.

 

[chopinski]

The players never need to read any code behind the game, yet they can suffer from misoptimizations of the programmer's code. One small performance loss is fine, but when there are a lot of them they pile up and can cause a noticeable effect on the game.

I've been playing games on a low-tier platform, and even in wc3 maps there can be a huge performance difference. That's why I value the end result more than readability.

Readability is important, yes. It allows the programmer to manage their code efficiently. But using .evaluate is just too much mate.

Just to settle it down, IMO, i think both things are important. Overfrost is right when he says that the performance should be a goal when creating code, especially because the final user (the players) do not care what is happening underneath the game, but LeP is also right when he says that creating a readable code is important, especially if you are creating something that other programmers will end up seeing/using and its a good practice.

About the solutions, i thank you all for the attention and i just want to point out some things:

1- I tried this:

Try creating a code variable, (must to global) and initialize its' value in the initialization function (assigned to the callback function). The function Overgrowth would be placed on top of OvergrowthStart and Overgrowth will refer to the code variable instead of the function object.

globals
    code overGrowth = null

function OverGrowth()
    ....
    call TimerStart(timer, real, real, overGrowth)

// Can come from any initializer
function onInit()
    overGrowth = function OverGrowthStart

and it worked, even better than that, its a simple solution, which i think makes it the best solution.

2- I also tried the .evaluate solution, which works too, but i found out that there's also a .execute function in vJass, and turns out that .execute is less costy than .evaluate according to the JassHelper documentation. Its even better than the

TriggerExecute()

native function, so i'm not sure which one of these solutions is better performance wise.

 

[Overfrost]

Sorry we debated here mate.

The first one that you tried is the best. Using .execute is not too different than .evaluate. It's about the compiler using TriggerExecute or TriggerEvaluate when executing the code, they both use triggers to handle the call. But don't use any of them, seriously. Using triggers manually to emulate what they do is way better performance-wise.

 

[Chaosy]

Optimizing an inefficient game omegalul

 

[LeP]

Sorry we debated here mate.

The first one that you tried is the best. Using .execute is not too different than .evaluate. It's about the compiler using TriggerExecute or TriggerEvaluate when executing the code, they both use triggers to handle the call. But don't use any of them, seriously. Using triggers manually to emulate what they do is way better performance-wise.

But that is wrong. Manually calling TriggerEvaluate or using .evaluate is literally the same. Where do your magic performance gains come from when calling TriggerEvaluate? Also it's not even about performance. I guarantee you that if a map would lag with .evaluate it also lagg with TriggerEvaluate, beacuse, once again, they are virtually the same.

The best method in this case is the global code variable.

 

[chopinski]

Optimizing an inefficient game omegalul

Isn't it why we should always try to do things as optimal as possible?

 

[Wrda]

Isn't it why we should always try to do things as optimal as possible?

Because that's just a sick obsession that won't do much in long term. Specially the triggers that are executed once in a while, what's to point to "gotta get that +0.0000000001 performance bonus"? It's just wasted effort. Even if you want to be a speed freak, I'd suggest you to go code in another language. "inefficient game"

 

[chopinski]

Because that's just a sick obsession that won't do much in long term. Specially the triggers that are executed once in a while, what's to point to "gotta get that +0.0000000001 performance bonus"? It's just wasted effort. Even if you want to be a speed freak, I'd suggest you to go code in another language. "inefficient game"

Because its a good practice. Start doing shit stuff just because of some silly excuse like that and your programmer career will not go far. i'm not saying that the game is efficient, i do know how bad it is and because of that, its always good to do stuff as best as possible. One little shit might not seem like much but start adding little shits together and in the end you got a mountain of crap.

 

[Wrda]

Start doing shit stuff just because of some silly excuse like that and your programmer career will not go far.

Which programmers? Whose careers? Man you made me laugh. 99% of map makers of Wc3 are no real programmers nor have such a career. Most of them just make maps for fun, and you know what? There's a high chance that a map which was made by someone for fun and its code is kind of shit/messed up, still has no bugs and is one of the most popular/successful maps in the list. While there's also a chance that a map might have "god like" coding, the most "efficient and fastest performance" ever and still a complete garbage in the general idea of map gameplay.
You completely ignored the context where you should be focusing more on speed and "lagless" and where it isn't needed, and just brought a snowball effect argument, which is, again, false. That just sounds more like an excuse to cover up your other flaws.
A true programmer knows when he should focus more on speed and performance, like a system which is highly demanding and used, and when it isn't needed to go berserk. In any case, you still shouldn't sacrifice readability for speed, since then not only other people can't read and comprehend your code properly but also not EVEN you can understand it without effort and some time.
Also, TriggerEvaluate is faster than TriggerExecute... not sure if someone said the opposite, lol

 

[MyPad]

I'm sure the debate has gone on enough. To each his own.