| 10-30-2007, 08:59 AM | #1 | ||
Below is my first trigger written in jass, if anyone has the time to quickly go through and point out any really bad stuff in there it would be greatly appreciated.
Here are the errors I got from the JassCraft syntax checker.
I think there's still some memory leaks in there, not sure how to fix them properly |
| 10-30-2007, 12:10 PM | #2 |
Let me see!!! |
| 10-30-2007, 01:10 PM | #3 |
JASS:ffunction Rebellion_Condtions takes nothing returns boolean return GetSpellAbilityId() == 'A000' // - checks the spell is the right one endfunction gg_trg_Rebellion is a global variable created by the world editor, to fix this problem in JASSCraft add this block: JASS:globals trigger gg_trg_Rebellion = null endglobals The charm error appears because you forgot to add ", should be "charm" The next error are actually 2. JASS:
loop // - loops until the timer has elapsed
exitwhen e = d
set e == TimerGetElapsed(t)
endloop
JASS:
loop // - loops until the timer has elapsed
exitwhen e == d
set e = TimerGetElapsed(t)
endloop
However i hope there won't be any error anymore, were too lazy to take a look on the entire script. ---Edit--- Also move this part JASS:function InitRebellion takes nothing returns nothing set gg_trg_Rebellion = CreateTrigger() call TriggerRegisterAnyUnitEventBJ( gg_trg_Rebellion, EVENT_PLAYER_UNIT_SPELL_EFFECT ) call TriggerAddCondition( gg_trg_Rebellion, Condition( function Trig_Rebellion_Conditions ) ) call TriggerAddAction( gg_trg_Rebellion, function Trig_Rebellion_Actions ) endfunction |
| 10-30-2007, 01:34 PM | #4 |
ffunction Rebellion_Condtions takes nothing returns boolean
Should be: function Rebellion_Condtions takes nothing returns boolean This is bad: JASS:function InitRebellion takes nothing returns nothing set gg_trg_Rebellion = CreateTrigger() call TriggerRegisterAnyUnitEventBJ( gg_trg_Rebellion, EVENT_PLAYER_UNIT_SPELL_EFFECT ) call TriggerAddCondition( gg_trg_Rebellion, Condition( function Trig_Rebellion_Conditions ) ) call TriggerAddAction( gg_trg_Rebellion, function Trig_Rebellion_Actions ) endfunction This function should be at the bottom, at least below the action/condition functions used in the trigger. This kinda sucks: JASS:call CreateUnitAtLoc (GetOwningPlayer(r), H003, GetUnitLoc (x), 1) // - creates the dummy caster next to the target set r = GetLastCreatedUnit() // - sets r to the dummy caster Because you used CreateUnitAtLoc which doesn't store the created unit in bj_lastCreatedUnit, so using GetLastCreatedUnit() won't retrieve it. You have two options:
I suggest the second option, it's faster anyways. Also, H003 must be an integer, so wrap it in '', like this: call CreateUnitAtLoc (GetOwningPlayer(r), @'H003'@, GetUnitLoc (x), 1) Same mistake here: call UnitApplyTimedLife(r, B001, 5) Should be: call UnitApplyTimedLife(r, 'B001', 5) Again (this should go at the top, but meh): local integer d = 20 + (GetUnitAbilityLevel (c, A000) * 5) Should be: local integer d = 20 + (GetUnitAbilityLevel (c, 'A000') * 5) Again: local integer b = GetUnitAbilityLevel (x, B000) Should be: local integer b = GetUnitAbilityLevel (x, 'B000') Almost the same mistake here: call IssueTargetOrder (r, charm, x) In this case you must wrap charm in "", so: call IssueTargetOrder (r, "charm", x) This loop is a wtf: JASS:loop exitwhen e = d set e == TimerGetElapsed(t) endloop This will crash the map for sure because the loop is executing an infinite number of times in a single moment, and the exitwhen you're using requires some time interval, so this: JASS:loop exitwhen e = d call TriggerSleepAction(0.1) set e == TimerGetElapsed(t) endloop Won't crash the map. It's checking every 0.1 seconds if the timer has elapsed. Also, this: call Destroy Doesn't do anything. What the hell is it supposed to be??? :) I thought I've seen them all, but I guess there's always something I haven't seen: JASS:set unit c = null set unit x = null set player p = null set unit r = null set timer t = null First of all: Hahahaha!! Sorry :P This is correct: set <variable name> = <value> This is a wtf (meaning wrong): set <variable type> <variable name> = <value> It doesn't work that way, so that part should look like this: JASS:set c = null set x = null set p = null set r = null set t = null EDIT: How I hate when somebody finishes before me, but mine is better anyways :P Btw, Fireeye, you didn't change anything in that loop part, you just posted two identical codes. |
| 10-30-2007, 01:48 PM | #5 |
i did, the condition had only 1 = and the set had 2 =. Mine got 2 = in the condition and 1 in the set, which is still wrong in your loop. |
| 10-30-2007, 08:26 PM | #6 |
Oh, I didn't see :), a single "=" is hard to notice. Sorry, my bad. |
| 10-30-2007, 10:26 PM | #7 |
thanks for all the help guys, the call destroy at the end was just something i forgot to get rid of after (hopefully)fixing leaks. Ill get working on the rest now ok, updated with all the fixes, turns out the ffunction was just from c&p |
| 10-31-2007, 03:50 AM | #8 |
None of you noticed that he put "InitRebellion" instead of "InitTrig_Rebellion". That's a pretty big error right there. |
| 10-31-2007, 06:20 AM | #9 | |
Quote:
I read a tutorial that said you dont need the Trig_ part in the function names (cant remember where), so i got rid of it because i think it looks bad, but ill put it back in so i dont have to worry about it. Maybe it meant in functions that get called from other functions, not functions that have init, conditions, and actions... /confused updated again |
| 10-31-2007, 09:15 AM | #10 |
Shouldn't you hide the unit, not change the vertex colouring? call ShowUnit(r,false) -Av3n |
| 10-31-2007, 02:14 PM | #11 | ||
Quote:
Yes, it meant about the functions that are used from InitTrig_ function Quote:
And that too :), but it still can't compete with set unit u = null |
| 10-31-2007, 07:34 PM | #12 |
InitTrig functions are essential. They are executed when the map initializes and it sets up the triggers. With the NewGen pack you can remove the need for InitTrigs, but then they won't be executed at init. So what you've done is replace the InitTrig_Rebellion function with one called "InitRebellion" that will never be called and thus never set up the spell's trigger. |
| 11-01-2007, 10:28 AM | #13 |
I was kidding with that "it's not so important" tone, of course it's essential. |
| 11-01-2007, 10:58 PM | #14 |
I was posting in response to Chop_tomato, not you Silvenon. :D |
| 11-02-2007, 09:14 AM | #15 |
I thought you were referring to both of us. My bad ![]() |
