First JASS spells

I don't want my post to sound harsh or mean, so don't get upset of what I am going to say. It is good that you started to learn JASS (hope that's what you did) but...

1. The whole stuff looks like a plain GUI -> JASS conversion. I can only see that you added locals and attempted to remove them. However, you kinda failed, because nullification was not made correctly.

Starting with Chaos End, you nullified global variables, instead of the locals. Change that, and it should be fine, in terms of variables.

Poison on the other hand... It's not good that you nullified only the last unit of the array. When you exit the loop, the value of GetForLoopIndexA() is 15 and so, only Last1[15] will be nullified. If you wanted to nullify it correctly, you should've written the nullification line just before the endloop.

2. There are obvious signs of conversion and I a eager to teach you how to solve them. First problem, is the condition of the trigger. Here is how your code looks (it's the same for any such conditions):

function Trig_Chaos_End_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A000' ) ) then
        return false
    endif
    return true
endfunction

What I notice here are useless returns and ifs. The returned value can be also an expression (the computer calculates the expression and returns its result).

GetSpellAbilityId() == 'A000' is a boolean expression. That means that it calculates the equality, and if the two are equal it returns true. Else, it will return false. That's the beauty of the booleans. It can be only true and false. And what is the function supposed to return? The value of the condition (true to go on, and false to stop). So you could do this instead:

function Trig_Chaos_End_Conditions takes nothing returns boolean
    return GetSpellAbilityId() == 'A000'
endfunction

Let's analyze it a little. What does this function do? It compares the Id of the ability cast, to 'A000'. If the two are equal, then the unit cast the ability you wanted, and it will return true. If not, it will return false! That way, you simplify a lot the function! Keep this in mind!

3. Loops
Another sign of obvious conversion from GUI to JASS is the loop. You used two bj global variables. This is bad, and I am going to help you fix this. The best thing about JASS is that instead of a loop that repeats a thing for an exact amount of times (does not depend on a condition), JASS loop repeats until a condition is not true. That means that the number of repeats can also be unknown. It's easier to turn such a loop into a loop with known repeats, then the opposite. Here is what you can do:

->You take a single local variable which will practically help you to count the number of repeats.
->You initialize it with 1
->You set the loop exit condition to (LOCAL_VARIABLE > number_of_repeats)
->You increase your local variable at the end of your loop

The code would look like this (of course, in a function):

local integer i = 1
loop
    exitwhen i>6
    //Other actions
    set i = i + 1
endloop

This loop is repeated 6 times.

I won't delete the spells (and don't do it either). I want you to understand that they are not really JASS spells, until you start learning some important elements. I want to see these spells improved and fixed, so that I know that you understand. These spells will be deleted in the end (they are far too simple), but not until you learnt something from that and I know you did.

I can teach you more if you have an MSN or AIM account. Look at my profile for each. Good luck!

~Daelin

First JASS spells

Comments