Hero evolution trigger

[UniEditXtreme]

I am having some major issues with a little code snippet I am trying to write. Basically, it is supposed to replace a hero with another when leveling up - just like in Creep Wars. My problem is that I am not simply replacing the unit, but also setting the new unit to the same level as the original. I have discovered that using SetHeroXP() or AddHeroXP() will crash the game unless I use this exact code; I cannot even replace one of my own variables with a build-in variable without it crashing.

I want this to use locals anyway so multiple heroes can evolve at the same time, but I cannot seem to make anything but this code work. This is my first "custom" Jass trigger, but I have read through and experimented with hundreds of others. If you all don't mind, a nice description of the problem would also be helpful; I want to know why the problem exists, not just how to fix it.

function Trig_Determine_Eligibility_Func007C takes nothing returns boolean
    if ( ( GetHeroLevel(GetLevelingUnit()) == 10 ) ) then
        return true
    endif
    if ( ( GetHeroLevel(GetLevelingUnit()) == 20 ) ) then
        return true
    endif
    if ( ( GetHeroLevel(GetLevelingUnit()) == 30 ) ) then
        return true
    endif
    if ( ( GetHeroLevel(GetLevelingUnit()) == 40 ) ) then
        return true
    endif
    return false
endfunction

function Trig_Determine_Eligibility_Conditions takes nothing returns boolean
    if ( not Trig_Determine_Eligibility_Func007C() ) then
        return false
    endif
    if (GetUnitUserData(GetLevelingUnit()) == 1) then
        return false
    endif
    return true
endfunction

function Trig_Determine_Eligibility_Func005001001 takes nothing returns boolean
    return ( GetPlayerController(GetFilterPlayer()) == MAP_CONTROL_USER )
endfunction

function Trig_Determine_Eligibility_Actions takes nothing returns nothing
    local boolean oldUnitSelected = IsUnitSelected(GetLevelingUnit(), GetOwningPlayer(GetLevelingUnit()))
    local integer oldExperience = GetHeroXP(GetLevelingUnit())
    local player oldPlayer = GetOwningPlayer(GetLevelingUnit())
    local location oldPosition = GetUnitLoc(GetLevelingUnit())
    local real oldRotation = GetUnitFacing(GetLevelingUnit())
    local integer oldUnitType = GetUnitTypeId(GetLevelingUnit())
    local unit newUnit
    local unit oldUnit = GetLevelingUnit()
    
    call ShowUnitHide( oldUnit )
    if ( ( oldUnitType == 'H009' ) == true ) then
        set newUnit = CreateUnitAtLoc( oldPlayer, 'H00A', oldPosition, oldRotation )
    else
        set newUnit = CreateUnitAtLoc( oldPlayer, 'H00A', oldPosition, oldRotation )
    endif
    
    call SetUnitUserData( newUnit, 1 )
    call AddHeroXP( newUnit, oldExperience, true )
    call SetUnitUserData( newUnit, 0 )
    if ( oldUnitSelected == true) then
        call SelectUnitForPlayerSingle( newUnit, oldPlayer )
    endif
    call RemoveUnit( oldUnit )
endfunction

//===========================================================================
function InitTrig_Determine_Eligibility takes nothing returns nothing
    set gg_trg_Determine_Eligibility = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Determine_Eligibility, EVENT_PLAYER_HERO_LEVEL )
    call TriggerAddCondition( gg_trg_Determine_Eligibility, Condition( function Trig_Determine_Eligibility_Conditions ) )
    call TriggerAddAction( gg_trg_Determine_Eligibility, function Trig_Determine_Eligibility_Actions )
endfunction

Just in case, this is the original code I had. This should replicate the problem.

function Trig_Determine_Eligibility_Func007C takes nothing returns boolean
    if ( ( GetHeroLevel(GetLevelingUnit()) == 10 ) ) then
        return true
    endif
    if ( ( GetHeroLevel(GetLevelingUnit()) == 20 ) ) then
        return true
    endif
    if ( ( GetHeroLevel(GetLevelingUnit()) == 30 ) ) then
        return true
    endif
    if ( ( GetHeroLevel(GetLevelingUnit()) == 40 ) ) then
        return true
    endif
    return false
endfunction

function Trig_Determine_Eligibility_Conditions takes nothing returns boolean
    if ( not Trig_Determine_Eligibility_Func007C() ) then
        return false
    endif
    return true
endfunction

function Trig_Determine_Eligibility_Func005001001 takes nothing returns boolean
    return ( GetPlayerController(GetFilterPlayer()) == MAP_CONTROL_USER )
endfunction

function Trig_Determine_Eligibility_Actions takes nothing returns nothing
    local boolean oldUnitSelected = IsUnitSelected(GetLevelingUnit(), GetOwningPlayer(GetLevelingUnit()))
    local integer oldExperience = GetHeroXP(GetLevelingUnit())
    local player oldPlayer = GetOwningPlayer(GetLevelingUnit())
    local location oldPosition = GetUnitLoc(GetLevelingUnit())
    local real oldRotation = GetUnitFacing(GetLevelingUnit())
    local integer oldUnitType = GetUnitTypeId(GetLevelingUnit())
    local unit newUnit
    
    call RemoveUnit( oldUnit )
    if ( ( oldUnitType == 'H009' ) == true ) then
        set newUnit = CreateUnitAtLoc( oldPlayer, 'H00A', oldPosition, oldRotation )
    else
        set newUnit = CreateUnitAtLoc( oldPlayer, 'H00A', oldPosition, oldRotation )
    endif
    
    call AddHeroXP( newUnit, oldExperience, true )
    if ( oldUnitSelected == true) then
        call SelectUnitForPlayerSingle( newUnit, oldPlayer )
    endif
endfunction

//===========================================================================
function InitTrig_Determine_Eligibility takes nothing returns nothing
    set gg_trg_Determine_Eligibility = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Determine_Eligibility, EVENT_PLAYER_HERO_LEVEL )
    call TriggerAddCondition( gg_trg_Determine_Eligibility, Condition( function Trig_Determine_Eligibility_Conditions ) )
    call TriggerAddAction( gg_trg_Determine_Eligibility, function Trig_Determine_Eligibility_Actions )
endfunction

Now, I also want to explain why I am doing (or not doing) certain things here. I am not replacing the original unit with the ReplaceUnitBJ() function because it also causes a crash, but the working code is basically a working hybrid between my original code and the ReplaceUnitBJ(). I also have the unit types in as literals right now, but I will put in an array of units for it to loop through for readability when I have other unit types created. The last major thing I can think of is that I am using custom values to ensure the new unit doesn't go through this trigger and create an infinite loop when it levels up, but I know there was some talk about it leaking. I am very interested in hearing about alternatives.

 

[deathismyfriend]

Check out my tutorial converting GUI to efficient jass. The jass script you have now is very inefficient.

As for your problem do getherolvl and setherolvl. I'm not sure if those are the exact names not on pc. If you have JNGP you can check in the function list.

 

[Spartipilo]

I'll just do ONE EXAMPLE to let you know better coding practices. JASS is an interpretative language: More text means more time to read and process, that's why it's better to keep text to a minimum and be as less verbose as possible.

Your whole current script (longer, unneficient, leaky) can be done this way: shorter, faster, better, leakless. Conditions and Actions are merged in the same function and there are no bj's.

function Trig_Determine_Eligibility takes nothing returns boolean
    local unit u = GetTriggerUnit() // Leveling Unit
    local integer lvl = GetHeroLevel(u) // Leveling Unit Level
    local boolean selected
    local integer exp
    local player p
    local real x
    local real y
    local real facing
    local integer utid
    
    if lvl == 10 or lvl == 20 or lvl == 30 or lvl == 40 then
        set p = GetOwningPlayer(u)
        set selected = IsUnitSelected(u, p)
        set exp = GetHeroXP(u)
        set x = GetUnitX(u)
        set y = GetUnitY(u)
        set facing = GetUnitFacing(u)
        set utid = GetUnitTypeId(u)
    
        call RemoveUnit(u)
        if utid == 'H009' then
            call BJDebugMsg("utid == 'H009'")
            set u = CreateUnit(p, 'H00A', x, y, facing)
        else
            call BJDebugMsg("utid NOT EQUAL to 'H009'")
        endif
        
//        call SetHeroXP(u, exp, true) // Uncomment to test
//        call SetHeroLevel(u, lvl) // Uncomment to test

        if GetLocalPlayer() == p then
            call ClearSelection()
            call SelectUnit(u, true)
        endif
    
        set p = null
    endif
    
    set u = null
    return false
endfunction

//===========================================================================
function InitTrig_Determine_Eligibility takes nothing returns nothing
    set gg_trg_Determine_Eligibility = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Determine_Eligibility, EVENT_PLAYER_HERO_LEVEL )
    call TriggerAddCondition( gg_trg_Determine_Eligibility, Condition( function Trig_Determine_Eligibility ) )
endfunction

 

[deathismyfriend]

Thanks spartipilo.
You should've used local trigger in the init trig function tho. Unless he plans on turning the trigger on and off.

Also show him the correct naming of functions when using jass.

globalVariablesLikeThis, FunctionNamesLikeThis, CONSTANT_VARIABLES_LIKE_THIS

Are there any more jass ones that i am forgetting about ?

 

[Spartipilo]

I'm not sure....

He was using the variable names to keep order of stuff (oldUnit, newUnit) but he could use (u1, u2) and comments to keep track of their meaning.

 

[UniEditXtreme]

deathismyfriend, I am coming out of a lot of C programming, so I keep using C naming conventions and stuff. Thank you for making me aware of the fact that naming conventions are different. I got the current function names from the "Convert to Custom Script" menu item though, so I guess I did not even think to check them.

Spartipilo, that looks a lot better. Thanks! Just one question: do I have to clear the player's selection before adding a unit to it? My goal was to make it a flawless transition, so the player does not have to re-select everyone and redo control groups.

 

[deathismyfriend]

i believe that is why sprtipilo has clear selection then he calls selectunit

Also i don't know any of C i just believe when someone asks for help you show them the right way and correct them were they were wrong. Naming conventions is a very easy thing to miss tho.

 

[Spartipilo]

You can test with or without ClearSelection() and see which one you like the most. If you don't use the ClearSelection() the change is unnoticeable even if your hero is selected amongs other units (But the unit still stop it's current order, so it will stop whatever it's doing on level up event), if you use ClearSelection() it will deselect everything and select only your hero. If you only have your hero selected, and you don't use ClearSelection() it will still be unnoticeable.

btw... you still have to transfer the items from old one to new one.

 

[UniEditXtreme]

I've just tested your code. It removes the hero and does not replace it. When I put in an else clause at the "if utid == 'H009' then" and create a hero, which makes the if thing redundant, the game crashes. I have narrowed it down to the SetHeroXP() function - it crashes the game every time it is called, but everything works if I use GUI.

 

[Spartipilo]

- Check the New script, made some changes in it.

- Since the unit is leveling up, try not to add experience, but set the level.

- If the new unit is not being created it's because:
a. The conditions aren't met (utid != 'H009') or
b. The unit id 'H00A' doesn't exist.

- Are you sure these are zeros and not o's in the Unit Type ID?

I also have the unit types in as literals right now, but I will put in an array of units for it to loop through for readability when I have other unit types created. The last major thing I can think of is that I am using custom values to ensure the new unit doesn't go through this trigger and create an infinite loop when it levels up, but I know there was some talk about it leaking. I am very interested in hearing about alternatives.

Forget about Loop (Not needed) and forget about manipulating Custom Value (it messess with Unit Indexer). Use a Hashtable and store the NewUnit id into the UnitTypeID of the leveling unit using the level as an index for replacement.

globals
    hashtable SpartJob_Hash = InitHashtable()
endglobals

function SpartJob_Data takes integer BaseUnitId, integer Level, integer NewUnitId returns nothing
    call SaveInteger(SpartJob_Hash, BaseUnitId, Level, NewUnitId)
endfunction

function SpartJob_SetData takes nothing returns nothing
    call SpartJob_Data('hpea', 10, 'hfoo') // Peasant turns to Footman on lvl 10
    call SpartJob_Data('hfoo', 20, 'hkni') // Footman turns to Knight on Level 20.
    call SpartJob_Data('hkni', 30, 'Hpal') // Knight turns to Paladin on Level 30.
    // and so on...
endfunction

function SpartJob_Change takes nothing returns nothing
    local unit u = GetTriggerUnit() //Leveling Unit
    local integer uid = GetUnitTypeId(u) // Unit Type ID of Leveling Unit
    local integer ulvl = GetHeroLevel(u) // Unit Level
    local integer NewId = LoadInteger(SpartJob_Hash, uid, ulvl) // Id of the replacement, if there's any on that level
    local unit New
    local integer index
    local item indexItem
    
    if NewId != 0 then
        // New Unit
        set New = CreateUnit(GetTriggerPlayer(), NewId, GetUnitX(u), GetUnitY(u), GetUnitFacing(u))
        
        // Transfer Level
        call SetHeroLevel(New, ulvl, true)
        
        // Transfer Items
        set index = 0
        loop
            exitwhen index > 6
            set indexItem = UnitItemInSlot(u, index)
            call UnitAddItem(New, indexItem)
            set index = index + 1
        endloop
        
        // Remove Old Unit
        call RemoveUnit(u)
        
        // Clear Leaks
        set New = null
    endif
    
    // Clear Leaks
    set u = null
endfunction

//===========================================================================
function InitTrig_SpartJobChange takes nothing returns nothing
    local trigger t = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_HERO_LEVEL )
    call TriggerAddCondition( t, Condition( function SpartJob_Change ) )
    set t = null
    call SpartJob_SetData()
endfunction

The reason with it crashes with the If then Else it's because If the utid doesn't match, ti creates the unit anyway and set it's level to 10, it triggers again, and if utid doesn't match, it creates the unit and set it's level to 10, it triggers again... and so on...

 

[UniEditXtreme]

The reason with it crashes with the If then Else it's because If the utid doesn't match, ti creates the unit anyway and set it's level to 10, it triggers again, and if utid doesn't match, it creates the unit and set it's level to 10, it triggers again... and so on...

Exactly. That is why I was using the custom value. If you check my original code (I think it is the first one) you will see that it only considers units with a custom value of 0. The new unit gets a custom value of 1 before he levels up, so he cannot be replaced by another dude, then his custom value get set back to 0 after the SetHeroXP() function.

 

[UniEditXtreme]

Alright, I got it working exactly how I wanted after a few minor modifications. Thanks very much, both of you, for helping me out here. I have learned a lot and I hope I can return the favor at some point.

 

[deathismyfriend]

I didn't help that much. If you look a my tutorial it should be able to help you more.

Also please don't double post instead hit the edit button on your last post.

 

[Spartipilo]

Post your final code so we can see what worked.