[vJASS] Weird Crash

[Mr_Bean]

Hello,

I have this script that I am using for a spell. Anyway, it is meant to check if the boss's life is less than 33%. Here it is:

scope BurningDownpour initializer InitTrigger

    private function cond takes nothing returns boolean
        if (udg_Boss_Unit != null) and ((GetWidgetLife(udg_Boss_Unit) / GetUnitState(udg_Boss_Unit, UNIT_STATE_MAX_LIFE)) <= 0.33) then
            call BJDebugMsg("It works!")
        endif
        return false
    endfunction
    
    private function InitTrigger takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterTimerEventPeriodic(t, 5)
        call TriggerAddCondition(t, Condition(function cond))
        set t = null
    endfunction
    
endscope

The rest of the script isn't important. When I set the boss's life to 30% (using a test trigger), the game crashes instead of displaying "It worked!". Any ideas?

Thanks!

 

[Maker]

For me the script works.

Are you sure it is this scope that causes the crash? If you comment it out, does the crash still occur?

 

[Mr_Bean]

What do you mean by comment it out?

 

[cohadar]

I see you have been listening to advice of some peoples suffering from BJism.

Do not use conditions as actions, it does not optimize anything.
It is just a fairy tail invented by **s who believe they know how to write code. (yes ** I said)

Also register events after you add conditions and actions, not before.

 

[Dirac]

None of cohadar just said is true, not even a word

 

[Heaven Editor]

I would agree with Cohadar. The function say itself. Register conditions means register conditions for actions. why abuse???

 

[Dr Super Good]

Because Actions are more prone to leaks than conditions. You can easilly unregister conditions from a trigger without leaks but not so for actions. Additionally as conditions are run before actions in the trigger execution pipeline they are slightly faster.

I advise making sure there is no fatal script interaction going on (where 2 pieces of script interact with each other in an unexpected way).

 

[cohadar]

Because Actions are more prone to leaks than conditions. You can easilly unregister conditions from a trigger without leaks but not so for actions. Additionally as conditions are run before actions in the trigger execution pipeline they are slightly faster.

This is all imaginary arguments.
Actions are prone to leaks? When?
When you do dynamic trigger stuff.
Are spell triggers dynamic? No.
Why would anyone unregister normal spell trigger?

prone == maybe == bullshit

Slightly faster? Like in 0.2 nanoseconds? Like in who gives a fuck?

slightly == another bullshit

Use conditions for conditions, use actions for actions, when you get hungry make a break and eat, this is the way of Tao.

 

[Dr Super Good]

Are spell triggers dynamic?

No but they really should be if people could actually program properly.

Why would anyone unregister normal spell trigger?

When the normal spell trigger becomes unreachable (eg it will be impossible to be cast as no one can get hold of that unit). This will prevent unnescescary code from running.

In real life you should use dynamic libraries for something like a spell to optimize memory footprint. Obviosuly this is not possible in WC3 but people could atleast have the coutersy to not expect their spell to be usable all the time during play.

Ability cast triggers should only be initialized when 1 unit is able to cast it (eg a hero learns the ability or a unit was built with the ability already active). Until then abilities should have active script footprint at all (next to the control system which is a shared resource between all scripted abilities).

Ofcourse an alternative approach would be to have a control system that responds to the events of all abilities and then fowards the response on to the appropiate piece of script. Same idea but this time only initializing the ability when it is cast. Disadvantage of still needing half the system described above to detect when an initialized ability should be freed. Might be slower in some situations but both seem reasonably fast solutions to me.

 

[cohadar]

All you said would be true if it was not for one small detail.
Trigger event execution is not a linear for loop where every condition is checked in the same manner every time.

wc3 engine is actually smart enough to optimize conditions execution.
In fact I believe that putting action stuff inside conditions fuck-up optimization engine and in the end makes such "optimized" scripts run slower.

Think about this: why is GUI in starcraft faster than galaxy code?
Because GUI can have a limited set of patterns. And for code that uses patterns smart programmers can make very good optimizations. No one can optimize random code that much.

EDIT:
Omg trigger hijacking again, and to MrBean again.
Sorry man.

 

[Dr Super Good]

Think about this: why is GUI in starcraft faster than galaxy code?

Except it is not... GUI is as nearly as fast as scripting in galaxy due to it not being a total mess like GUI in WC3 is. It is impossible for it to execute faster than galaxy script because it compiles to galaxy script which is ordered prety much identicly to what you would produce if you made it directly in galaxy script.

wc3 engine is actually smart enough to optimize conditions execution.

Considering it does not free all unit data when a unit is removed, I have my doubts about the validility of this. People have done timing benchmarks and GUI is definatly a lot slower than using the JASS directly to make the structures more efficient.

And for code that uses patterns smart programmers can make very good optimizations. No one can optimize random code that much.

Proper compilers ignore code ordering anyway as they use instruction reordering to efficiently use the pipeline systems modern processors have. You can throw any idea of this having any relevance to JASS out the window due to the JASS machine interpreting names.

 

[cohadar]

It is impossible for it to execute faster than galaxy script because it compiles to galaxy script

Oh yes it can because it is not optimization on instruction level but on algorithmic level.

A link for those benchmarks you mentioned please?

PS: not-freeing all unit memory is a safety mechanism to prevent null pointer crashes.

 

[Switch33]

Off-Topic:
Cohadar lives in the world where wc3 was like 3 or 4 years ago disbelieving in all of the major script advancements recently for whatever reason. And DrSuperGood believes that everyone should just pack up and move to starcraft 2 for editing instead of wc3 hence he explains everything in both terms of sc2 and wc3 in the wc3 help section.

Together they make one giant off-topic festival.

On-Topic:
Can you at least show the triggers that "set the boss variable" and the trigger that "sets the life to 30.0" as that would probably reveal what's causing this.

 

[Dr Super Good]

Oh yes it can because it is not optimization on instruction level but on algorithmic level.

You seriously believe Blizzard would have bothered to make any form of advanced optimization? It would have been easier for them to plug an existing but highly optimized language rather than go through the bother to make their own if that is the case. If you say something like this you will need to link some evidence as currently GUI in galaxy produces the same script as you would directly with a tiny bit more bloat due to poor template design.

PS: not-freeing all unit memory is a safety mechanism to prevent null pointer crashes.

That does not even make sense considering I was testing with only 2 handles (which were recycled constantly). Even if you are telling the truth about the leak source, it still is bugged as it does leak in situations it should not (after a handle index has been reallocated to a new value like a different unit).

A link for those benchmarks you mentioned please?

Most of them were done at WarCraft3Campaigns and its partner sights (of which some may or may not still exist). Additionally people would have raised performance anomolies when a bloated solution executed faster than a "optimized" one which just has never happened.

 

[PurgeandFire]

I assume that udg_Boss_Unit has no value. I'm not sure why it is crashing though. Even when making comparisons, it should usually just stop the thread iirc.

To make sure it doesn't crash, make sure udg_Boss_Unit has an initial value of null. Also, when you say crash, do you mean a literal crash as in it exits the game, or do you mean it just doesn't show the message?

 

[Dirac]

@Cohadar
"Small" adjustments to codes can cause great speed performance, if a map were to abuse a proyectile system like my own (Like most TD's would like to do) the code must be as fast as possible to prevent the game from lagging. Giving a trigger actions and conditions for the same result is slower than just putting everything inside the condition.
It's not hard to imagine that a TD that would use 500 projectiles at the same time.
And is by doing small adjustments to my code following (what you consider rambling coder imagination) small improvements is that i've seen the FPS go from CRASH to stable 30 fps with the same result.

Back to topic I agree with Purge, its probably crashing because the global isn't initialized

 

[mckill2009]

why does it need "udg_Boss_Unit != null" anyway?, when GetWidgetLife(udg_Boss_Unit) is enough...

 

[Dr Super Good]

To prevent division by 0 thread crash in the case of the unit being null (null probably has a maximum life of 0). It is also good practice to avoid passing arguments null values unless it is intentional as not all natives handle null arguments robustly (some could possibly cause a fatal error I think).

The actual practice orignates in languages like C on virtual memory mapped systems where trying to read a null value will cause a segmentation fault (fatal error).

Remember that in the below statement.

bool1 and bool2

It will first evaluate bool1.
If bool1 is false then it will not bother evaluating bool2 (as false&X where X is any value is always false) and will evaluate the entire statement as alse.
Only if bool1 is true will it evaluate bool2.
If bool2 is false then it will evaluate the whole statement to false (same reas as above).
If bool2 is true the statement evaluates to true.

This probably means it is faster in the worse case as it avoids 2 function calls and a division.

 

[Mr_Bean]

For testing purposes I set udg_Boss_Unit to a boss that I have placed. I set this at map initialisation. I'm not at my pc now, but I use a simple GUI trigger to set it's life. I activate it with a chat command.

 

[Heaven Editor]

I believe..
Concentrate on optimization make game smoother. Its a should.
Concentrate on game play make people play your game. Its a must.

Therefore, construct a nice game play and then polish them from time to time.
Take example from the world leading Wc3 map. DotA Allstars. Its' 5th or early 6th version loading screen take around 3 minutes to complete. That moment people still crazy playing them. And i was one of those people.

No need to worry about BJ functions or any advancements. The priority is to attract people to play your game. Unless you have different purpose of map making ( and that will be odd).

 

[Troll-Brain]

For testing purposes I set udg_Boss_Unit to a boss that I have placed. I set this at map initialisation. I'm not at my pc now, but I use a simple GUI trigger to set it's life. I activate it with a chat command.

I don't know if your "map initialisation" is a GUI or a vJass initializer, but last time i checked, GUI globals defaults value (function InitGlobals) are given after all vJass initializers in the function main.
Which means if you initialize a GUI variable in a vJass initializer, it will be erased.

That does not even make sense considering I was testing with only 2 handles (which were recycled constantly). Even if you are telling the truth about the leak source, it still is bugged as it does leak in situations it should not (after a handle index has been reallocated to a new value like a different unit).

Not sure, but i think you're talking about that.
Don't take me wrong, i'm not saying you should care about it, just the leak mentionned is not related only to units, but handles in general.

 

[Mr_Bean]

I run map setup with GUI triggers. When I get back from holiday I will be able to do more testing.

 

[Troll-Brain]

If wc3 is closed without any error message, then it's an infinite new thread loop.
Most likely a trigger which contains an action, and this action fires an event of the trigger, and so one.

 

[Mr_Bean]

It says that Warcraft has stopped responding. However, this only occurs when it checks whether the boss has less than 33% life, NOT when I set his life.

 

[Heaven Editor]

try return something if the condition is true...

 

[Mr_Bean]

I'm away until Friday, so I can't.

 

[Dr Super Good]

Don't take me wrong, i'm not saying you should care about it, just the leak mentionned is not related only to units, but handles in general.

Not true, I have tested it with other types of handle such as triggers, events and timers and none of them gave a leak (memory usage remained approximatly constant). Using units the memory usage was always forever increasing until a crash occured (obviously some internal series overflowed). Further more, the rate of memory increase was proportional to the rate I created units but was uneffect by the rate of creation of triggers, events and timers.

One must remember that the memory usage of WC3 does systimaticly increase by itself even if no script is running. This is likly due to leaks inside the actual game engine itself or even that the game engine caches values for efficiency but never deems it nescescary to uncache them.

 

[Troll-Brain]

Hmm, i tested it with boolexprs, Jesus4Lyf with groups, so i assume it's a general bug.
But it's a really minor one (only few bytes), you have to use a huge loop to see it (and of course wait for the wc3 memory usage stability before and after the test, which is perfectly possible in a map with this only one trigger and you do nothing else than the test).

Now, i supposed that units leak much more than this tiny leak, but i can tell you there is one (or it was fixed in a newer patch, but i don't believe that much on Blizzard)