That function was added in 1.29 I believe, and the version of standalone JASSHelper maintained by cohadar is years out of date. Instead you should use WEX, and then replace the created addresses.xml file with this one (after updating to 1.29 if you haven't yet, though I imagine you have).
I would recommend you try to replace as many BJs as you can in your code (red functions in [code=jass] tags) as that's a practice that can never be started too early. Most of them are just wrappers, so when you convert GUI code to see the function names just look the function up in the WEX trigger editor's "function list" browser. It will show you that when you call call UnitApplyTimedLifeBJ( 5.00, 'BTLF', GetLastCreatedUnit() ) what it's doing is:
JASS:
function UnitApplyTimedLifeBJ takes real duration, integer buffId, unit whichUnit returns nothing
call UnitApplyTimedLife(whichUnit, buffId, duration)
endfunction
//spoiler alert
//...
//...
//it's the same arguments in a different order!
//stupid
//look what this one does:
function CreateNUnitsAtLoc takes integer count, integer unitId, player whichPlayer, location loc, real face returns group
call GroupClear(bj_lastCreatedGroup)
loop
set count = count - 1
exitwhen count < 0
call CreateUnitAtLocSaveLast(whichPlayer, unitId, loc, face)
call GroupAddUnit(bj_lastCreatedGroup, bj_lastCreatedUnit)
endloop
return bj_lastCreatedGroup
endfunction
In general BJ functions are just wrappers for JASS natives and so calling them is strictly slower and less efficient than the natives directly. There are, however, cases where this doesn't matter. Setup is a good one. Excluding things like execution limit for onInit() functions you can run BJs there and it won't matter; in fact a lot of them save you time when writing code. I love this one:
JASS:
function TriggerRegisterAnyUnitEventBJ takes trigger trig, playerunitevent whichEvent returns nothing
local integer index
set index = 0
loop
call TriggerRegisterPlayerUnitEvent(trig, Player(index), whichEvent, null)
set index = index + 1
exitwhen index == bj_MAX_PLAYER_SLOTS
endloop
endfunction
Look at them, see for yourself if it does something useful. Lots of GUI code uses locations but the JASS natives will take real coordinates. Reals are primitives, locations are handles which eat more memory and aren't automatically deleted, so that's why most JASS code uses real coords instead of location based functions.
Next item to check off is this stupid business the WE creates for conditions....
JASS:
if ( not ( GetSpellAbilityId() == 'A046' ) ) then
return false
endif
return true
Instead of an if statement you can simply return the boolean comparison itself, since it's a boolean. return GetSpellAbilityId() == 'A046', nothing more required. In this case, though you don't even need a checker function and could do it right in the if statment in spawnHell, that leaves your trigger without any conditions and it will fire on every spell cast by every unit on the map. Conditions are actually super fast to evaluate so it's cool if they have to a lot. I'll show how to add that function as a condition later below.
A note about your usage of variables: good on the globals, but your functions can be streamlined and improved with more local variables to reduce the number of function calls. Something like local unit u = GetTriggerUnit() //use u elsewhere in code saves you 2 in spawnHell. You should also probably have hellDamage public too just in case you need that name for something else somewhere else. Then you make more globals for all your configurables so it's nice and easy to do so! For primitives that won't change at all (for example: always the string "let's go" never anything else) you can also add the constant keyword and JASSHelper will inline it to reduce function calls.
Good job with removing temppoint once but you did it in the wrong order and missed some others, so you need to fully understand leaks so here is a good resource to understand since I don't want to type all that. Ultimately you leak an effect that you create on each hellhound that never gets destroyed, so instead I suggest you attach the model you want to the hellhounds with a useless passive item ability as detailed in this tutorial instead (which won't leak). Also use EVENT_PLAYER_UNIT_SPELL_EFFECT to detect casts, CHANNEL should only be used for channeling spells. All cleaned up your code should ideally look something like this:
JASS:
scope spawnHellHounds initializer activate
//initializer OnInit
globals
public trigger spawner = null
public integer hellDamage = 5
public constant integer SPELL_RAW = 'A046'
public constant integer HOUND_RAW = 'n008'
endglobals
function checker takes nothing returns boolean
return GetSpellAbilityId() == SPELL_RAW
endfunction
function spawnHell takes nothing returns nothing
local unit u = GetTriggerUnit()
local player p = GetOwningPlayer(u)
local real x = GetUnitX(u)
local real y = GetUnitY(u)
local real tx = GetSpellTargetX()
local real ty = GetSpellTargetY()
local real a = Atan2(ty-y,tx-x) //this angle is in radians, not degrees!
local unit h = CreateUnit(p,HOUND_RAW,x,y,a)
//set fx = AddSpecialEffectTarget("Environment\\LargeBuildingFire\\LargeBuildingFire2.mdl", h, "head")
//this would leak, I suggest a better way in the thread
call IssuePointOrder(h,"move",tx,ty)
call UnitApplyTimedLife(h,'BTLF',5.00)
// selecting for one player, I rewrote the BJ here because that's best for the scenario...
// but you can use the BJ directly if you want
// beware GetLocalPlayer() descyncs if used incorrectly, this is correct
if (GetLocalPlayer() == p) then
call SelectUnit(h, true)
endif
call BlzSetUnitBaseDamage(h,1000*hellDamage,0)
endfunction
function activate takes nothing returns nothing
set spawner = CreateTrigger()
call TriggerRegisterAnyUnitEventBJ(spawner, EVENT_PLAYER_UNIT_SPELL_EFFECT)
call TriggerAddCondition(spawner, Condition(function checker))
call TriggerAddAction(spawner, function spawnHell)
endfunction
endscope
That function was added in 1.29 I believe, and the version of standalone JASSHelper maintained by cohadar is years out of date. Instead you should use WEX, and then replace the created addresses.xml file with this one (after updating to 1.29 if you haven't yet, though I imagine you have).
I would recommend you try to replace as many BJs as you can in your code (red functions in [code=jass] tags) as that's a practice that can never be started too early. Most of them are just wrappers, so when you convert GUI code to see the function names just look the function up in the WEX trigger editor's "function list" browser. It will show you that when you call call UnitApplyTimedLifeBJ( 5.00, 'BTLF', GetLastCreatedUnit() ) what it's doing is:
JASS:
function UnitApplyTimedLifeBJ takes real duration, integer buffId, unit whichUnit returns nothing
call UnitApplyTimedLife(whichUnit, buffId, duration)
endfunction
//spoiler alert
//...
//...
//it's the same arguments in a different order!
//stupid
//look what this one does:
function CreateNUnitsAtLoc takes integer count, integer unitId, player whichPlayer, location loc, real face returns group
call GroupClear(bj_lastCreatedGroup)
loop
set count = count - 1
exitwhen count < 0
call CreateUnitAtLocSaveLast(whichPlayer, unitId, loc, face)
call GroupAddUnit(bj_lastCreatedGroup, bj_lastCreatedUnit)
endloop
return bj_lastCreatedGroup
endfunction
In general BJ functions are just wrappers for JASS natives and so calling them is strictly slower and less efficient than the natives directly. There are, however, cases where this doesn't matter. Setup is a good one. Excluding things like execution limit for onInit() functions you can run BJs there and it won't matter; in fact a lot of them save you time when writing code. I love this one:
JASS:
function TriggerRegisterAnyUnitEventBJ takes trigger trig, playerunitevent whichEvent returns nothing
local integer index
set index = 0
loop
call TriggerRegisterPlayerUnitEvent(trig, Player(index), whichEvent, null)
set index = index + 1
exitwhen index == bj_MAX_PLAYER_SLOTS
endloop
endfunction
Look at them, see for yourself if it does something useful. Lots of GUI code uses locations but the JASS natives will take real coordinates. Reals are primitives, locations are handles which eat more memory and aren't automatically deleted, so that's why most JASS code uses real coords instead of location based functions.
Next item to check off is this stupid business the WE creates for conditions....
JASS:
if ( not ( GetSpellAbilityId() == 'A046' ) ) then
return false
endif
return true
Instead of an if statement you can simply return the boolean comparison itself, since it's a boolean. return GetSpellAbilityId() == 'A046', nothing more required. In this case, though you don't even need a checker function and could do it right in the if statment in spawnHell, that leaves your trigger without any conditions and it will fire on every spell cast by every unit on the map. Conditions are actually super fast to evaluate so it's cool if they have to a lot. I'll show how to add that function as a condition later below.
A note about your usage of variables: good on the globals, but your functions can be streamlined and improved with more local variables to reduce the number of function calls. Something like local unit u = GetTriggerUnit() //use u elsewhere in code saves you 2 in spawnHell. You should also probably have hellDamage public too just in case you need that name for something else somewhere else. Then you make more globals for all your configurables so it's nice and easy to do so! For primitives that won't change at all (for example: always the string "let's go" never anything else) you can also add the constant keyword and JASSHelper will inline it to reduce function calls.
Good job with removing temppoint once but you did it in the wrong order and missed some others, so you need to fully understand leaks so here is a good resource to understand since I don't want to type all that. Ultimately you leak an effect that you create on each hellhound that never gets destroyed, so instead I suggest you attach the model you want to the hellhounds with a useless passive item ability as detailed in this tutorial instead (which won't leak). Also use EVENT_PLAYER_UNIT_SPELL_EFFECT to detect casts, CHANNEL should only be used for channeling spells. All cleaned up your code should ideally look something like this:
JASS:
scope spawnHellHounds initializer activate
//initializer OnInit
globals
public trigger spawner = null
public integer hellDamage = 5
public constant integer SPELL_RAW = 'A046'
public constant integer HOUND_RAW = 'n008'
endglobals
function checker takes nothing returns boolean
return GetSpellAbilityId() == SPELL_RAW
endfunction
function spawnHell takes nothing returns nothing
local unit u = GetTriggerUnit()
local player p = GetOwningPlayer(u)
local real x = GetUnitX(u)
local real y = GetUnitY(u)
local real tx = GetSpellTargetX()
local real ty = GetSpellTargetY()
local real a = Atan2(ty-y,tx-x) //this angle is in radians, not degrees!
local unit h = CreateUnit(p,HOUND_RAW,x,y,a)
//set fx = AddSpecialEffectTarget("Environment\\LargeBuildingFire\\LargeBuildingFire2.mdl", h, "head")
//this would leak, I suggest a better way in the thread
call IssuePointOrder(h,"move",tx,ty)
call UnitApplyTimedLife(h,'BTLF',5.00)
// selecting for one player, I rewrote the BJ here because that's best for the scenario...
// but you can use the BJ directly if you want
// beware GetLocalPlayer() descyncs if used incorrectly, this is correct
if (GetLocalPlayer() == p) then
call SelectUnit(h, true)
endif
call BlzSetUnitBaseDamage(h,1000*hellDamage,0)
endfunction
function activate takes nothing returns nothing
set spawner = CreateTrigger()
call TriggerRegisterAnyUnitEventBJ(spawner, EVENT_PLAYER_UNIT_SPELL_EFFECT)
call TriggerAddCondition(spawner, Condition(function checker))
call TriggerAddAction(spawner, function spawnHell)
endfunction
endscope
I know codahar's allowed for different for loop syntax. But do you know if it's just loops I need to rewrite or is their a lot of other syntax/functions that could possibly need changing to work with Vexorians?
Yeah that error specifically is coming from the For...EndFor loops he implemented, but there are bound to be other things that use some modified syntax.
Rewriting the affected bits of code actually shouldn't be that hard. Save, find error, fix error, repeat. If you are incapable I can probably go through the map but it may take me a bit to get to it.
Yeah that error specifically is coming from the For...EndFor loops he implemented, but there are bound to be other things that use some modified syntax.
Rewriting the affected bits of code actually shouldn't be that hard. Save, find error, fix error, repeat. If you are incapable I can probably go through the map but it may take me a bit to get to it.
Okay, yeah I went through it a while back and was slowly starting to change the code. I was able to fix the loops but ran into another problem forgot what it was but was out of my experience level.
Thanks man, I'll either message you or make another post here in a week or so. I've still got a lot of other things to do first. Appreciate your help.
scope preloadUnits initializer OnInit
globals
private unit array pu
private integer pc = 0
endglobals
private function RemovePreloads takes nothing returns nothing
for pc = pc downto 0
call ReleaseUnit(pu[pc])
set pu[pc] = null
endfor
call DestroyTimer(GetExpiredTimer())
endfunction
private function PreloadUnit takes integer id returns nothing
set pu[pc] = CreateUnit(Player(15), id, 2600, 6400, 270)
set pc = pc + 1
endfunction
public function OnInit takes nothing returns nothing
local timer t = CreateTimer()
call TimerStart(t, .0, false, function RemovePreloads)
set t = null
call PreloadUnit('U00E')
call PreloadUnit('E001')
call PreloadUnit('E002')
call PreloadUnit('AFVF')
call PreloadUnit('U00C')
call PreloadUnit('U00B')
call PreloadUnit('U00K')
call PreloadUnit('U00D')
call PreloadUnit('U009')
call PreloadUnit('U007')
call PreloadUnit('U008')
call PreloadUnit('U00A')
call PreloadUnit('HSyM')
call PreloadUnit('H01L')
call Preloader("scripts\\VampirismFire.pld")
endfunction
endscope
Any idea how I can change this script. I see that it's using a for loop. But what I don't understand is what is 'for pc = pc downto 0' specifically the 'downto' what does that mean. And how can I rewrite this? Thanks.
Loop that counts down rather than up. pc, pc-1, pc-2 ... 2, 1, 0. I have absolutely no idea if his for/endfor loops reset the value of the looped variable afterwards or not... so maybe comment out that stuff and see if it breaks the preloader
JASS:
private function RemovePreloads takes nothing returns nothing
//local integer tmp = pc
loop
call ReleaseUnit(pu[pc])
set pu[pc] = null
set pc = pc-1
exitwhen pc < 0
endloop
call DestroyTimer(GetExpiredTimer())
//set pc = tmp
endfunction
Does this mean, while i <= 0 and i < auracount loop?
JASS:
function GetAura() -> integer {
integer i;
integer j;
integer min = 10;
integer usedlist[];
integer randomlist[];
integer randomcount = 0;
// Initialise to 0
for (0 <= i < auracount)
usedlist[i] = 0;
// Determine how many of each aura are in play
for (0 <= i < 12)
for (0 <= j <= auracount)
if (players[i].minionAura == auralist[j] && players[i].state == STATE_PLAYING)
usedlist[j] += 1;
// Get the least used aura
for (0 <= i < auracount)
if (min > usedlist[i])
// Ignore if limited aura
if (not(auralist[i] == ID_REPLACE_UNHOLY_AURA && min > 0))
min = usedlist[i];
// Create list of least used auras
for (0 <= i < auracount)
if (usedlist[i] == min)
// Ignore if limited aura
if (not(auralist[i] == ID_REPLACE_UNHOLY_AURA && min > 0)) {
randomlist[randomcount] = auralist[i];
randomcount += 1;
}
return randomlist[GetRandomInt(0, randomcount - 1)];
}
Yeah it's a loop over i from (and including) 0 up to (not including) i = auracount. So: 0, 1, 2 ... (auracount-1), (auracount) EXITWHEN without operating on auracount. If it says 0 <= i <= auracount it instead includes auracount in the loop then exits after: 0, 1, 2 ... (auracount-1), (auracount), (auracount+1) EXITWHEN.
JASS:
//This way is 'intuitive'
// n <= i <= m
set i = n
loop
exitwhen i > m
//
set i = i+1
endloop
// n <= i < m
set i = n
loop
exitwhen i >= m
//
set i = i+1
endloop
// n < i <= m
set i = n+1
loop
exitwhen i > m
//
set i = i+1
endloop
// n < i < m
set i = n+1
loop
exitwhen i >= m
//
set i = i+1
endloop
JASS:
//Or this way if it's visually pleasing to you
// n <= i <= m
set i = n-1
loop
set i = i+1
exitwhen i > m
//
endloop
// n <= i < m
set i = n-1
loop
set i = i+1
exitwhen i >= m
//
endloop
// n < i <= m
set i = n
loop
set i = i+1
exitwhen i > m
//
endloop
// n < i < m
set i = n
loop
set i = i+1
exitwhen i >= m
//
endloop
Yeah it's a loop over i from (and including) 0 up to (not including) i = auracount. So: 0, 1, 2 ... (auracount-1), (auracount) EXITWHEN without operating on auracount. If it says 0 <= i <= auracount it instead includes auracount in the loop then exits after: 0, 1, 2 ... (auracount-1), (auracount), (auracount+1) EXITWHEN.
JASS:
//This way is 'intuitive'
// n <= i <= m
set i = n
loop
exitwhen i > m
//
set i = i+1
endloop
// n <= i < m
set i = n
loop
exitwhen i >= m
//
set i = i+1
endloop
// n < i <= m
set i = n+1
loop
exitwhen i > m
//
set i = i+1
endloop
// n < i < m
set i = n+1
loop
exitwhen i >= m
//
set i = i+1
endloop
JASS:
//Or this way if it's visually pleasing to you
// n <= i <= m
set i = n-1
loop
set i = i+1
exitwhen i > m
//
endloop
// n <= i < m
set i = n-1
loop
set i = i+1
exitwhen i >= m
//
endloop
// n < i <= m
set i = n
loop
set i = i+1
exitwhen i > m
//
endloop
// n < i < m
set i = n
loop
set i = i+1
exitwhen i >= m
//
endloop
You need to set i=0 before you loop over i, and similarly for j, and many of your loops are missing those. Your first loop's exitwhen should be i >= auracount, same with every for loop that was 0<=i<auracount. Your second one is checking if j< when it should be j>. You dropped a few iterators (set i=i+1) too. In general you need to go over what I posted; there are different combinations of > and = depending on where in the original n,i,m equation there are equals signs.
I'm trying to understand this. This is the old code:
JASS:
// Initialise to 0
for (0 <= i < auracount)
usedlist[i] = 0;
New code:
JASS:
set i = 0
loop
usedlist[i] = 0;
set i = i + 1
exitwhen i > auracount
//
endloop
Why do you have to set ' i ' before the loop? The i integer was declared at the top of the function(Or is it because just creating the variable makes it null?; But anyway lets just start with this first loop, does this look right?
I set the array index 0 to 0, increment i and continually set each index to 0 until i is greater than the aura count.
0 <= i < auracountdoes not operate on i=auracount; it exits the loop after (auracount-1). So you need to use exitwhen i >= auracount. Think of it this way: if the upper bound using a for does not have an = sign in it, then your exitwhen line must have an >=. If the upper bound in the for does have an = sign in it, then your exitwhen line will not have an =.
Primitive types can't be "null". They are 0, 0.00, false, and the empty string "" for integers, reals, booleans, and strings respectively. If you do not initialize the variable before you use it you'll crash the thread:
JASS:
local integer a = 15
local integer b
call SomeFunc(a) //no crash
call SomeFunc(b) //thread crash
Here the reason you need to set i before each loop is because if you're going from i to i+1 to i+2... after the loop i is not equal to 0, which you want it to be to start the next loop (since all of the loops you've posted here start at 0). This is especially easy to forget in nested loops (when you use j above) and will really matter there.
JASS:
function bad takes integer END returns nothing
local integer i = 0
local integer j = 0
loop
exitwhen i >= END
loop
exitwhen j >= END
call BJDebugMsg(I2S(i)+"-"+I2S(j))
set j = j+1
endloop
set i = i+1
endloop
endfunction
function good takes integer END returns nothing
local integer i = 0
local integer j // = 0 dont need this because its initialized each loop below
loop
exitwhen i >= END
# # set j = 0
loop
exitwhen j >= END
call BJDebugMsg(I2S(i)+"-"+I2S(j))
set j = j+1
endloop
set i = i+1
endloop
endfunction
call bad(3) //prints: 0-0, 0-1, 0-2, 1-3, 1-4, 1-5, 2-6, 2-7, 2-8
call good(3) //prints: 0-0, 0-1, 0-2, 1-0, 1-1, 1-2, 2-0, 2-1, 2-2
// Initialise to 0
for (0 <= i < auracount)
usedlist[i] = 0;
New:
JASS:
set i = 0
loop
usedlist[i] = 0;
set i = i + 1
exitwhen i >= auracount
endloop
Old:
JASS:
// Determine how many of each aura are in play
for (0 <= i < 12)
for (0 <= j <= auracount)
if (players[i].minionAura == auralist[j] && players[i].state == STATE_PLAYING)
usedlist[j] += 1;
New:
JASS:
set i = 0
set j = 0
loop
exitwhen i >= 12
set i = i + 1
loop
exitwhen j>= auracount
set j = j + 1
if (players[i].minionAura == auralist[j] && players[i].state == STATE_PLAYING)
usedlist[j] += 1;
endloop
endloop
Old:
JASS:
// Get the least used aura
for (0 <= i < auracount)
if (min > usedlist[i])
// Ignore if limited aura
if (not(auralist[i] == ID_REPLACE_UNHOLY_AURA && min > 0))
min = usedlist[i];
New:
JASS:
set i = 0
loop
exitwhen i > auracount
set i = i + 1
if (min > usedlist[i])
// Ignore if limited aura
if (not(auralist[i] == ID_REPLACE_UNHOLY_AURA && min > 0))
min = usedlist[i];
endloop
Old:
JASS:
// Create list of least used auras
for (0 <= i < auracount)
if (usedlist[i] == min)
// Ignore if limited aura
if (not(auralist[i] == ID_REPLACE_UNHOLY_AURA && min > 0)) {
randomlist[randomcount] = auralist[i];
randomcount += 1;
}
New:
JASS:
set i = 0
loop
exitwhen i > auracount
set i = i + 1
if (usedlist[i] == min)
// Ignore if limited aura
if (not(auralist[i] == ID_REPLACE_UNHOLY_AURA && min > 0)) {
randomlist[randomcount] = auralist[i];
randomcount += 1;
}
endloop
1. Move your exitwhen to the top of the loop. If auracount = 0 it needs to exit the loop before it attempts to do things. In general I always put the exitwhen first or second line in the loop (only after whatever sets my iterator variable, if for example doing a group loop that's set u = FirstOfGroup(g)).
2. Move your i = i+1 and j = j+1 lines to be the last lines of their loops because the way you've written it skips over i=0 and j=0. Your j exitwhen is wrong, it should be > not >= (again as i said above because the upper bound DOES have an = your exitwhen should not have one).
3. Move i = i+1 to the bottom of the loop.
4. You forgot your exitwhen i >= auracount
General: You need to write all of the SOMEVAR += 1 to be set SOMEVAR = SOMEVAR+1. You've also forgotten a few sets in many of these. You need to remove the ;s at the end of lines and remake the if blocks to not use {} braces/. I believe && is a cohadar thing and needs to become and. For the love of god indent your code properly!
This site uses cookies to help personalise content, tailor your experience and to keep you logged in if you register.
By continuing to use this site, you are consenting to our use of cookies.
