How to make this more efficient.

[Spartipilo]

Hi again, and thank you for reading.

I made a combat system a time ago, but checking for leaks and else wanted to improve it the best I could. I have tested this one and works fine with me alone, haven't tested with others yet. I'd like to know if there's a way to improve it.

The map is an RPG and has about 16 Combat Regions, and the combat system has to work simultaneously for at least 5 of them at the same time. This is just 1, and I need to know if it's good for making it 15 more times in the same trigger (would be long and heavy to load)

If improving involves JASS; teach me how to, or kindly do it for me, since I don't know enough JASS nor vJASS to do this.

Combat System

• Combat System 2 COMMENTED
  • Events
    • Time - SpawnTimer expires
  • Conditions
  • Actions
    • -------- Global --------
    • Set UONH = (Units owned by Neutral Hostile)
    • -------- Where the creep count appear is created. --------
    • Set CCP[1] = (Center of BeachCombat <gen>)
    • -------- Pre-setting the groups. --------
    • Set Beach[1] = (Units in BeachCombat <gen> matching ((((Matching unit) is alive) Equal to True) and (((Matching unit) is in UONH) Equal to True)))
    • Set Beach[2] = (Units in BeachCombat <gen> matching (((Matching unit) is A Hero) Equal to True))
    • Set Beach[3] = (Units in BeachEscape <gen> matching (((Matching unit) is A Hero) Equal to True))
    • Set Beach[4] = (Units in BeachCombat <gen> owned by Neutral Hostile)
    • -------- Creating the creep count. --------
    • Floating Text - Create floating text that reads (Creeps Left: + (|cfffff700 + ((String((Number of units in Beach[1]))) + |r))) at CCP[1] with Z offset 0.00, using font size 10.00, color (100.00%, 100.00%, 100.00%), and 0.00% transparency
    • Floating Text - Change (Last created floating text): Disable permanence
    • Floating Text - Change the lifespan of (Last created floating text) to 1.20 seconds
    • -------- Removing the Creep Count point. --------
    • Custom script: call RemoveLocation(udg_CCP[1])
    • -------- Saving Position of Hero and Sending to combat --------
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (Random integer number between 1 and 100) Less than or equal to (Random integer number between 4 and 16)
        • (Number of units in Beach[2]) Equal to 0
      • Then - Actions
        • Unit Group - Pick every unit in Beach[0] and do (Actions)
          • Loop - Actions
            • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • ((Picked unit) has buff Safety ) Equal to False
              • Then - Actions
                • Set Temp_Int = (Player number of (Owner of (Picked unit)))
                • Set Temp_HeroPos[Temp_Int] = (Position of (Picked unit))
                • Set Temp_RandomHeroSpawn[Temp_Int] = (Random point in BeachSpawn <gen>)
                • Custom script: if GetLocalPlayer() == GetOwningPlayer(GetEnumUnit()) then
                • Cinematic - Fade out over 0.00 seconds using texture White Mask and color (0.00%, 0.00%, 0.00%) with 0.00% transparency
                • Custom script: endif
                • Unit - Move (Picked unit) instantly to Temp_RandomHeroSpawn[(Player number of (Owner of (Picked unit)))]
                • Custom script: if GetLocalPlayer() == GetOwningPlayer(GetEnumUnit()) then
                • Cinematic - Fade in over 1.00 seconds using texture White Mask and color ((Random real number between 0.00 and 100.00)%, (Random real number between 0.00 and 100.00)%, (Random real number between 0.00 and 100.00)%) with 0.00% transparency
                • Custom script: endif
                • Custom script: call RemoveLocation(udg_Temp_RandomHeroSpawn[udg_Temp_Int])
                • Unit Group - Add (Picked unit) to Beach[2]
                • Unit Group - Remove (Picked unit) from Beach[0]
              • Else - Actions
        • -------- Spawning the Creeps --------
        • Set Int_BC = ((Number of units in Beach[4]) x 2)
        • Set Int_RBC = (Random integer number between 1 and Int_BC)
        • For each (Integer A) from 1 to Int_RBC, do (Actions)
          • Loop - Actions
            • Set Temp_PointArray[2] = (Random point in BeachSpawn <gen>)
            • Unit - Create 1 BeachCreeps[(Random integer number between 1 and 7)] for Neutral Hostile at Temp_PointArray[2] facing Default building facing degrees
            • Custom script: call RemoveLocation(udg_Temp_PointArray[2])
      • Else - Actions
    • -------- Beach Group Cleaning --------
    • -------- Cleaning the mess / Restoring positions --------
    • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • Or - Any (Conditions) are true
          • Conditions
            • (Number of units in Beach[2]) Equal to (Number of units in Beach[3])
            • (Number of units in Beach[2]) Equal to 0
      • Then - Actions
        • Unit Group - Pick every unit in Beach[2] and do (Actions)
          • Loop - Actions
            • Set Temp_Int = (Player number of (Owner of (Picked unit)))
            • Custom script: if GetLocalPlayer() == GetOwningPlayer(GetEnumUnit()) then
            • Cinematic - Fade out over 0.00 seconds using texture White Mask and color (0.00%, 0.00%, 0.00%) with 0.00% transparency
            • Custom script: endif
            • Unit - Move (Picked unit) instantly to Temp_HeroPos[Temp_Int]
            • Custom script: if GetLocalPlayer() == GetOwningPlayer(GetEnumUnit()) then
            • Cinematic - Fade in over 1.00 seconds using texture White Mask and color ((Random real number between 0.00 and 100.00)%, (Random real number between 0.00 and 100.00)%, (Random real number between 0.00 and 100.00)%) with 0.00% transparency
            • Custom script: endif
            • Unit - Create 1 DummySafetyCaster for Neutral Passive at Temp_HeroPos[Temp_Int] facing Default building facing degrees
            • Unit - Order (Last created unit) to Human Priest - Inner Fire (Picked unit)
            • Unit - Remove (Last created unit) from the game
            • Custom script: call RemoveLocation(udg_Temp_HeroPos[udg_Temp_Int])
            • Unit Group - Remove (Picked unit) from Beach[2]
            • Unit Group - Add (Picked unit) to Beach[0]
        • Unit Group - Pick every unit in Beach[4] and do (Actions)
          • Loop - Actions
            • Unit - Remove (Picked unit) from the game
        • Item - Pick every item in BeachCombat <gen> and do (Actions)
          • Loop - Actions
            • Item - Remove (Picked item)
        • Destructible - Pick every destructible in BeachCombat <gen> and do (Destructible - Resurrect (Picked destructible) with (Max life of (Picked destructible)) life and Hide birth animation)
      • Else - Actions
    • -------- Removing group leaks --------
    • Custom script: call DestroyGroup(udg_UONH)
    • Custom script: call DestroyGroup(udg_Beach[1])
    • Custom script: call DestroyGroup(udg_Beach[2])
    • Custom script: call DestroyGroup(udg_Beach[3])
    • Custom script: call DestroyGroup(udg_Beach[4])

Thanks!

 

[Narogog]

well, in all of the pick every unit set the picked unit to a variable, and use that variable instead of picked unit. it will improve the efficiency by quit a bit

why

well because picked unit is the function GetEnumUnit() and it runs every time you use picked unit which isn't very efficient

• Unit Group - Remove (Picked unit) from Beach[2]
  • Unit Group - Add (Picked unit) to Beach[0]

what is this for?
in

• nit Group - Pick every unit in Beach[2] and do (Actions)
  • Loop - Actions
  • Set Temp_Int = (Player number of (Owner of (Picked unit)))
  • Custom script: if GetLocalPlayer() == GetOwningPlayer(GetEnumUnit()) then
  • Cinematic - Fade out over 0.00 seconds using texture White Mask and color (0.00%, 0.00%, 0.00%) with 0.00% transparency
  • Custom script: endif
  • Unit - Move (Picked unit) instantly to Temp_HeroPos[Temp_Int]
  • Custom script: if GetLocalPlayer() == GetOwningPlayer(GetEnumUnit()) then
  • Cinematic - Fade in over 1.00 seconds using texture White Mask and color ((Random real number between 0.00 and 100.00)%, (Random real number between 0.00 and 100.00)%, (Random real number between 0.00 and 100.00)%) with 0.00% transparency
  • Custom script: endif
  • Unit - Create 1 DummySafetyCaster for Neutral Passive at Temp_HeroPos[Temp_Int] facing Default building facing degrees
  • Unit - Order (Last created unit) to Human Priest - Inner Fire (Picked unit)
  • Unit - Remove (Last created unit) from the game
  • Custom script: call RemoveLocation(udg_Temp_HeroPos[udg_Temp_Int])
  • Unit Group - Remove (Picked unit) from Beach[2]
  • Unit Group - Add (Picked unit) to Beach[0]

you might want to add an expiration timer to the caster instead of removing it so that the unit can cast its spell.

other then that i it looks pretty efficient

edit1: why do you have a condition that makes so that if Beach[2] has no units it will run the action ForGroup Beach[2] do actions?

• Any (Conditions) are true
  • Conditions
    • (Number of units in Beach[2]) Equal to (Number of units in Beach[3])
    • (Number of units in Beach[2]) Equal to 0

 

[Spartipilo]

Sorry for my English! I'm doing my best here.

Great! Thanks for giving an answers, now it's my turn

- Great, i'll make the 'Picked Unit' variable and use it instead of 'Picked unit' Thanks.

- My regions are not big squares covered by a region because they are irregular (otherwise I would do it that way), but there are 2 regions in each entrance/exit, one to add the hero to the Group[0] and another to remove it from Group[0]. Group[0] is the group of heroes in a place where combat can be engaged. When the units engage combat, they're moved to Group[2], otherwise, they would still engage combat during the combat (since the units for combats are picked from Group[0]). =)

- I was using the expiration timer, but I'd wanted to test with (Remove Unit) and it worked well, so I left it that way .

Detailed answer to your 'Edit1' question:
- Group[2] are the heroes in combat, Group[3] are the heroes in the 'Exit/Escape' region. To leave a combat, all the heroes in the fight have to stand in the exit to restore their positions. When units die, they are removed from Group[0] (If they revive in town, shouldn't engage combat), also from Group[2] (Otherwise, the rest of heroes alive wouldn't be able to exit, since the hero outside the combat region would be still in the combat group), and also from Beach[3] (Honestly, I don't remember why if the units are never added to that group xD)

So, if all heroes die, the area is cleaned, just like if everyone have left combat standing in the Exit/Escape region.

Edit: Done. Changed all the 'Picked Unit' for 'Temp Unit'. Works... almost the same, but i'm sure it's better this way. Thanks for the suggestion.