[Solved] Problem with 'Kill x enemies' Trigger

[skillzmfg3]

Recently I've been trying to add a Trigger to a map I've made that would create a Quest to kill a certain number of units in order to complete it.

At first sight everything was working properly but now it seems that I can't put the quest in it's final, completed stage. It seems like my array integer doesn't count the kills properly and just never stops to actually complete the quest.

This is the first time I'm doing such quests so I expect the error to be something stupid, but I just can't see it. I've looked up a few guides and none of them helped me. Hope anyone has an idea how to make this work.

Here's the trigger:

• [/B]Lothaleven Kill Q Count
  • Events
    • Unit - A unit owned by Player 12 (Brown) Dies
  • Conditions
    • And - All (Conditions) are true
      • Conditions
        • LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] Equal to started
        • ((Killing unit) is A Hero) Equal to True
        • (Unit-type of (Dying unit)) Not equal to Healing Ward
        • (Unit-type of (Dying unit)) Not equal to Stasis Trap
        • (Unit-type of (Dying unit)) Not equal to Watcher Ward
        • ((Dying unit) is Summoned) Equal to False
        • (Unit-type of (Dying unit)) Not equal to Sentry Ward
        • ((Dying unit) is A structure) Equal to False
        • LothalevenKillCount[(Player number of (Owner of (Killing unit)))] Less than 10
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] Equal to started
      • Then - Actions
        • Set LothalevenKillCount[(Player number of (Owner of (Triggering unit)))] = LothalevenKillCount[(LothalevenKillCount[(Player number of (Owner of (Triggering unit)))] + 1)]
        • Game - Display to (Player group((Owner of (Killing unit)))) the text: |c00FFFF00Quest Upd...
      • Else - Actions
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • LothalevenKillCount[(Player number of (Owner of (Killing unit)))] Equal to 10
          • Then - Actions
            • Game - Display to (Player group((Owner of (Killing unit)))) the text: |c00FFFF00Quest Upd...
            • Set LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] = completed
            • Cinematic - Ping minimap for (Player group((Owner of (Killing unit)))) at (Position of Expedition Leaders 1061 <gen>) for 10.00 seconds, using a Flashy ping of color (100.00%, 100.00%, 100.00%)
          • Else - Actions
            • Skip remaining actions
        • Do nothing[B][trigger][/B][/INDENT]

 

[Pyrogasm]

Do Nothing should never ever be used. It literally does nothing. Skip Remaining Actions is kind of pointless here because nothing would naturally happen after that action is taken anyway.

There are two problems in the trigger:

1. This line uses Triggering Unit where it should use Killing Unit: Set LothalevenKillCount[(Player number of (Owner of (Triggering unit)))] = LothalevenKillCount[(LothalevenKillCount[(Player number of (Owner of (Triggering unit)))] + 1)]
2. The quest can never complete because once it starts then the first IF statement will always be true and it will just keep incrementing the kill counter every time without checking to see if its >= 10 (since it never goes to the first statement's ELSE. To fix this you will want to move the entire second IF-THEN-ELSE block to right after the Set LothalevenKillCount line in the first IF block, and then move the Game - Display line right below that line to inside the 2nd If's ELSE block where the Skip action is right now so you only see one update message per kill (instead of 2 on the 10th kill)
3. For good practice it's generally best to check >= 10 rather than == 10 in case something happens to your integer variable and it goes up to 11.

 

[IcemanBo]

Just a hint. The code is a bit painfull to read with the bold format, at least on my screen.

 

[skillzmfg3]

Do Nothing should never ever be used. It literally does nothing. Skip Remaining Actions is kind of pointless here because nothing would naturally happen after that action is taken anyway.

There are two problems in the trigger:

1. This line uses Triggering Unit where it should use Killing Unit: Set LothalevenKillCount[(Player number of (Owner of (Triggering unit)))] = LothalevenKillCount[(LothalevenKillCount[(Player number of (Owner of (Triggering unit)))] + 1)]
2. The quest can never complete because once it starts then the first IF statement will always be true and it will just keep incrementing the kill counter every time without checking to see if its >= 10 (since it never goes to the first statement's ELSE. To fix this you will want to move the entire second IF-THEN-ELSE block to right after the Set LothalevenKillCount line in the first IF block, and then move the Game - Display line right below that line to inside the 2nd If's ELSE block where the Skip action is right now so you only see one update message per kill (instead of 2 on the 10th kill)
3. For good practice it's generally best to check >= 10 rather than == 10 in case something happens to your integer variable and it goes up to 11.

Either I don't understand what you're trying to say or the this doesn't fix the trigger. I've tried following your instruction, but then the counter doesn't act at all. Also, I've experimented some more myself and I got a few scenarios. In the first the counter doesn't work at all and doesn't recognize the kills in any way. In the second I get the infinite loop of 'quest updates' and never the actual completion of the quest. The third one consists of both quest update and quest completion appearing any time I kill a quest unit. I'll upload the map here so you can take a look at it yourself and hopefully fix it if you have the time. I'd be very thankful since I wasted about 4 hours on this alone already.
The trigger in question is under 'Triggers>PvP Quests>Lothaleven Kill Q Count' and to start the quest just approach the Mortar Team unit near the initial location of the Hero I've placed for testing. After you've picked the quest up, just go straight south to reach the encampment where the quest units are.

Just a hint. The code is a bit painfull to read with the bold format, at least on my screen.

I've removed the bold format from the trigger section of the post, hopefully it's easier to read now.

 

[Pyrogasm]

The trigger I see in that map is the same as the one you posted above, so I don't see anything you did to change it. This trigger should work correctly:

• Lothaleven Kill Q Count
  • Events
    • Unit - A unit owned by Player 12 (Brown) Dies
  • Conditions
    • And - All (Conditions) are true
      • Conditions
        • LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] Equal to started
        • ((Killing unit) is A Hero) Equal to True
        • (Unit-type of (Dying unit)) Not equal to Healing Ward
        • (Unit-type of (Dying unit)) Not equal to Stasis Trap
        • (Unit-type of (Dying unit)) Not equal to Watcher Ward
        • ((Dying unit) is Summoned) Equal to False
        • (Unit-type of (Dying unit)) Not equal to Sentry Ward
        • ((Dying unit) is A structure) Equal to False
        • LothalevenKillCount[(Player number of (Owner of (Killing unit)))] Less than 10
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] Equal to started
      • Then - Actions
        • Set LothalevenKillCount[(Player number of (Owner of (Killing unit)))] = LothalevenKillCount[(LothalevenKillCount[(Player number of (Owner of (Killing unit)))] + 1)]
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • LothalevenKillCount[(Player number of (Owner of (Killing unit)))] Greater than or equal to 10
          • Then - Actions
            • Game - Display to (Player group((Owner of (Killing unit)))) the text: |c00FFFF00Quest Upd...
            • Set LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] = completed
            • Cinematic - Ping minimap for (Player group((Owner of (Killing unit)))) at (Position of Expedition Leaders 1061 <gen>) for 10.00 seconds, using a Flashy ping of color (100.00%, 100.00%, 100.00%)
          • Else - Actions
            • Game - Display to (Player group((Owner of (Killing unit)))) the text: |c00FFFF00Quest Upd...
      • Else - Actions

I also see some point leaks and player group leaks, which you can learn how to clean in this tutorial: Things That Leak

 

[skillzmfg3]

The trigger I see in that map is the same as the one you posted above, so I don't see anything you did to change it. This trigger should work correctly:

• Lothaleven Kill Q Count
  • Events
    • Unit - A unit owned by Player 12 (Brown) Dies
  • Conditions
    • And - All (Conditions) are true
      • Conditions
        • LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] Equal to started
        • ((Killing unit) is A Hero) Equal to True
        • (Unit-type of (Dying unit)) Not equal to Healing Ward
        • (Unit-type of (Dying unit)) Not equal to Stasis Trap
        • (Unit-type of (Dying unit)) Not equal to Watcher Ward
        • ((Dying unit) is Summoned) Equal to False
        • (Unit-type of (Dying unit)) Not equal to Sentry Ward
        • ((Dying unit) is A structure) Equal to False
        • LothalevenKillCount[(Player number of (Owner of (Killing unit)))] Less than 10
  • Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] Equal to started
      • Then - Actions
        • Set LothalevenKillCount[(Player number of (Owner of (Killing unit)))] = LothalevenKillCount[(LothalevenKillCount[(Player number of (Owner of (Killing unit)))] + 1)]
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • LothalevenKillCount[(Player number of (Owner of (Killing unit)))] Greater than or equal to 10
          • Then - Actions
            • Game - Display to (Player group((Owner of (Killing unit)))) the text: |c00FFFF00Quest Upd...
            • Set LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] = completed
            • Cinematic - Ping minimap for (Player group((Owner of (Killing unit)))) at (Position of Expedition Leaders 1061 <gen>) for 10.00 seconds, using a Flashy ping of color (100.00%, 100.00%, 100.00%)
          • Else - Actions
            • Game - Display to (Player group((Owner of (Killing unit)))) the text: |c00FFFF00Quest Upd...
      • Else - Actions

I also see some point leaks and player group leaks, which you can learn how to clean in this tutorial: Things That Leak

Well yeah I just didn't save the changes since they did nothing to fix my problem and I didn't want to confuse anyone trying to fix it by completely changing the trigger itself. Therefore I just removed the changes and reversed to the starting point.

EDIT: After copying and testing the trigger suggested by Pyrogasm I still have the same problem of kill count variable never reaching its final state. If anyone has other suggestions I would appreciate them very much!

 

[Pyrogasm]

Hmmm okay here's another round of fixes:

• Set LothalevenKillCount[TempInt] = LothalevenKillCount[(LothalevenKillCount[(Player number of (Owner of (Killing unit)))] + 1)]
  This line has an erroneous double LothalevenKillCount[] so it's not being updated correctly. Should be:
  Set LothalevenKillCount[TempInt] = (LothalevenKillCount[(Player number of (Owner of (Killing unit)))] + 1)
  
  
• The entire outer IF block is unnecessary since it just checks if the quest is started and that's already a condition for the trigger to run in the first place.
  
  
• All trigger conditions use AND by default so there's no reason to use that extra AND grouping. In addition they check each condition in order and immediately stop once one is false so you can put the most common ones at the top to save some code execution time (not much though). I moved the structure check up.
  
  
• There's no need to check that the killcount is < 10 in the trigger conditions because once it gets to be >=10 the quest is no longer active so it fails at the conditions anyway.
  
  
• It might be easier to read/simpler to store the player ID for array indices into a variable instead of computing it every time; I called it TempInt below
  
  
• I've fixed the point and player group leaks.

• Lothaleven Kill Q Count
  • Events
    • Unit - A unit owned by Player 12 (Brown) Dies
  • Conditions
    • LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] Equal to started
    • ((Killing unit) is A Hero) Equal to True
    • ((Dying unit) is A structure) Equal to False
    • (Unit-type of (Dying unit)) Not equal to Healing Ward
    • (Unit-type of (Dying unit)) Not equal to Stasis Trap
    • (Unit-type of (Dying unit)) Not equal to Watcher Ward
    • (Unit-type of (Dying unit)) Not equal to Sentry Ward
    • ((Dying unit) is Summoned) Equal to False
  • Actions
    • Set TempInt = (Player number of (Owner of (Killing unit)))
    • Set LothalevenKillCount[TempInt] = (LothalevenKillCount[TempInt] + 1)
    • Set TempPG = (Player group(Player(TempInt)))
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • LothalevenKillCount[TempInt] Greater than or equal to 10
      • Then - Actions
        • Set LothalevenPVPQ[TempInt] = completed
        • Game - Display to TempPG the text: |c00FFFF00Quest Upd...
        • Set TempPoint = (Position of Expedition Leaders 1061 <gen>)
        • Cinematic - Ping minimap for TempPG at TempPoint for 10.00 seconds, using a Flashy ping of color (100.00%, 100.00%, 100.00%)
        • Custom script: call RemoveLocation(udg_TempPoint) //don't forget to change the variable name here to match yours but keep the udg_ prefix
      • Else - Actions
        • Game - Display to TempPG the text: |c00FFFF00Quest Upd...
    • Custom script: call DestroyForce(udg_TempPG) //same here

 

[skillzmfg3]

Hmmm okay here's another round of fixes:

• Set LothalevenKillCount[TempInt] = LothalevenKillCount[(LothalevenKillCount[(Player number of (Owner of (Killing unit)))] + 1)]
  This line has an erroneous double LothalevenKillCount[] so it's not being updated correctly. Should be:
  Set LothalevenKillCount[TempInt] = (LothalevenKillCount[(Player number of (Owner of (Killing unit)))] + 1)
  
  
• The entire outer IF block is unnecessary since it just checks if the quest is started and that's already a condition for the trigger to run in the first place.
  
  
• All trigger conditions use AND by default so there's no reason to use that extra AND grouping. In addition they check each condition in order and immediately stop once one is false so you can put the most common ones at the top to save some code execution time (not much though). I moved the structure check up.
  
  
• There's no need to check that the killcount is < 10 in the trigger conditions because once it gets to be >=10 the quest is no longer active so it fails at the conditions anyway.
  
  
• It might be easier to read/simpler to store the player ID for array indices into a variable instead of computing it every time; I called it TempInt below
  
  
• I've fixed the point and player group leaks.

• Lothaleven Kill Q Count
  • Events
    • Unit - A unit owned by Player 12 (Brown) Dies
  • Conditions
    • LothalevenPVPQ[(Player number of (Owner of (Killing unit)))] Equal to started
    • ((Killing unit) is A Hero) Equal to True
    • ((Dying unit) is A structure) Equal to False
    • (Unit-type of (Dying unit)) Not equal to Healing Ward
    • (Unit-type of (Dying unit)) Not equal to Stasis Trap
    • (Unit-type of (Dying unit)) Not equal to Watcher Ward
    • (Unit-type of (Dying unit)) Not equal to Sentry Ward
    • ((Dying unit) is Summoned) Equal to False
  • Actions
    • Set TempInt = (Player number of (Owner of (Killing unit)))
    • Set LothalevenKillCount[TempInt] = (LothalevenKillCount[TempInt] + 1)
    • Set TempPG = (Player group(Player(TempInt)))
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • LothalevenKillCount[TempInt] Greater than or equal to 10
      • Then - Actions
        • Set LothalevenPVPQ[TempInt] = completed
        • Game - Display to TempPG the text: |c00FFFF00Quest Upd...
        • Set TempPoint = (Position of Expedition Leaders 1061 <gen>)
        • Cinematic - Ping minimap for TempPG at TempPoint for 10.00 seconds, using a Flashy ping of color (100.00%, 100.00%, 100.00%)
        • Custom script: call RemoveLocation(udg_TempPoint) //don't forget to change the variable name here to match yours but keep the udg_ prefix
      • Else - Actions
        • Game - Display to TempPG the text: |c00FFFF00Quest Upd...
    • Custom script: call DestroyForce(udg_TempPG) //same here

Finally, this fixed it! Thank you very much!

I have a few questions though since I want to understand this a bit better for future use.
- Do I need to make additional TempInt's for other quests or can I reuse the ones from this trigger?
- Will this trigger work in the case of many players doing the same quest at once?

 

[Pyrogasm]

You can reuse TempInt and any such variables as you see fit.

Generally when people write example code with Temp<VARIABLE TYPE> in them those temp variables are only used transiently and do not ever need tohold data for longer than the trigger they're assigned in. An entire trigger execution happens in milliseconds (as long as no Wait actions are involved) and no two triggers can be running at the same time. However if Trigger A does something (example: kill a unit) that causes another trigger to fire it will pause execution of Trigger A, run all of Trigger B, then finish Trigger A.

If you are using Wait commands it's best to assume any/all of your temp variables would be over-written in that time and you should not rely on them after the wait (also clean them up before a wait if they are leakable points, etc.). If a wait is necessary you can create a variable specifically for that trigger (if you're certain the trigger can't be triggered a second time during the wait) or shadow a global variable in GUI so it behaves as a local variable.