[JASS] Simple trigger, high leak. Help pls.

[puffdragon]

//main
    set trigTimedLife_h099=CreateTrigger() 
	call TriggerRegisterAnyUnitEventBJ(trigTimedLife_h099,EVENT_PLAYER_UNIT_ATTACKED)
	call TriggerAddCondition(trigTimedLife_h099,Condition(function IsUnit_h099))
	call TriggerAddAction(trigTimedLife_h099,function Group_h099_TimedLife)

//functions
function IsUnit_h099 takes nothing returns boolean
return (GetUnitTypeId(GetTriggerUnit())=='h099')
endfunction

function Group_h099_TimedLife takes nothing returns nothing	
	if (((IsUnitInGroup(GetTriggerUnit(),group034))==false)and((IsUnitInGroup(GetTriggerUnit(),group035))==false)) then
	call GroupAddUnit(group034,GetTriggerUnit())
	call ForGroupBJ(group034,function h099TimedLife)
	endif
endfunction

function h099TimedLife takes nothing returns nothing
        if (GetUnitTypeId(GetEnumUnit())=='h099') then 
              call UnitApplyTimedLifeBJ(8.,'BTLF',GetEnumUnit())
        endif
        call GroupAddUnit(group035,GetEnumUnit())
endfunction

I GroupClear group034 and group035 after all enemies are dead, but this LEAKS so BAD. When I make 15-20 of this units game just crash out of memory (I think this is the leak or something else related to this unit >.<). h099 unit has explodes on death, (not tirggered, just suicide (Asds with long cast time to avoid dying immediately)

This is the ~10th version of this trigger, I use to make and destroy group but the leak was too high and so I tried to make its own TWO groups just for this unit so I just clear them instead.

I dont know what is going on... can anyone help?

 

[Bribe]

These functions aren't leaking. You'll need to post more for us to know, and it would be a good idea to post the original GUI instead of this converted stuff.

 

[Dehua_Darbuya]

It's quite difficult to grasp what this trigger is meant to do, and you didn't exactly say what it was meant to do either. If you could tell us, that'd be great. This is my interpretation of it in the form of pseudo-code:

• A unit is attacked
• Attacked unit is type h099
• If not in group34 and group35, add it to group034
• For group034, check if the picked unit is type h099
• If so, give it an 8 second timed life
• Add picked unit to group035

Edit: As Bribe stated, the functions don't leak so it's probably stemming from other triggers.

 

[TheLordOfChaos201]

when units die they leak

this if they are ordered to die via a trig

instead use the function of

unit- add generic timer that expires In 1 second

 

[puffdragon]

Ty guys for the response, unfortunately I have never worked with GUI, would take me ages to try to make that into the GUI...

is it possible that the memory leaks happen cause of the huge cast time?

All rest code related to that unit, I tried to put it in order they are used or close to that for easier reading:

//main
    set Trig_Cast_h099=CreateTrigger()
	call TriggerAddAction(Trig_Cast_h099,function Func_h099)


function Func_h099 takes nothing returns nothing 
// check if the type of this unit exists, create a group and add them, set their boolean as true
        call DestroyGroup(group027)
	set group027=Create_Group('h099')
	if(Count_h099())then
		set Booleans_Casts[15]=true				
	else
		set Booleans_Casts[15]=false		
	endif
endfunction


function Create_Group takes integer loc_integer01 returns group
//creates a group using unit id
	set group026=CreateGroup()
	call GroupEnumUnitsOfType(group026,UnitId2String(loc_integer01),null)
	return group026
endfunction

function Count_h099 takes nothing returns boolean
	return(CountUnitsInGroup(group027)>0)
endfunction

function Bool_h099 takes nothing returns boolean // **
	return(Booleans_Casts[15])
endfunction



//main
    set Trig_Casts=CreateTrigger()
	call TriggerRegisterTimerEventPeriodic(Trig_Casts,1.)
	call TriggerAddCondition(trig_casts,Condition(function Round_Boolean))
	call TriggerAddAction(Trig_Casts,function Casts_h099)



function Round_Boolean takes nothing returns boolean
//check if there is an ongoing round
        return (r_bool)
endfunction

function Casts_h099 takes nothing returns nothing
//checks if there are units made to teleport
	if(Bool_h099())then	// **			
		call ForGroupBJ(group027,function Teleport_h099) 
	endif
endfunction

function Teleport_h099 takes nothing returns nothing
//checks if there are enemies alive if not teleport to other location
   if(All_Enemies_Dead())then
      set locrect028=GetRandomLocInRect(rect028)
	  set location001=GetUnitLoc(GetEnumUnit())
	  call PauseUnit(GetEnumUnit(),true)
      call SetUnitPositionLoc(GetEnumUnit(),locrect028)
      call IssueImmediateOrderById(GetTriggerUnit(),851972)
      call PauseUnit(GetEnumUnit(),false)
      call AddSpecialEffectLocBJ(location001,"Abilities\\Spells\\Human\\MassTeleport\\MassTeleportTarget.mdl")
      call DestroyEffect(bj_lastCreatedEffect)
      call GroupClear(group027)
      call DestroyGroup(group027)
      call RemoveLocation(locrect028)
      call RemoveLocation(location001)	
   endif
endfunction

function All_Enemies_Dead takes nothing returns boolean
	return(CountUnitsInGroup(groups006[1+GetPlayerId(GetEnumPlayer())])==0)
endfunction

It's quite difficult to grasp what this trigger is meant to do, and you didn't exactly say what it was meant to do either. If you could tell us, that'd be great. This is my interpretation of it in the form of pseudo-code:

• A unit is attacked
• Attacked unit is type h099
• If not in group34 and group35, add it to group034
• For group034, check if the picked unit is type h099
• If so, give it an 8 second timed life
• Add picked unit to group035

Edit: As Bribe stated, the functions don't leak so it's probably stemming from other triggers.

I add them in both groups so game don't try to add timedlife twice (I had a much simpler version at first but I remade this 10 times...)

when units die they leak

this if they are ordered to die via a trig

instead use the function of

unit- add generic timer that expires In 1 second

If this is true then definitely is the problem, I use timelife to kill a bunch of those. When they are like 5-6 no problem but when they are 20+ game crash, but how could I go about to fixing this? Create a dummy under the player that cast a different spell not (Asds) at the location and then remove dummy elsewell? I mean to get kills credited to player.

Edit: added to the units does not decay, can't raise and then problem persists.

 

[Bribe]

unfortunately I have never worked with GUI, would take me ages to try to make that into the GUI...

It seems you've misunderstood me. There's no way you haven't worked in GUI before. Your triggers are GUI converted to JASS and you slightly changed a few names.

This is GUI:

• Trigger
  • Events
  • Conditions
  • Actions

 

[puffdragon]

@Bribe

Well I know my code is shitty, but thats cause I had a shitty teacher.... my apologies

 

[KILLCIDE]

Why are you not posting the whole code...? Are you also certain that this trigger is the cause of the crash?

 

[puffdragon]

well you want me to post 20k lines of code?

What I can tell for sure is that, this unit is somehow responsible for the cmemblock.cpp errors, cause game crash when I spam this specific units (like 20-30 of them). It doesn't happen with other units.

 

[KILLCIDE]

well you want me to post 20k lines of code?

Lol 20,000 lines of code? I think you're going to need to relook some of the things you've done because I find it hard to believe any type of system or spell in a Warcraft 3 map needing that many lines of code.

 

[puffdragon]

Lol 20,000 lines of code? I think you're going to need to relook some of the things you've done because I find it hard to believe any type of system or spell in a Warcraft 3 map needing that many lines of code.

wait... I thought you meant the entire war3map.j lol. Well this is the entire code of the spell, thats why its titled simple

Also the next post with code, is all other code related to that unit. Either 1st or 2nd post is screwing the game up, not sure what but am sure the game crash when this units start to die in numbers.

Ok, I will type the "concept"

1. Unit h099 has skill Asds (suicide), with 20sec cast time.
2. This unit has cargo ability elsewell and 0-0 attack.
3. After this unit is attacked once, the unit dies after 8seconds
4. Cause this unit has Asds ability when it dies explodes killing things around it.
5. Need this to be able to work for a large number of unit 50+, without cmemblock.cpp the game.

Another possible problem is that 20sec cast time. Unit tries to cast suicide on a target, target dies, unit tries to suicide again and again so MAYBE that causes the crash. Am really not sure, lags start tho after this unit start dying.

 

[KILLCIDE]

What you posted is most definitely not the "entire" code of the spell. I will wait for your post.

 

[puffdragon]

What you posted is most definitely not the "entire" code of the spell. I will wait for your post.

Indeed I do same for 2 other units, like this

function h099TimedLife takes nothing returns nothing
        if (GetUnitTypeId(GetEnumUnit())=='h099') then 
              call UnitApplyTimedLifeBJ(8.,'BTLF',GetEnumUnit())
       elseif (GetUnitTypeId(GetEnumUnit())=='h098') then 
              call UnitApplyTimedLifeBJ(7.,'BTLF',GetEnumUnit())
       elseif (GetUnitTypeId(GetEnumUnit())=='h097') then 
              call UnitApplyTimedLifeBJ(6.,'BTLF',GetEnumUnit())
        endif
        call GroupAddUnit(group035,GetEnumUnit())
endfunction

I don't see how that changes anything tho.

 

[Dr Super Good]

Please post the appropriate script from your map or provide a demonstration map where the problem can be easily recreated.

The scripts you posted above are not valid JASS. Ignoring the out of scope calls, you also reference functions above their declaration which is not allowed by JASS.

 

[IcemanBo]

This must be troll code. Noone will read code like it. If you are interested in an honest opinion:

function Func_h099 takes nothing returns nothing 
// check if the type of this unit exists, create a group and add them, set their boolean as true
        call DestroyGroup(group027)
    set group027=Create_Group('h099')
    if(Count_h099())then
        set Booleans_Casts[15]=true             
    else
        set Booleans_Casts[15]=false        
    endif
endfunction

Totaly useless function.
- Bad function and variable name
- Totaly hard coded (does only work with pre-defined variables)
- useless
Nobody will understand why such function exists. Use-functions should help against redundancy.

function Create_Group takes integer loc_integer01 returns group
//creates a group using unit id
    set group026=CreateGroup()
    call GroupEnumUnitsOfType(group026,UnitId2String(loc_integer01),null)
    return group026
endfunction

Just an other function with random variable names which are just pre-defined, and hardcoded. Useless.

function Count_h099 takes nothing returns boolean
    return(CountUnitsInGroup(group027)>0)
endfunction

Totaly useless. Use the function CountUnitsInGroup directly.
Also, hardcoded variables.

function Bool_h099 takes nothing returns boolean // **
    return(Booleans_Casts[15])
endfunction

Totaly useless. Use the boolean directly.

-----

Why all the shitty names? group026, you just know it's a group. Nothing more at all.
Same goes for all other variables and function names. They are not descriptive. Not at all.
You can work with names like "group g" and "unit u" and so on for local variables.. but doing it for ALL global ones is just a very bad habit.
You should change it asap.
And since you don't use "udg" prefix you likely use JNGP, and have access to JassHelper, which allows you things like globals and so on, which gives you chance to make good names.

I'm not sure you know good about paramters in JASS, because all these bad functions, and their names.
When you define a function you can also define arguments. This makes the function dynamic. Example:

Your code:

// For one group
function Count_h099 takes nothing returns boolean
    return(CountUnitsInGroup(group027)>0)
endfunction

// For second group
function Count_h100 takes nothing returns boolean
    return(CountUnitsInGroup(group028)>0)
endfunction

Example, for all groups:

function CountGroup takes group g returns boolean
    return(CountUnitsInGroup(g)>0)
endfunction

^You see. No hardcoded stuff inside the function, and no unclear names.
This was not a perfect example, because the function body can be used directly anyways, as it's only 1 line, but you see the sense of taking function arguments.

------

I'm not meant to seem rude, but you must learn to code properly. Especially when you are posting code in public.
People will be more willing to help you, if they comprehend the code. But besides this critique, I wish you good luck to find the bug.

 

[Bribe]

Not to mention a far easier way to check if the group is empty is "FirstOfGroup(g)!=null".

The crash is likely due to a "unit is issued an order" trigger that hits an infinite loop.

 

[puffdragon]

@IcemanBo
Well that looks much more neat indeed, problem is I learn by example cause I really don't have time to read jass, and I really have short attention span anyway. Your examples did help, on making me write cleaner code, nonetheless the code does do what I want it to do, if you exclude the cmemblock.cpp crash

Not to mention a far easier way to check if the group is empty is "FirstOfGroup(g)!=null".

The crash is likely due to a "unit is issued an order" trigger that hits an infinite loop.

But I specifically wrote it like that with 2 groups to make sure no orders are given twice for same unit, I even inserted BJDebugMsg on every step to make sure they are only called once >.< (on first code).

On second code, the teleport function does get a call spam but it stops when all enemies are dead else well. Not to mention, even when called it pretty much does nothing till conditions are met... (which are all enemies are dead)

 

[Bribe]

Yeah you likely have an infinite loop causing the crash or something else causing invalid data to attempt to be accessed causing the crash. You may want to share the map with someone to troubleshoot it, as if you disable these triggers you posted the game should still crash.

 

[puffdragon]

So, I actually pin pointed the moment (or at least one of them) that the game starts to memblock.

Its when function Teleport_h099 is called, cause its called for every single h099 alive at the same time,(even if function it practically "empty" till conditions are met). So ye, I don't really know how I will fix that yet.

function Teleport_h099 takes nothing returns nothing
//checks if there are enemies alive if not teleport to other location
   if(All_Enemies_Dead())then
      set locrect028=GetRandomLocInRect(rect028)
      set location001=GetUnitLoc(GetEnumUnit())
      call PauseUnit(GetEnumUnit(),true)
      call SetUnitPositionLoc(GetEnumUnit(),locrect028)
      call IssueImmediateOrderById(GetTriggerUnit(),851972)
      call PauseUnit(GetEnumUnit(),false)
      call AddSpecialEffectLocBJ(location001,"Abilities\\Spells\\Human\\MassTeleport\\MassTeleportTarget.mdl")
      call DestroyEffect(bj_lastCreatedEffect)
      call GroupClear(group027)
      call DestroyGroup(group027)
      call RemoveLocation(locrect028)
      call RemoveLocation(location001)  
   endif
endfunction

Anyone know how can I re-write this so it doesn't memblock the game when a 100 units of a group are called to execute that at the same time? (Doesn't even have to be exact same time)