[JASS] Converted Jass to Proper Jass

[Rheiko]

Hello, so I had this custom exp system that I made in GUI. I made a lot of debug messages in the trigger. When I was done, I deleted the debug messages normally but after that, they still appeared in-game. So I converted it to Jass and deleted the functions manually. But I figured it can't be converted back to GUI so I want it to be at least efficient in my map and I'm no good with Jass.

Can anyone help me making this *more* efficient, pretty please?

Spoiler

function Trig_Exp_System_Conditions takes nothing returns boolean
    if ( not ( IsUnitType(GetTriggerUnit(), UNIT_TYPE_STRUCTURE) == false ) ) then
        return false
    endif
    return true
endfunction

function Trig_Exp_System_Func003002003001 takes nothing returns boolean
    return ( IsUnitType(GetFilterUnit(), UNIT_TYPE_HERO) == true )
endfunction

function Trig_Exp_System_Func003002003002001 takes nothing returns boolean
    return ( IsUnitEnemy(GetFilterUnit(), GetTriggerPlayer()) == true )
endfunction

function Trig_Exp_System_Func003002003002002 takes nothing returns boolean
    return ( GetOwningPlayer(GetFilterUnit()) != Player(PLAYER_NEUTRAL_PASSIVE) )
endfunction

function Trig_Exp_System_Func003002003002 takes nothing returns boolean
    return GetBooleanAnd( Trig_Exp_System_Func003002003002001(), Trig_Exp_System_Func003002003002002() )
endfunction

function Trig_Exp_System_Func003002003 takes nothing returns boolean
    return GetBooleanAnd( Trig_Exp_System_Func003002003001(), Trig_Exp_System_Func003002003002() )
endfunction

function Trig_Exp_System_Func005C takes nothing returns boolean
    if ( not ( IsUnitType(GetTriggerUnit(), UNIT_TYPE_HERO) == true ) ) then
        return false
    endif
    return true
endfunction

function Trig_Exp_System_Func006C takes nothing returns boolean
    if ( not ( IsUnitAlly(GetKillingUnitBJ(), GetTriggerPlayer()) == true ) ) then
        return false
    endif
    return true
endfunction

function Trig_Exp_System_Func007A takes nothing returns nothing
    call AddHeroXPSwapped( R2I(udg_XPS_Final), GetEnumUnit(), true )
endfunction

function Trig_Exp_System_Actions takes nothing returns nothing
    set udg_XPS_Loc = GetUnitLoc(GetTriggerUnit())
    set udg_XPS_Group = GetUnitsInRangeOfLocMatching(udg_XPS_Area, udg_XPS_Loc, Condition(function Trig_Exp_System_Func003002003))
    if ( Trig_Exp_System_Func005C() ) then
        set udg_XPS_Final = ( I2R(udg_XPS_XPH[GetHeroLevel(GetTriggerUnit())]) * ( I2R(udg_XPS_XPGainPH[CountUnitsInGroup(udg_XPS_Group)]) / 100.00 ) )
    else
        set udg_XPS_Final = ( I2R(udg_XPS_XPU[GetUnitLevel(GetTriggerUnit())]) * ( I2R(udg_XPS_XPGainPH[CountUnitsInGroup(udg_XPS_Group)]) / 100.00 ) )
    endif
    if ( Trig_Exp_System_Func006C() ) then
        set udg_XPS_Final = ( udg_XPS_Final * ( I2R(udg_XPS_XPGainDenyPerc) / 100.00 ) )
    else
    endif
    call ForGroupBJ( udg_XPS_Group, function Trig_Exp_System_Func007A )
    call DestroyGroup(udg_XPS_Group)
    call RemoveLocation(udg_XPS_Loc)
endfunction

//===========================================================================
function InitTrig_Exp_System takes nothing returns nothing
    set gg_trg_Exp_System = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Exp_System, EVENT_PLAYER_UNIT_DEATH )
    call TriggerAddCondition( gg_trg_Exp_System, Condition( function Trig_Exp_System_Conditions ) )
    call TriggerAddAction( gg_trg_Exp_System, function Trig_Exp_System_Actions )
endfunction

Thanks in advance.

 

[Jampion]

First you should ensure that your code is readable again. Converted GUI is horrible to read, mostly because of the way conditions are converted.

Make conditions only one line:

function Trig_Exp_System_Conditions takes nothing returns boolean
    if ( not ( IsUnitType(GetTriggerUnit(), UNIT_TYPE_STRUCTURE) == false ) ) then
        return false
    endif
    return true
endfunction

->

function Trig_Exp_System_Conditions takes nothing returns boolean
     return not IsUnitType(GetTriggerUnit(), UNIT_TYPE_STRUCTURE)
endfunction

And inline them:

function Trig_Exp_System_Func006C takes nothing returns boolean
    if ( not ( IsUnitAlly(GetKillingUnitBJ(), GetTriggerPlayer()) == true ) ) then
        return false
    endif
    return true
endfunction
...
    if ( Trig_Exp_System_Func006C() ) then
        set udg_XPS_Final = ( udg_XPS_Final * ( I2R(udg_XPS_XPGainDenyPerc) / 100.00 ) )
    else
    endif

->

    if ( IsUnitAlly(GetKillingUnitBJ(), GetTriggerPlayer()) ) then
        set udg_XPS_Final = ( udg_XPS_Final * ( I2R(udg_XPS_XPGainDenyPerc) / 100.00 ) )
    endif

After you have done these two things, your code will be much more readable. It will only increase performance by a little bit.
Now you should have 5 functions: init, actions, conditions, group and filter

Replace the bj's with natives where it makes sense.

Use coordinates (x,y) instead of locations.

You could replace the ForGroup loop with a FirstOfGroup loop.

In functions that use GetTriggerUnit() multiple times it can make sense to use a local variable at the beginning and store it:
local unit triggerUnit = GetTriggerUnit()

CountUnitsInGroup could be done directly inside the filter function.

Though since your system is not performance heavy, you should think at each step, if it is worth the effort to change it.

 

[Rheiko]

Thank you for the reply!

First you should ensure that your code is readable again. Converted GUI is horrible to read, mostly because of the way conditions are converted.

Exactly, that's why I want to make it readable and efficient if possible.

I haven't done everything as you said, still trying-
am I on a right track?

Spoiler

function Trig_Exp_System_Func003002003001 takes nothing returns boolean
    return ( IsUnitType(GetFilterUnit(), UNIT_TYPE_HERO) == true )
endfunction

function Trig_Exp_System_Func003002003002001 takes nothing returns boolean
    return ( IsUnitEnemy(GetFilterUnit(), GetTriggerPlayer()) == true )
endfunction

function Trig_Exp_System_Func003002003002002 takes nothing returns boolean
    return ( GetOwningPlayer(GetFilterUnit()) != Player(PLAYER_NEUTRAL_PASSIVE) )
endfunction

function Trig_Exp_System_Func003002003002 takes nothing returns boolean
    return GetBooleanAnd( Trig_Exp_System_Func003002003002001(), Trig_Exp_System_Func003002003002002() )
endfunction

function Trig_Exp_System_Func003002003 takes nothing returns boolean
    return GetBooleanAnd( Trig_Exp_System_Func003002003001(), Trig_Exp_System_Func003002003002() )
endfunction

function Trig_Exp_System_Func007A takes nothing returns nothing
    call AddHeroXPSwapped( R2I(udg_XPS_Final), GetEnumUnit(), true )
endfunction

function Trig_Exp_System_Conditions takes nothing returns boolean
    local unit u = GetTriggerUnit()
    if ( IsUnitType(u, UNIT_TYPE_STRUCTURE) == true ) then
        set udg_XPS_Loc = GetUnitLoc(u)
        set udg_XPS_Group = GetUnitsInRangeOfLocMatching(udg_XPS_Area, udg_XPS_Loc, Condition(function Trig_Exp_System_Func003002003))
        if ( IsUnitType(u, UNIT_TYPE_HERO) == true ) then
            set udg_XPS_Final = ( I2R(udg_XPS_XPH[GetHeroLevel(u)]) * ( I2R(udg_XPS_XPGainPH[CountUnitsInGroup(udg_XPS_Group)]) / 100.00 ) )
        else
            set udg_XPS_Final = ( I2R(udg_XPS_XPU[GetUnitLevel(u)]) * ( I2R(udg_XPS_XPGainPH[CountUnitsInGroup(udg_XPS_Group)]) / 100.00 ) )
        endif
        if ( IsUnitAlly(u, GetTriggerPlayer()) == true ) then
            set udg_XPS_Final = ( udg_XPS_Final * ( I2R(udg_XPS_XPGainDenyPerc) / 100.00 ) )
        endif
        call ForGroupBJ( udg_XPS_Group, function Trig_Exp_System_Func007A )
        call DestroyGroup(udg_XPS_Group)
        call RemoveLocation(udg_XPS_Loc)
    endif
    return false
endfunction

//===========================================================================
function InitTrig_Exp_System takes nothing returns nothing
    set gg_trg_Exp_System = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Exp_System, EVENT_PLAYER_UNIT_DEATH )
    call TriggerAddCondition( gg_trg_Exp_System, Condition( function Trig_Exp_System_Conditions ) )
endfunction

 

[Tasyen]

The filter functions should be fused into one function.

== true should be removed.

 

[Rheiko]

The filter functions should be fused into one function.

I know they should, I don't know how to do it.

== true should be removed.

Which one?

 

[Tasyen]

function Trig_Exp_System_Func003002003001 takes nothing returns boolean
    return ( IsUnitType(GetFilterUnit(), UNIT_TYPE_HERO) == true )
endfunction

function Trig_Exp_System_Func003002003002001 takes nothing returns boolean
    return ( IsUnitEnemy(GetFilterUnit(), GetTriggerPlayer()) == true )
endfunction

function Trig_Exp_System_Func003002003002002 takes nothing returns boolean
    return ( GetOwningPlayer(GetFilterUnit()) != Player(PLAYER_NEUTRAL_PASSIVE) )
endfunction

->

function EXPSystemFilter takes nothing returns boolean
   return IsUnitType(GetFilterUnit(), UNIT_TYPE_HERO) and IsUnitEnemy(GetFilterUnit(), GetTriggerPlayer() ) and GetOwningPlayer(GetFilterUnit() ) != Player(PLAYER_NEUTRAL_PASSIVE)
endfunction

Then you use this EXPSystemFilter inside your enumgroup thing.

After doing so you can remove this 2 FUnctions

function Trig_Exp_System_Func003002003002 takes nothing returns boolean
    return GetBooleanAnd( Trig_Exp_System_Func003002003002001(), Trig_Exp_System_Func003002003002002() )
endfunction

function Trig_Exp_System_Func003002003 takes nothing returns boolean
    return GetBooleanAnd( Trig_Exp_System_Func003002003001(), Trig_Exp_System_Func003002003002() )
endfunction