[Trigger] Spell crashes wc3 if used multiple times at the same time

[Pvt.Toma]

The spell seems to crash the game if too much of it is used at the same time. Any idea?

Trigger

• Hook
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to (==) Fish
  • Actions
    • Set HookIntCaster = (HookIntCaster + 1)
    • Set TempL = (Target point of ability being cast)
    • Set HookI1[HookIntCaster] = (Level of Fish for (Triggering unit))
    • Set HookL1[HookIntCaster] = (Position of (Triggering unit))
    • Unit - Create 1 Hook1 for (Owner of (Triggering unit)) at HookL1[HookIntCaster] facing (Angle from HookL1[HookIntCaster] to TempL) degrees
    • Set Hook[HookIntCaster] = (Last created unit)
    • Unit Group - Add (Last created unit) to HookG2
    • Set Hook2[HookIntCaster] = (Triggering unit)
    • Set HookBoo[HookIntCaster] = False
    • Lightning - Create a Magic Leash lightning effect from source HookL1[HookIntCaster] to target HookL1[HookIntCaster]
    • Set HookLig[HookIntCaster] = (Last created lightning effect)
    • Custom script: call RemoveLocation(udg_TempL)
      • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
        • If - Conditions
          • (Hook Loop <gen> is on) Equal to (==) False
        • Then - Actions
          • Trigger - Turn on Hook Loop <gen>
        • Else - Actions

• Hook Loop
  • Events
    • Time - Every 0.05 seconds of game time
  • Conditions
  • Actions
    • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (HookG2 is empty) Equal to (==) False
      • Then - Actions
        • Do Multiple ActionsFor each (Integer HookInt2) from 1 to HookIntCaster, do (Actions)
          • Loop - Actions
            • Set TempL = (Position of Hook[HookInt2])
            • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • HookBoo[HookInt2] Equal to (==) False
              • Then - Actions
                • Set TempL2 = (TempL offset by 40.00 towards (Facing of Hook[HookInt2]) degrees)
                  • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
                    • If - Conditions
                      • Multiple ConditionsOr - Any (Conditions) are true
                        • Conditions
                          • Multiple ConditionsAnd - All (Conditions) are true
                            • Conditions
                              • HookI1[HookInt2] Less than or equal to (<=) 4
                              • (Distance between HookL1[HookInt2] and TempL2) Greater than or equal to (>=) (500.00 + (100.00 x (Real(HookI1[HookInt2]))))
                          • Multiple ConditionsAnd - All (Conditions) are true
                            • Conditions
                              • HookI1[HookInt2] Greater than (>) 4
                              • (Distance between HookL1[HookInt2] and TempL2) Greater than or equal to (>=) (900.00 + (50.00 x ((Real(HookI1[HookInt2])) - 4.00)))
                    • Then - Actions
                      • Set HookBoo[HookInt2] = True
                    • Else - Actions
                • Unit - Move Hook[HookInt2] instantly to TempL2
                • Lightning - Move HookLig[HookInt2] to source HookL1[HookInt2] and target TempL2
                • Custom script: call RemoveLocation(udg_TempL2)
              • Else - Actions
                • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
                  • If - Conditions
                    • (Distance between HookL1[HookInt2] and TempL) Greater than (>) 50.00
                  • Then - Actions
                    • Set TempL2 = (TempL offset by 40.00 towards ((Facing of Hook[HookInt2]) - 180.00) degrees)
                    • Unit - Move Hook[HookInt2] instantly to TempL2
                    • Lightning - Move HookLig[HookInt2] to source HookL1[HookInt2] and target TempL2
                    • Custom script: call RemoveLocation(udg_TempL2)
                  • Else - Actions
                    • Unit Group - Remove Hook[HookInt2] from HookG2
                    • Unit - Remove Hook[HookInt2] from the game
                    • Lightning - Destroy HookLig[HookInt2]
                    • Custom script: call RemoveLocation(udg_HookL1[udg_HookInt2])
            • Custom script: call RemoveLocation(udg_TempL)
      • Else - Actions
        • Set HookIntCaster = 0
        • Custom script: call DestroyGroup(udg_HookG2)
        • Custom script: set udg_HookG2 = CreateGroup()
        • Trigger - Turn off Hook Loop <gen>

Spell as attached below.

 

[Almia]

Maybe you are destroying null lightnings. Destroying null lightnings crashes maps.

 

[Zwiebelchen]

This is because you never decrement HookIntCaster again. HookIntCaster just rises and rises with every use of the ability. As soon as it hits the max index for array variables (8191), the game crashes. This is called a "stack overflow", as what you are creating is - in programming terms - a stack: http://en.wikipedia.org/wiki/Stack_(abstract_data_type

"HookIntCaster" is basicly a stack count and HookInt2 is your iterator. Whenever you remove an instance of your spell, you must swap the top entry of your stack with the entry that is to be removed, to keep the stack from collapsing. Then you decrement HookIntCaster by one. This will ensure that there are no "dead values" in your stack of spell instances that have already been terminated and keeps your stack as short as possible.

So, basicly: Whenever one of your hook spells end, you must swap all data indexed by HookIntCaster and HookInt2, then decrement HookIntCaster by 1.

 

[Pvt.Toma]

Even though I don't think the stack ever reaches 8191, or even 1k (as it crashes right when 3-5 instances of the spell is cast close to each other), I'll try to implement what you suggested.

 

[Almia]

@Zwieb
Yes, I don't think that It can get to that instantly(over 8191)

What I am guessing here is that you are destroying an already destroyed instance of lightning

 

[Pvt.Toma]

Right, I'll test it out too.

Edit:
Actually, I don't think that that's the problem, cause wc3 crashes right when the 3rd/4th/5th unit casts it, not when it ends.

But as stated, the crash is a 'memory can't be written' crash, and the map is attached so that anyone can replicate it themselves.

As well as the fact that the only instance of me destroying lightning at the end seems justified.

Edit 2:
Wait, testing it out now.

Edit 3:

Take back everything I said, it's the lightning thing alright.
Added a condition where Hook[HookInt2] needs to be alive for the stuffs to run, and the crash is no more.
Problem solved +rep <3

 

[edo494]

accessing array out of bounds does nothing, not even crashing the running execution thread.

So it is the lightning, but it is not as easy as destroying null lightning, I tested this a while back, and couldnt get the game to crash

 

[Pvt.Toma]

Perhaps trying to destroy it rigorously? No idea, but this solved the problem.

Derp

• Hook Loop
  • Events
    • Time - Every 0.05 seconds of game time
  • Conditions
  • Actions
    • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
      • If - Conditions
        • (HookG2 is empty) Equal to (==) False
      • Then - Actions
        • Do Multiple ActionsFor each (Integer HookInt2) from 1 to HookIntCaster, do (Actions)
          • Loop - Actions
            • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
              • If - Conditions
                • (Hook[HookInt2] is alive) Equal to (==) True
              • Then - Actions
                • Set TempL = (Position of Hook[HookInt2])
                  • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
                    • If - Conditions
                      • HookBoo[HookInt2] Equal to (==) False
                    • Then - Actions
                      • Set TempL2 = (TempL offset by 40.00 towards (Facing of Hook[HookInt2]) degrees)
                        • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
                          • If - Conditions
                            • Multiple ConditionsOr - Any (Conditions) are true
                              • Conditions
                                • Multiple ConditionsAnd - All (Conditions) are true
                                  • Conditions
                                    • HookI1[HookInt2] Less than or equal to (<=) 4
                                    • (Distance between HookL1[HookInt2] and TempL2) Greater than or equal to (>=) (500.00 + (100.00 x (Real(HookI1[HookInt2]))))
                                • Multiple ConditionsAnd - All (Conditions) are true
                                  • Conditions
                                    • HookI1[HookInt2] Greater than (>) 4
                                    • (Distance between HookL1[HookInt2] and TempL2) Greater than or equal to (>=) (900.00 + (50.00 x ((Real(HookI1[HookInt2])) - 4.00)))
                          • Then - Actions
                            • Set HookBoo[HookInt2] = True
                          • Else - Actions
                      • Unit - Move Hook[HookInt2] instantly to TempL2
                      • Lightning - Move HookLig[HookInt2] to source HookL1[HookInt2] and target TempL2
                      • Custom script: call RemoveLocation(udg_TempL2)
                    • Else - Actions
                      • Multiple FunctionsIf (All Conditions are True) then do (Then Actions) else do (Else Actions)
                        • If - Conditions
                          • (Distance between HookL1[HookInt2] and TempL) Greater than (>) 50.00
                        • Then - Actions
                          • Set TempL2 = (TempL offset by 40.00 towards ((Facing of Hook[HookInt2]) - 180.00) degrees)
                          • Unit - Move Hook[HookInt2] instantly to TempL2
                          • Lightning - Move HookLig[HookInt2] to source HookL1[HookInt2] and target TempL2
                          • Custom script: call RemoveLocation(udg_TempL2)
                        • Else - Actions
                          • Unit Group - Remove Hook[HookInt2] from HookG2
                          • Unit - Remove Hook[HookInt2] from the game
                          • Lightning - Destroy HookLig[HookInt2]
                          • Custom script: call RemoveLocation(udg_HookL1[udg_HookInt2])
                • Custom script: call RemoveLocation(udg_TempL)
              • Else - Actions
      • Else - Actions
        • Set HookIntCaster = 0
        • Custom script: call DestroyGroup(udg_HookG2)
        • Custom script: set udg_HookG2 = CreateGroup()
        • Trigger - Turn off Hook Loop <gen>

As you can see, all I added was a condition right after the For each bla bla function.

 

[Zwiebelchen]

Just a question: does any of the hooked units die while the spell is running? You have no protection against that case and it might result in the spell attempting to move a null unit in case the unit is set to death type "does not decay".

EDIT: Beat me to it.

 

[Pvt.Toma]

Nay, the spell does no damage to anything.

 

[Zwiebelchen]

If that alive check helped, then the problem was that you attempted to move/create lightnings for units that had already been removed from the system, as the spell terminated on them.

It's basicly as I told you: your stack has shadow references that cause the system to malfunction as you do not properly clean the stack once a spell instance is removed from it.

So, again, you need to repair your data structure or it doesn't work.


EDIT: I'll try to illustrate the problem to you.

If you check the attached image, this would be a stack in which 5 spell instances are stored via your index variable.
If you remove spell instance 3, for example, the iterating variable "HookInt2" would try to do all the stuff in your loop for non-existing lightnings and units at non-defined places. This is what causes your crash.

What you did with that alive check is basicly just skipping the removed spell instance 3 and going to the next (from 2 to 4). This works, but it's the naive approach to this problem, as the stack just keeps growing and growing with every spell cast.

The better solution would be to swap spell instance 5 with spell instance 3 before deleting spell instance 3, so that the stack has no "deserted entry" in the middle. Then you decrement your max stack value (HookIntCaster) by 1 again.

 

[Pvt.Toma]

Aye, I agree with you. That would prevent the probable event of stack overflowing from happening.
I can probably just add in the part where i remove Hook[HookInt2] to replace all of HookXYZ[HookInt2] with HookXYZ[HookIntCaster] and that would be fine, yeah.
Just that there's a test tomorrow, so I'll have to get to it later, so the easier solution comes first.

 

[Zwiebelchen]

But I drew you pictures.

 

[Wietlol]

Shouldn't it be 1 -> 2 -> 4 -> 5 instead of 1 -> 2 -> 5 -> 4?

 

[Zwiebelchen]

Shouldn't it be 1 -> 2 -> 4 -> 5 instead of 1 -> 2 -> 5 -> 4?

The order of the stack doesn't matter in his case. By taking only the top of the stack and swapping it with the removed entry, you save additional operations for maintaining the order.