[Trigger] Loot/Unit Death - Please Critique / Suggest Better

[Gnuoy]

This trigger runs frequently and by multiple players. Seems to work but worried it's not efficient.

Spoiler: demon dies trigger

• demon dies
  • Events
    • Unit - A unit owned by Player 7 (Green) Dies
    • Unit - A unit owned by Player 11 (Dark Green) Dies
    • Unit - A unit owned by Player 12 (Brown) Dies
    • Unit - A unit owned by Player 17 (Wheat) Dies
    • Unit - A unit owned by Player 18 (Peach) Dies
    • Unit - A unit owned by Player 19 (Mint) Dies
    • Unit - A unit owned by Player 21 (Coal) Dies
    • Unit - A unit owned by Player 22 (Snow) Dies
    • Unit - A unit owned by Player 23 (Emerald) Dies
    • Unit - A unit owned by Player 24 (Peanut) Dies
  • Conditions
    • (Level of Legion Unit Class (Icon) for (Dying unit)) Equal to 1
  • Actions
    • Set VariableSet demon_kill_PN = (Player number of (Owner of (Killing unit)))
    • -------- count death for player --------
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • is_playing[demon_kill_PN] Equal to True
      • Then - Actions
        • Set VariableSet demon_kills[demon_kill_PN] = (demon_kills[demon_kill_PN] + 1)
      • Else - Actions
    • -------- remove unit from group variable --------
    • Unit Group - Remove (Dying unit) from demon_grp.
    • Unit Group - Remove (Dying unit) from death_attk_demon_grp.
    • Unit Group - Remove (Dying unit) from demon_flyer.
    • -------- rolls a random number 1-100 --------
    • Set VariableSet loot_ran_num = (Random integer number between 1 and 100)
    • -------- sets point variable --------
    • Set VariableSet demon_death_pt = (Position of (Dying unit))
    • -------- checks roll output loot --------
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • loot_ran_num Less than 15
        • ((Dying unit) is A structure) Equal to False
      • Then - Actions
        • Item - Create Mana Stone at demon_death_pt
        • Set VariableSet last_created_stone_mana = (Last created item)
        • -------- gives healing rune if difficulty to easy (1) or lower, also roll < 10 --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • loot_ran_num Less than 10
            • difficulty_setting Less than or equal to 2
          • Then - Actions
            • Item - Create Rune of Lesser Healing at demon_death_pt
            • Set VariableSet last_created_rune_hp = (Last created item)
          • Else - Actions
        • -------- uses/puts loot in inventory if hero --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • ((Killing unit) is A Hero) Equal to True
          • Then - Actions
            • Hero - Give last_created_stone_mana to (Killing unit)
            • Hero - Give last_created_rune_hp to (Killing unit)
            • Set VariableSet last_created_stone_mana = No item
            • Set VariableSet last_created_rune_hp = No item
          • Else - Actions
      • Else - Actions
        • -------- check if teleporter --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • (Level of Teleporter Unit Class (Icon) for (Dying unit)) Equal to 1
          • Then - Actions
            • Set VariableSet teleporter_counter = (teleporter_counter - 1)
          • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • loot_ran_num Greater than 85
        • ((Dying unit) is A structure) Equal to False
      • Then - Actions
        • Item - Create Health Stone at demon_death_pt
        • Set VariableSet last_created_stone_hp = (Last created item)
        • -------- gives mana rune if difficulty to easy (1) or lower, also roll > 90 --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • loot_ran_num Greater than 90
            • difficulty_setting Less than or equal to 2
          • Then - Actions
            • Item - Create Rune of Mana at demon_death_pt
            • Set VariableSet last_created_rune_mana = (Last created item)
          • Else - Actions
        • -------- uses/puts loot in inventory if hero --------
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • ((Killing unit) is A Hero) Equal to True
          • Then - Actions
            • Hero - Give last_created_stone_hp to (Killing unit)
            • Hero - Give last_created_rune_mana to (Killing unit)
            • Set VariableSet last_created_stone_hp = No item
            • Set VariableSet last_created_rune_mana = No item
          • Else - Actions
      • Else - Actions
    • -------- creates a demon loot if roll is a specific number at death point --------
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • loot_ran_num Equal to 50
        • ((Dying unit) is A structure) Equal to False
      • Then - Actions
        • Item - Create Demonic Figurine at demon_death_pt
      • Else - Actions
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • loot_ran_num Equal to 49
        • ((Dying unit) is A structure) Equal to False
      • Then - Actions
        • Item - Create Spiked Collar at demon_death_pt
      • Else - Actions
    • -------- remove / reset variable leaks --------
    • Custom script: call RemoveLocation(udg_demon_death_pt)

 

[Uncle]

1) Store (Dying unit) and (Killing unit) in their own variables and reference them throughout. This is more efficient and helps avoid issues.

2) Reduce redundancy, you should only need to check if it's a Structure once. A single If Then Else should check this and you should put all of the loot Actions inside of it's Then - Actions section. Although, this begs the question, do any of the Structures even have the Legion Unit Class (Icon) ability? If not, the Structure check is unnecessary.

3) Take advantage of the Else - Actions section to prevent unnecessary checks. If loot_ran_num is Less than 15 then it couldn't possibly be Greater than 85 so why bother asking that question as well? I know a lot of people find the triggers hard to read when things become nested but this is the most efficient way of doing it.

4) Only do what NEEDS to be done. You're always setting the random number, Point, and Point removal, even though there's a chance that these aren't used at all. Only Set them when you know that they're going to be used.

5) Your Teleporter logic looks flawed. It's only checking if a Teleporter died when the random number is > 15. That seems like something you need to ALWAYS check regardless of the random roll. So this is a case where you don't want to use Else - Actions.

I highly recommend using unique variables in your triggers that have the Unit Dies event. This is due to the problematic behavior of this Event, it runs DURING other triggers instead of after them which can cause problems with shared variables (variable values can get set to something else). It looks like you're handling this properly but I figured I'd still mention it just in case.

 

[Gnuoy]

Thanks I'll get to work on that. Is it more efficient / negligible, these two events?

• demon dies
  • Events
    • Unit - A unit owned by Player 7 (Green) Dies
    • Unit - A unit owned by Player 11 (Dark Green) Dies
    • Unit - A unit owned by Player ...

• demon dies
  • Events
    • Unit - A unit Dies

do any of the Structures even have the Legion Unit Class (Icon) ability?

Now that you mention it no. The structure check was a legacy condition i forgot to remove after I added the ability integer checks.

3) Take advantage of the Else - Actions to prevent unnecessary checks. If loot_ran_num is Less than 15 then it couldn't possibly be Greater than 85 so why bother asking that question? I know a lot of people find the triggers hard to read when things become nested but this is the most efficient way of doing it.

should i put another If in after the else part to check if it's over 85? ok.

 

[Uncle]

I always rely on the Generic event for this but I'm not 100% sure.

Also, I edited my last post and added some extra info.

The If Then Else breakdown:


If random_number < 15
// do loot stuff

Else if random_number > 85
// do loot stuff

Else if random_number = 49
// do loot stuff

Else if random_number = 50
// do loot stuff

If teleporter
// do teleporter stuff


Where I wrote "// do loot stuff" is where you would Set the Point, create the Items, and Remove the Point. It's not the prettiest solution since there will be some redundancy (which I recommended against) but in this case you're trading organization for more efficiency. I think this trade is worth it, especially when it comes to an Event as common as a Unit dying.