[Trigger] Trigger causes game to crash after a certain game time

[Lockheart]

Below is the current trigger that is causing crashes after a certain time.
Any suggestions whats wrong with it? Tried to solve it on our own but it didn't work out.

Oh and: "Gleich" means "Same to", "Schleifen" means "Loop" and "Gegenstand" means "Item".

GoldInit

• GoldInit
  • Ereignisse
    • Zeit - Every 0.50 seconds of game time
  • Bedingungen
  • Aktionen
    • Set Map = (Entire map)
    • Gegenstand - Pick every item in Map and do (Actions)
      • Schleifen - Aktionen
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • 'IF'-Bedingungen
            • Or - Any (Conditions) are true
              • Bedingungen
                • (Item-type of (Picked item)) Gleich |cffffcc001 Gold|r
                • (Item-type of (Picked item)) Gleich |cffffcc002 Gold|r
                • (Item-type of (Picked item)) Gleich |cffffcc003 Gold|r
                • (Item-type of (Picked item)) Gleich |cffffcc004 Gold|r
                • (Item-type of (Picked item)) Gleich |cffffcc005 Gold|r
                • (Item-type of (Picked item)) Gleich |cffffcc006 Gold|r
                • (Item-type of (Picked item)) Gleich |cffffcc007 Gold|r
                • (Item-type of (Picked item)) Gleich |cffffcc008 Gold|r
                • (Item-type of (Picked item)) Gleich |cffffcc009 Gold|r
          • 'THEN'-Aktionen
            • Set Gold = (Picked item)
            • Set GoldPoint = (Position of Gold)
            • Set GoldDebug = (Center of GoldDebug <gen>)
            • Set GoldPickupRadius = (Units within 99.00 of (Position of Gold))
            • Auslöser - Run GoldPickup <gen> (ignoring conditions)
          • 'ELSE'-Aktionen
    • Custom script: call RemoveRect(udg_Map)

GoldPickup

• GoldPickup
  • Ereignisse
  • Bedingungen
  • Aktionen
    • Einheitengruppe - Pick every unit in GoldPickupRadius and do (Actions)
      • Schleifen - Aktionen
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • 'IF'-Bedingungen
            • ((Picked unit) Gleich HeroRed) or ((Picked unit) Gleich HeroBlue)
          • 'THEN'-Aktionen
            • Set PickingUnit = (Picked unit)
            • Gegenstand - Move Gold to GoldDebug
            • Held - Give Gold to PickingUnit
          • 'ELSE'-Aktionen

GoldDebug

• GoldDebug
  • Ereignisse
    • Einheit - A unit Erwirbt einen Gegenstand
  • Bedingungen
    • Or - Any (Conditions) are true
      • Bedingungen
        • (Item-type of (Item being manipulated)) Gleich |cffffcc001 Gold|r
        • (Item-type of (Item being manipulated)) Gleich |cffffcc002 Gold|r
        • (Item-type of (Item being manipulated)) Gleich |cffffcc003 Gold|r
        • (Item-type of (Item being manipulated)) Gleich |cffffcc004 Gold|r
        • (Item-type of (Item being manipulated)) Gleich |cffffcc005 Gold|r
        • (Item-type of (Item being manipulated)) Gleich |cffffcc006 Gold|r
        • (Item-type of (Item being manipulated)) Gleich |cffffcc007 Gold|r
        • (Item-type of (Item being manipulated)) Gleich |cffffcc008 Gold|r
        • (Item-type of (Item being manipulated)) Gleich |cffffcc009 Gold|r
  • Aktionen
    • Gegenstand - Move Gold to GoldDebug
    • Custom script: call RemoveLocation(udg_GoldPoint)
    • Custom script: call RemoveLocation(udg_GoldDebug)
    • Custom script: call DestroyGroup(udg_GoldPickupRadius)

Since I really don't know if this trigger can be set to work again I also requested one under Requests in case someone feels like submitting one.
I'm thankful for any help.

 

[IcemanBo]

Set Map = Entire Map only once in InitTrigger. Do not destroy it.

You don't need to run an seperated trigger in each loop. Simply add the actions into the first trigger.

Store ItemType into a variable because you use it more often.

Each time you PickUnitsInRange you leak an unit group.

You leak locations.

There is no need to PickEveryUnit if you only check it for two units. You directly could check distance of Hero Red/Blue and item.

Have a look on this system by BPower, maybe you want to use it: http://www.hiveworkshop.com/forums/spells-569/autopickupsystem-v-1-0-0-3-a-244610/

Read the tutorial "Things That Leak". You can find a link in my signature.

 

[Lockheart]

Thank you I'll try to think of what you said. I also thought about using the system you linked.
But to be honest I don't get how to configure it that way that it only picks gold for player 1 and 2 and nothing more.

I'm really not used to JASS.

Edit: I didn't copy the whole Trigger. I fixed it. Now the Trigger displays correctly.

 

[Kazeon]

Set Map = Entire Map only once in InitTrigger. Do not destroy it.

actually you don't need to store it into a variable

@Lockheart: guten abend her Lockheart
you did some mistake there..

1

• Set Gold = (Picked item)
• Set GoldPoint = (Position of Gold)
• Set GoldDebug = (Center of GoldDebug <gen>)
• Set GoldPickupRadius = (Units within 99.00 of (Position of Gold))
• Auslöser - Run GoldPickup <gen> (ignoring conditions)

==>

• Set Gold = (Picked item)
• Set GoldPoint = (Position of Gold)
• Set GoldDebug = (Center of GoldDebug <gen>)
• Set GoldPickupRadius = (Units within 99.00 of GoldPoint)
• Auslöser - Run GoldPickup <gen> (ignoring conditions)
• Custom script: call RemoveLocation(udg_GoldPoint)

2

• Gegenstand - Pick every item in Map and do (Actions)
• Schleifen - Aktionen
• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
• 'IF'-Bedingungen
• Or - Any (Conditions) are true
• Bedingungen
• (Item-type of (Picked item)) Gleich |cffffcc001 Gold|r
• (Item-type of (Picked item)) Gleich |cffffcc002 Gold|r
• (Item-type of (Picked item)) Gleich |cffffcc003 Gold|r
• (Item-type of (Picked item)) Gleich |cffffcc004 Gold|r
• (Item-type of (Picked item)) Gleich |cffffcc005 Gold|r
• (Item-type of (Picked item)) Gleich |cffffcc006 Gold|r
• (Item-type of (Picked item)) Gleich |cffffcc007 Gold|r
• (Item-type of (Picked item)) Gleich |cffffcc008 Gold|r
• (Item-type of (Picked item)) Gleich |cffffcc009 Gold|r
• 'THEN'-Aktionen
• Set Gold = (Picked item)
• Set GoldPoint = (Position of Gold)
• Set GoldDebug = (Center of GoldDebug <gen>)
• Set GoldPickupRadius = (Units within 99.00 of (Position of Gold))
• Auslöser - Run GoldPickup <gen> (ignoring conditions)
• 'ELSE'-Aktionen

==>

• Gegenstand - Pick every item in Map and do (Actions)
• Schleifen - Aktionen
• Set Gold = (Picked item)
• Set GoldType = (Item-type of Gold)
• If (All Conditions are True) then do (Then Actions) else do (Else Actions)
• 'IF'-Bedingungen
• Or - Any (Conditions) are true
• Bedingungen
• GoldType Gleich |cffffcc001 Gold|r
• GoldType Gleich |cffffcc002 Gold|r
• GoldType Gleich |cffffcc003 Gold|r
• GoldType Gleich |cffffcc004 Gold|r
• GoldType Gleich |cffffcc005 Gold|r
• GoldType Gleich |cffffcc006 Gold|r
• GoldType Gleich |cffffcc007 Gold|r
• GoldType Gleich |cffffcc008 Gold|r
• GoldType Gleich |cffffcc009 Gold|r
• 'THEN'-Aktionen
• Set GoldPoint = (Position of Gold)
• Set GoldDebug = (Center of GoldDebug <gen>)
• Set GoldPickupRadius = (Units within 99.00 of GoldPoint)
• Auslöser - Run GoldPickup <gen> (ignoring conditions)
• Custom script: call RemoveLocation(udg_GoldPoint)
• Custom script: call RemoveLocation(udg_GoldDebug)
• 'ELSE'-Aktionen

there still one another, it's the last trigger. the main point is storing repeated fcs in a variable first..

 

[IcemanBo]

Why is using a variable for entire map bad, especially when you use it very often?

Would you prefer GetEntireMapRect() each time? Or have already your variable initialized.

The same reason made you store item type into a variable.

 

[Lockheart]

It's getting a bit confusing to me :/...
Gonna try it out unless someone has further advices?

 

[Wrda]

You don't need to store GetEntireMapRect(), because it always returns the same rect and doesn't even leak. not a bad BJ.

 

[IcemanBo]

If you have a huge amounf of function call, you usually put the value into a variable on top of trigger.

So the reason is not always a leak, but also to prevent many needless further function calls.

If he runs this trigger each 0.5 seconds --> lot of function calls in a game --> I recommended to use a variable for it.

It's getting a bit confusing to me :/...
Gonna try it out unless someone has further advices?

You should try to solve it some more, if you are too annoyed you maybe really should try to use BPower's system.

In case you follow instructions and it's still not working you could ask him for help, why it's not working.

 

[Lockheart]

Well thanks kindly to all of you for your help. I'll make sure to get the best out of all your suggestions!

 

[Wrda]

It's not a problem calling it every 0.5 seconds. Spend creating a useless variable on this situation is weird.

 

[JaysProjex]

• GoldDebug
  • Ereignisse
  • Bedingungen
  • Aktionen
    • Custom script: call RemoveLocation(udg_GoldPoint)
    • Custom script: call RemoveLocation(udg_GoldDebug)
    • Custom script: call DestroyGroup(udg_GoldPickupRadius)

Since there is nothing "set" for it to destroy or remove in this trigger this should be used somewhere in the first trigger because it looks like you're stacking leaks.

Example: It's best to set a "point" then remove that "point" (with in the same trigger) before it ends.

 

[Kazeon]

Since there is nothing "set" for it to destroy or remove in this trigger this will cause your map to crash

not true..

 

[Zwiebelchen]

You always leak a "Position of Gold" for every gold item on the ground every 0.5 seconds.

Other than that, your trigger is extremely complicated for a simple thing to be achieved.

Also, it's prone to bugs, as you destroy a group and two locations whenever a unit picks up a gold item. If the item did not get picked up by the goldInit trigger, but by the actual unit instead (in other words: by players rightclicking it between two interations of goldInit), the GoldDebug trigger fires and removes handles that never got created, crashing the whole thread.