• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • ✅ Time to vote for the top 3 models! The POLL for Hive's 6th HD Modeling Contest: Mechanical is now open! 📅 Poll close on July 16, 2024! 🔗 Cast your vote now!

Script error.

Status
Not open for further replies.
Level 12
Joined
Dec 2, 2016
Messages
733
JASS:
scope spawnHellHounds initializer activate
//initializer OnInit

globals

public trigger spawner
integer hellDamage = 5
endglobals





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


function spawnHell takes nothing returns nothing

    if ( checker() ) then
        call CreateNUnitsAtLocFacingLocBJ( 1, 'n008', GetOwningPlayer(GetTriggerUnit()), GetUnitLoc(GetTriggerUnit()), GetSpellTargetLoc() )
        call AddSpecialEffectTargetUnitBJ( "head", GetLastCreatedUnit(), "Environment\\LargeBuildingFire\\LargeBuildingFire2.mdl" )
        call IssuePointOrderLocBJ( GetLastCreatedUnit(), "move", GetSpellTargetLoc() )
        set udg_TempPoint = GetSpellTargetLoc()
        call UnitApplyTimedLifeBJ( 5.00, 'BTLF', GetLastCreatedUnit() )
        call SelectUnitAddForPlayer( GetLastCreatedUnit(), GetOwningPlayer(GetTriggerUnit()) )
        call RemoveLocation(udg_TempPoint)
       call BlzSetUnitBaseDamage( GetLastCreatedUnit(), 1000*hellDamage, 0 )
    else
    endif
endfunction




function activate takes nothing returns nothing
    set spawner = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(spawner, EVENT_PLAYER_UNIT_SPELL_CHANNEL )
    call TriggerAddAction(spawner, function spawnHell)
endfunction






endscope

The following errors on this line:
JASS:
call BlzSetUnitBaseDamage( GetLastCreatedUnit(), 1000*hellDamage, 0 )

The output returns that Blz is an undeclared function.
I'm using codhar's jasshelper to compile this.
 
Level 41
Joined
Feb 27, 2007
Messages
5,179
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
 
Level 12
Joined
Dec 2, 2016
Messages
733
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


Thank you very much.

Vexorian JH seems to error with my code right now. A lot of the code in my game was written by previous developers. Using codahar's jasshelper.


For example, this error happens on compile: Screenshot - db85afd0851614e2a06d4501ab92548c - Gyazo


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?
 
Level 41
Joined
Feb 27, 2007
Messages
5,179
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.
 
Level 12
Joined
Dec 2, 2016
Messages
733
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.
 
Level 12
Joined
Dec 2, 2016
Messages
733
So I started redoing the code. I got through three scripts re-writing loops. And then ran into this issue.

Screenshot - 4f8a69359fe25453be962ab1779f5814 - Gyazo

JASS:
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.
 
Level 41
Joined
Feb 27, 2007
Messages
5,179
But what I don't understand is what is 'for pc = pc downto 0' specifically the 'downto' what does that mean.
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
 
Level 12
Joined
Dec 2, 2016
Messages
733
Thanks,

This is the next error I ran into.

I don't understand how this for loop reads.

Lets say auracount is 4.

'for (0 <= i < auracount)'

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)];
}
 
Level 41
Joined
Feb 27, 2007
Messages
5,179
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
 
Last edited:
Level 12
Joined
Dec 2, 2016
Messages
733
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


Do I have this right?



JASS:
function GetAura() -> integer {
    integer i;
    integer j;
    integer min = 10;
    integer usedlist[];
    integer randomlist[];
    integer randomcount = 0;

      
      
   loop
       exitwhen i > auracount
       usedlist[i] = 0;
       set i = i+1
   endloop
      
      
      
      

    // Determine how many of each aura are in play
  
       loop
       exitwhen i > 12
       loop
       exitwhen j <= auracount
       if (players[i].minionAura == auralist[j] && players[i].state == STATE_PLAYING)
                usedlist[j] += 1;
      
   endloop
   endloop
            

   loop
   exitwhen i > auracount
          if (min > usedlist[i])
            // Ignore if limited aura
            if (not(auralist[i] == ID_REPLACE_UNHOLY_AURA && min > 0))
                min = usedlist[i]; 
   endloop
  
  
   loop
   exitwhen 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)];
   endloop

}
 
Level 41
Joined
Feb 27, 2007
Messages
5,179
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.
 
Level 12
Joined
Dec 2, 2016
Messages
733
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.
 
Level 41
Joined
Feb 27, 2007
Messages
5,179
0 <= i < auracount does 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
 
Level 12
Joined
Dec 2, 2016
Messages
733
Am I getting this right?
Thanks for the help btw.

Old:

JASS:
 // 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
 
Level 41
Joined
Feb 27, 2007
Messages
5,179
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!
 
Status
Not open for further replies.
Top