Unit Spawn - Submission

[The_Spellweaver]

A bit sloppy at the end, hopefully it's alright.

Edit: Fixed some leaks

Spoiler: Code

scope UnitSpawn initializer Init
    globals
        private unit array barracks
        private integer array unitsData
    endglobals
   
    private function isPlayerForce takes nothing returns boolean
        return GetUnitTypeId(GetFilterUnit()) == 'hfoo' or GetUnitTypeId(GetFilterUnit()) == 'hrif'
    endfunction
   
    private function ExplodeAllUnits takes nothing returns nothing
        local unit u
        local group g = CreateGroup()
       
        call GroupEnumUnitsInRect(g, GetPlayableMapRect(), Filter(function isPlayerForce))
        loop
            set u = FirstOfGroup(g)
            exitwhen u == null
            call SetUnitExploded(u, true)
            call KillUnit(u)
            call GroupRemoveUnit(g, u)
        endloop
       
        call DestroyGroup(g)
        set g = null
        set u = null
    endfunction

    private function InitiateTraining takes nothing returns nothing
        local integer i = 0
        loop
            exitwhen i == 4
            if (barracks[i] != null) then
                call IssueTrainOrderByIdBJ(barracks[i], unitsData[GetRandomInt(0,1)])
            endif
            set i = i + 1
        endloop
    endfunction
   
    private function VictoryCondition takes nothing returns boolean
        set unitsData[2] = unitsData[2] - 1
        if (unitsData[2] == 0) then
            call ExplodeAllUnits()
            set barracks[0] = null
            set barracks[1] = null
            set barracks[2] = null
            set barracks[3] = null
            call DisplayTextToForce(GetPlayersAll(), "You won!")
        endif
        return TRUE
    endfunction

    private function Init takes nothing returns nothing
        local integer i = 0
        local timer t = CreateTimer()
        local real x
        local real y
        local trigger unitDiesTrigger = CreateTrigger()
        local player playerHostile = Player(PLAYER_NEUTRAL_AGGRESSIVE)
        local unit u
       
        call TriggerRegisterPlayerUnitEvent(unitDiesTrigger, playerHostile, EVENT_PLAYER_UNIT_DEATH, null)
        call TriggerAddCondition(unitDiesTrigger, function VictoryCondition)
       
        set unitsData[0] = 'hfoo'
        set unitsData[1] = 'hrif'
       
        // Spawning neutral hostile units
        loop
            exitwhen i == 20
            // There are no units on some lvls so they don't spawn sometimes
            loop
                set u = CreateUnit(playerHostile, ChooseRandomCreep(GetRandomInt(1, 10)), 0, 0, 0)
                exitwhen u != null
            endloop
            set unitsData[2] = unitsData[2] + 1
            set i = i + 1
        endloop
       
        // Creating player barracks
        set i = 0
        loop
            exitwhen i == 4
            set x = 3000 * Cos(bj_PI * (i * 0.5 - 0.25))
            set y = 3000 * Sin(bj_PI * (i * 0.5 - 0.25))
           
            set barracks[i] = CreateUnit(Player(i), 'hbar', x, y, bj_UNIT_FACING)
            call IssuePointOrder(barracks[i], "smart", 0, 0)
            set i = i + 1
        endloop
       
        call TimerStart(t, 2.5, true, function InitiateTraining)
       
        set t = null
        set unitDiesTrigger = null
        set playerHostile = null
        set u = null
    endfunction
endscope

 

[MyPad]

It looks like we meet again.

Anyway, I would suggest changing certain things:

1.) Change GetPlayableMapRect() into bj_mapInitialPlayableArea
2.) You can do away with the filterfunc isPlayerForce, replacing it with null and doing your filtering within the FirstOfGroup loop, copying the conditions of the former.
3.) If you like, you can change IssueTrainOrderByIdBJ into IssueImmediateOrderById
​

 

[IcemanBo]

@Jampion , @Tasyen , what do you think?

 

[Tasyen]

I assume that private means only visible in this scope ( I only used basic jass):
The map test seemd overall fine except the end.
there are some flaws in the code:

• There is an likely bugg, to spawn new Player units after the game is won (appeared on testing).
  
• you save UnitIds which you use for spawning. But in the "isPlayerForce"-filter you use hardcoded unitIds with the same values as the saved ones. ,maybe use the saved ones?
• One could object: The units don't spawn every 2.5 seconds. they spawn every ( 2.5 * x + 2 ) seconds. Don't know how here the mission's goal is.
  1. Spawn 2.5 timer + 2 training time = 4.5
  2. Spawn 5 timer + 2 training time = 7
     
  3. Spawn 7.5 timer + 2 training time = 9.5
  4. and so on.

• One could object that you did not check for the underlined Living Base and order possible dead ones to do stuff ( i did a simple test: the default barrack was removed, aka null, ca. 32 seconds after it was destroyed). -> Get 12 to 13 orders as dead ones before the null check works.
• It is good habit and faster for Messages to use the natives with Player using GetLocalPlayer() as argument, this BJ asks anway is GetLocalPlayer() in the force.

From the Blizzard.j:

function DisplayTextToForce takes force toForce, string message returns nothing
    if (IsPlayerInForce(GetLocalPlayer(), toForce)) then
       // Use only local code (no net traffic) within this block to avoid desyncs.
        call DisplayTextToPlayer(GetLocalPlayer(), 0, 0, message)
    endif
endfunction

• Offtopic: This check for GetLocalPlayer() applies for other Actions showing/hiding something not gamestate relevant using an force as argument from the BJs aswell.
• Offtopic: Using GetLocalPlayer() as Player argument is really good as long you do nothing gamestate relevant.

 

[Jampion]

MyPad and Tasyen said pretty much everything already.
I think there is still missing, that players 1-4 are allies, unless you did this in the force property menu.
I already mentioned it in another submission of that mission, but I think that's not the focus of this mission, so it does not need to be considered I think:
Some units have summon abilities, so in a real scenario 20 killed neutral units does not imply that there are none left.
I like that you considered, that creeps of some levels might not exist.

There is an easier solution: ChooseRandomCreep(-1)
Using -1 as parameter will choose any level (it will not choose a level that does not exist). It will also change the random distribution I assume. It probably will give every unit the same chance, whereas yours would give every level the same chance, so if there is only 1 level 10 creep and 100 level 1 creeps, the specific level 10 creep will have a much higher chance than a specific level 1 creep.
That's not something you need to change, but it's good to know, that -1 also works. If you look at the GUI equivalent it is explained there, but when only using JASS there is unfortunately no way to know this.