Wanderer - Submission

Rigborn · Mar 17, 2019

This includes both part 1 and 2

JASS:

scope Wanderer initializer init

globals
   private hashtable h
endglobals

public function periodic takes nothing returns nothing
   local timer t = GetExpiredTimer()
   local group tempgroup = CreateGroup()
   local real ang
   local real dist
   local real x
   local real y
   local unit tempunit
   local integer id
   local unit gryid
   set x = LoadReal(h, GetHandleId(t), 0)
   set y = LoadReal(h, GetHandleId(t), 1)
   call GroupEnumUnitsInRange(tempgroup, x, y, 50.00, null)
   loop
       set tempunit = FirstOfGroup(tempgroup)
       set id = GetHandleId(tempunit)
       set gryid = LoadUnitHandle(h, GetHandleId(t), 2)
       exitwhen tempunit == null
       if tempunit == gryid then
           call SetTerrainType(LoadReal(h, GetHandleId(t), 0), LoadReal(h, GetHandleId(t), 1), LoadInteger(h, GetHandleId(t), 4), LoadInteger(h, GetHandleId(t), 3), 1, 0)
           set dist = GetRandomReal(500.00, 1500.00)
           set ang = GetRandomReal(0.00, 360)*bj_PI/180
           set x = GetUnitX(tempunit)+dist*Cos(ang)
           set y = GetUnitY(tempunit)+dist*Sin(ang)
           call IssuePointOrderById(tempunit, 851986, x, y)
           call GroupRemoveUnit(tempgroup, tempunit)
           call PauseTimer(t)
       endif
   endloop
   call DestroyGroup(tempgroup)
   set tempgroup = null
   set tempunit = null
   set t = null
endfunction          
          
public function actions takes nothing returns boolean
   local timer t
   local integer order_id
   local real orderx
   local real ordery
   local integer getrandomint
   local integer terraintype
   local real ang = GetRandomReal(0.00, 360)*bj_PI/180
   local real dist = GetRandomReal(500.00, 1500.00)
   local real a
   local real b
  
   set orderx = GetOrderPointX()
   set ordery = GetOrderPointY()
   if RectContainsCoords(GetPlayableMapRect(), orderx, ordery) == false then
       call BJDebugMsg("Unit tried to move an area outside the map. Ordering unit to move again.")
       set a = GetUnitX(GetTriggerUnit())+dist*Cos(ang)
       set b = GetUnitY(GetTriggerUnit())+dist*Sin(ang)
       call IssuePointOrderById(GetTriggerUnit(), 851986, a, b)
   endif
   set order_id = GetIssuedOrderId()
   if order_id == 851986 then
       set t = CreateTimer()
       call SaveReal(h, GetHandleId(t), 0, orderx)
       call SaveReal(h, GetHandleId(t), 1, ordery)
       call SaveUnitHandle(h, GetHandleId(t), 2, GetTriggerUnit())
       set getrandomint = GetRandomInt(0,2)
       set terraintype = GetTerrainType(orderx, ordery)
       call SaveInteger(h, GetHandleId(t), 3, getrandomint)
       call SaveInteger(h, GetHandleId(t), 4, terraintype)
       call SetTerrainType(LoadReal(h, GetHandleId(t), 0), LoadReal(h, GetHandleId(t), 1), 'Nsnw', getrandomint, 1, 0)
       call TimerStart(t, 0.30, true, function periodic)
       set t = null
   endif
   return false
endfunction

public function init takes nothing returns nothing
   local real ang = GetRandomReal(0.00, 360)*bj_PI/180
   local real dist = GetRandomReal(500.00, 1500.00)
   local real a
   local real b
   local unit Gryphon
   local trigger t
   set h = InitHashtable()
   set t = CreateTrigger()
   call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_ISSUED_POINT_ORDER)
   call TriggerAddCondition(t, Condition(function actions))
   set Gryphon = CreateUnit(Player(0), 'hgry', 0.00, 0.00, 0.00)
   set a = GetUnitX(Gryphon)+dist*Cos(ang)
   set b = GetUnitY(Gryphon)+dist*Sin(ang)
   call IssuePointOrderById(Gryphon, 851986, a, b)
   set Gryphon = null
   set t = null
endfunction
endscope

IcemanBo · Mar 17, 2019

It looks mostly good, but something to note.

It would be better to bind the unit directly to the checking timer instance. Enumerating all units in range of "50" might work in most cases, especially in the demo map, but it is kind of a less clean solution.

Also some other minor notes just as personal thoughts:

variable names like "getrandomint" for purpose of terrain tile variation is not very expressive, and hard to read. At best a name always describes its purpose, "terrainVariation" etc..
when the unit is ordered a new order (world bounds check) then the rest of the trigger should not run anymore
if many same calls are used, then it's easier to read if locals are used.. for GetHandleId(t) for example
one might always look it up, but it's also good if magic numbers are explained, 851986 . But it probably is just something, or something similar like move order one might assume

Rigborn · Mar 22, 2019

Here's the update:

JASS:

scope Wanderer initializer init
globals
private hashtable h
endglobals

public function periodic takes nothing returns nothing
   local timer t = GetExpiredTimer()
   local real ang
   local real dist
   local real x
   local real y
   local real x2
   local real y2
   local unit Gryphon
   local integer orderMove = 851986
   local integer handleId = GetHandleId(t)
  

   set Gryphon = LoadUnitHandle(h, handleId, 0)
   set x = LoadReal(h, handleId, 2)
   set y = LoadReal(h, handleId, 3)
   set x2 = GetUnitX(Gryphon)
   set y2 = GetUnitY(Gryphon)
   set dist = SquareRoot(((x2-x)*(x2-x))+((y2-y)*(y2-y)))
   if dist < 20.00 then // if I set it lower than this, the condition never becomes true. I guess it has something to do with either collision or tiles.
       call SetTerrainType(LoadReal(h, handleId, 2), LoadReal(h, handleId, 3), LoadInteger(h,handleId, 5), LoadInteger(h, handleId, 4), 1, 0)
       set dist = GetRandomReal(500.00, 1500.00)
       set ang = GetRandomReal(0.00, 360)*bj_PI/180
       set x = GetUnitX(Gryphon)+dist*Cos(ang)
       set y = GetUnitY(Gryphon)+dist*Sin(ang)
       call IssuePointOrderById(Gryphon, orderMove, x, y)
   endif
   set t = null
endfunction          
          
          
public function actions takes nothing returns boolean
   local timer t = CreateTimer()
   local integer order_id
   local integer orderMove = 851986
   local real orderX
   local real orderY
   local integer terrainVariation
   local integer terrainType
   local real ang = GetRandomReal(0.00, 360)*bj_PI/180
   local real dist = GetRandomReal(500.00, 1500.00)
   local real a
   local real b
   local integer handleId = GetHandleId(t)
   local unit Gryphon = LoadUnitHandle(h, 0, 0)
  
  
   call SaveUnitHandle(h, handleId, 0, Gryphon)
   set orderX = GetOrderPointX()
   set orderY = GetOrderPointY()
   if RectContainsCoords(GetPlayableMapRect(), orderX, orderY) == false then
       call PauseTimer(t)
       call DestroyTimer(t)
       set t = null
   endif
   set order_id = GetIssuedOrderId()
   if order_id == orderMove then
       call SaveReal(h, handleId, 2, orderX)
       call SaveReal(h, handleId, 3, orderY)
       set terrainVariation = GetRandomInt(0,2)
       set terrainType = GetTerrainType(orderX, orderY)
       call SaveInteger(h, handleId, 4, terrainVariation)
       call SaveInteger(h, handleId, 5, terrainType)
       call SetTerrainType(orderX, orderY, 'Nsnw', terrainVariation, 1, 0)
       call TimerStart(t, 0.30, true, function periodic)
       set t = null
   endif
   return false
endfunction

public function init takes nothing returns nothing
   local real ang = GetRandomReal(0.00, 360)*bj_PI/180
   local real dist = GetRandomReal(500.00, 1500.00)
   local real a
   local real b
   local unit Gryphon
   local trigger tr
   local integer orderMove = 851986 // move order id
   set tr = CreateTrigger()
   set h = InitHashtable()
   call TriggerRegisterAnyUnitEventBJ(tr, EVENT_PLAYER_UNIT_ISSUED_POINT_ORDER)
   call TriggerAddCondition(tr, Condition(function actions))
   set Gryphon = CreateUnit(Player(0), 'hgry', 0.00, 0.00, 0.00)
   call SaveUnitHandle(h, 0, 0, Gryphon)
   set a = GetUnitX(Gryphon)+dist*Cos(ang)
   set b = GetUnitY(Gryphon)+dist*Sin(ang)
   call IssuePointOrderById(Gryphon, orderMove, a, b)
   set Gryphon = null
   set tr = null
endfunction
endscope

IcemanBo · Mar 22, 2019

A timer is created each move command, which is then started periodically. It should be created just once and then started, it may for example happen in init.
The usage of both literal values "0" as hashtable keys is not purposeful for hashtable mission, it would be same and easier to work with globals (which should be avoided)

===

Names do look better
Many BJ functions, in this case GetPlayableMapRect(), are a simple wrapper with no extra functionality
In such cases it's better to just use the native JASS code behind. One may look up what those BJs do in the blizzard.j

Rigborn · Mar 23, 2019

Removed point order event
Moved timer to init function
Got rid of BJs

JASS:

scope Wanderer initializer init
globals
public hashtable h
endglobals

public function onloop takes nothing returns nothing
   local timer t = GetExpiredTimer()
   local integer handleId = GetHandleId(t)
   local unit Gryphon = LoadUnitHandle(h, handleId, 1)
   local integer phase = LoadInteger(h, handleId, 2)
   local integer orderMove = 851986
   local real x
   local real x2
   local real y
   local real y2
   local real dist
   local real ang
   local integer terrainType
   local integer terrainVariation
   
   if phase == 1 then
       set dist = GetRandomReal(500, 1500)
       set ang = GetRandomReal(0, 360)*bj_PI/180
       set x2 = GetUnitX(Gryphon)+dist*Cos(ang)
       set y2 = GetUnitY(Gryphon)+dist*Sin(ang)
       if (GetRectMinX(bj_mapInitialPlayableArea) <= x2) and (x2 <= GetRectMaxX(bj_mapInitialPlayableArea)) and (GetRectMinY(bj_mapInitialPlayableArea) <= y2) and (y2 <= GetRectMaxY(bj_mapInitialPlayableArea)) then
           set terrainType = GetTerrainType(x2, y2)
           set terrainVariation = GetRandomInt(0, 2)
           call SaveReal(h, handleId, 3, x2)
           call SaveReal(h, handleId, 4, y2)
           call SaveInteger(h, handleId, 5, terrainType)
           call SaveInteger(h, handleId, 6, terrainVariation)
           call SetTerrainType(x2, y2, 'Nsnw', terrainVariation, 1, 0)
           call IssuePointOrderById(Gryphon, orderMove, x2, y2)
           set phase = 2
           call SaveInteger(h, handleId, 2, phase)
           call TimerStart(t, 0.30, true, function onloop)
       else
           call IssueImmediateOrder(Gryphon, "stop")
           call PauseTimer(t)
           call DestroyTimer(t)
       endif
   elseif phase == 2 then
       set x = GetUnitX(Gryphon)
       set y = GetUnitY(Gryphon)
       set x2 = LoadReal(h, handleId, 3)
       set y2 = LoadReal(h, handleId, 4)
       set terrainType = LoadInteger(h, handleId, 5)
       set terrainVariation = LoadInteger(h, handleId, 6)
       set dist = SquareRoot(((x2-x)*(x2-x))+((y2-y)*(y2-y)))
       if dist < 20.00 then
           call SetTerrainType(x2, y2, terrainType, terrainVariation, 1, 0)
           set phase = 1
           call SaveInteger(h, handleId, 2, phase)
       endif
   endif
endfunction
public function init takes nothing returns nothing
   local real ang = GetRandomReal(0.00, 360)*bj_PI/180
   local real dist = GetRandomReal(500.00, 1500.00)
   local real a
   local real b
   local unit Gryphon
   local trigger tr
   local integer orderMove = 851986 // move order id
   local timer t = CreateTimer()
   local integer handleId = GetHandleId(t)
   local integer phase = 1
   
   set tr = CreateTrigger()
   set h = InitHashtable()
   set Gryphon = CreateUnit(Player(0), 'hgry', 0.00, 0.00, 0.00)
   call SaveUnitHandle(h, handleId, 1, Gryphon)
   set a = GetUnitX(Gryphon)+dist*Cos(ang)
   set b = GetUnitY(Gryphon)+dist*Sin(ang)
   call SaveInteger(h, handleId, 2, phase)
   call TimerStart(t, 0.01, false, function onloop)
   set Gryphon = null
   set tr = null
endfunction
endscope

IcemanBo · Mar 31, 2019

Due to PMs, is an update planned?

Rigborn · Mar 31, 2019

Updated

JASS:

scope Wanderer initializer init

   globals
       public hashtable h
   endglobals

   function onloop takes nothing returns nothing
  
       local timer t = GetExpiredTimer()
       local integer handleId = GetHandleId(t)
       local unit Gryphon = LoadUnitHandle(h, handleId, 1)
       local integer phase = LoadInteger(h, handleId, 2)
       local integer orderMove = 851986
       local real x
       local real x2
       local real y
       local real y2
       local real dist
       local real ang
       local integer terrainType
       local integer terrainVariation
  
       if phase == 1 then
           set dist = GetRandomReal(500, 1500)
           set ang = GetRandomReal(0, 360)*bj_DEGTORAD
           set x2 = GetUnitX(Gryphon)+dist*Cos(ang)
           set y2 = GetUnitY(Gryphon)+dist*Sin(ang)
           if (GetRectMinX(bj_mapInitialPlayableArea) <= x2) and (x2 <= GetRectMaxX(bj_mapInitialPlayableArea)) and (GetRectMinY(bj_mapInitialPlayableArea) <= y2) and (y2 <= GetRectMaxY(bj_mapInitialPlayableArea)) then
               set terrainType = GetTerrainType(x2, y2)
               set terrainVariation = GetRandomInt(0, 2)
               call SaveReal(h, handleId, 3, x2)
               call SaveReal(h, handleId, 4, y2)
               call SaveInteger(h, handleId, 5, terrainType)
               call SetTerrainType(x2, y2, 'Nsnw', terrainVariation, 1, 0)
               call IssuePointOrderById(Gryphon, orderMove, x2, y2)
               set phase = 2
               call SaveInteger(h, handleId, 2, phase)
               call TimerStart(t, 0.30, true, function onloop)
               set Gryphon = null
               set t = null
           else
               call IssueImmediateOrder(Gryphon, "stop")
               call PauseTimer(t)
               call DestroyTimer(t)
               set t = null
               set Gryphon = null
           endif
       elseif phase == 2 then
           set x = GetUnitX(Gryphon)
           set y = GetUnitY(Gryphon)
           set x2 = LoadReal(h, handleId, 3)
           set y2 = LoadReal(h, handleId, 4)
           set terrainType = LoadInteger(h, handleId, 5)
           set dist = SquareRoot(((x2-x)*(x2-x))+((y2-y)*(y2-y)))
           if dist < 20.00 then
               call SetTerrainType(x2, y2, terrainType, 1, 1, 0)
               set phase = 1
               set Gryphon = null
               set t = null
               call SaveInteger(h, handleId, 2, phase)
           endif
       endif
   endfunction
  
   function init takes nothing returns nothing
  
       local unit Gryphon
       local trigger tr
       local integer orderMove = 851986 // move order id
       local timer t = CreateTimer()
       local integer handleId = GetHandleId(t)
       local integer phase = 1
  
       set tr = CreateTrigger()
       set h = InitHashtable()
       set Gryphon = CreateUnit(Player(0), 'hgry', 0.00, 0.00, 0.00)
       call SaveUnitHandle(h, handleId, 1, Gryphon)
       call SaveInteger(h, handleId, 2, phase)
       call TimerStart(t, 0.01, false, function onloop)
       set Gryphon = null
       set tr = null
   endfunction
  
endscope

IcemanBo · Apr 1, 2019

For me the mission is solved.

Some points just to to note that were already pointed out in PM, and some new ones:

Variabletr is declared, but never used.
terrainVariation is stored but never used. Or it should be used, or removed.
Instead of stopping the timer, a re-calculation could happen.
There's a case where the Gryphon variable won't be nulled in the loop function. To avoid such cases, and redundancy such related actions should happen on same nesting-level.
Example-good:
JASS:
```
local unit u = GetTriggerUnit()

if ( ... ) then
    ...
   
elseif ( ... ) then
    ...
else
    ...
endif
set u = null
```
Exmple-bad:
JASS:
```
local unit u = GetTriggerUnit()

if ( ... ) then
    ...
    set u = null
elseif ( ... ) then
    ...
    set u = null
else
    ...
    set u = null
endif
```
.. so or always null on same nesting-level, or just always in end, if it's easier to read.
call TimerStart(t, 0.01, false, function onloop)
There's no really need not to use "0.00". For example, I personally asked myself if amount must be larger than "0.00", but it's not the case.
I would personally find it fitting to introduce a new function, to encapsulate some functionality, like maybe a Move-function, which's purpose is to take unit is parameter, and which should handle everything for a new successful run.

Rigborn · Apr 3, 2019

Updated.

JASS:

scope Wanderer initializer init

   globals
       private hashtable h = InitHashtable()
   endglobals
   
   private function move takes unit u, real x, real y returns nothing
       local integer orderMove = 851986
       call IssuePointOrderById(u, orderMove, x, y)
   endfunction
   
   private function onloop takes nothing returns nothing
       local timer t = GetExpiredTimer()
        local integer handleId = GetHandleId(t)
        local unit Gryphon = LoadUnitHandle(h, handleId, 1)
        local integer phase = LoadInteger(h, handleId, 2)
        local integer orderMove = 851986
        local real x
        local real x2
        local real y
        local real y2
        local real dist
        local real ang
        local integer terrainType
     
       if phase == 1 then
           set dist = GetRandomReal(500, 1500)
            set ang = GetRandomReal(0, 360)*bj_DEGTORAD
            set x2 = GetUnitX(Gryphon)+dist*Cos(ang)
            set y2 = GetUnitY(Gryphon)+dist*Sin(ang)
            if (GetRectMinX(bj_mapInitialPlayableArea) <= x2) and (x2 <= GetRectMaxX(bj_mapInitialPlayableArea)) and (GetRectMinY(bj_mapInitialPlayableArea) <= y2) and (y2 <= GetRectMaxY(bj_mapInitialPlayableArea)) then
                set terrainType = GetTerrainType(x2, y2)
                call SaveReal(h, handleId, 3, x2)
                call SaveReal(h, handleId, 4, y2)
                call SaveInteger(h, handleId, 5, terrainType)
                call SetTerrainType(x2, y2, 'Nsnw', GetRandomInt(0, 2), 1, 0)
                call IssuePointOrderById(Gryphon, orderMove, x2, y2)
                set phase = 2
                call SaveInteger(h, handleId, 2, phase)
            else
                call IssueImmediateOrder(Gryphon, "stop")
            endif
        elseif phase == 2 then
            set x = GetUnitX(Gryphon)
            set y = GetUnitY(Gryphon)
            set x2 = LoadReal(h, handleId, 3)
            set y2 = LoadReal(h, handleId, 4)
            set terrainType = LoadInteger(h, handleId, 5)
            set dist = SquareRoot(((x2-x)*(x2-x))+((y2-y)*(y2-y)))
            if dist < 20.00 then
                call SetTerrainType(x2, y2, terrainType, 1, 1, 0)
                set phase = 1
                call SaveInteger(h, handleId, 2, phase)
            endif
        endif
       set Gryphon = null
        set t = null
    endfunction
   private function init takes nothing returns nothing
        local unit Gryphon
        local integer orderMove = 851986
        local timer t = CreateTimer()
        local integer handleId = GetHandleId(t)
        local integer phase = 1
        set Gryphon = CreateUnit(Player(0), 'hgry', 0.00, 0.00, 0.00)
        call SaveUnitHandle(h, handleId, 1, Gryphon)
        call SaveInteger(h, handleId, 2, phase)
        call TimerStart(t, 0.30, true, function onloop)
       set Gryphon = null
   endfunction
endscope

I would personally find it fitting to introduce a new function, to encapsulate some functionality, like maybe a Move-function, which's purpose is to take unit is parameter, and which should handle everything for a new successful run.

I didn't quite get that. Are you suggesting that I should use a seperate move function to move the unit ? If so, is it possible to get the handle id of the timer in that function

?

IcemanBo · Apr 3, 2019

Maybe it was a bit too critical for jass class, it's actually fine. But what I meant I would personally prefer is something like splitting duty of the function. Yes with "move", but not the move-order alone, but the whole logics binded to it, too. ( checking for valid point, storing terrain type at move-order etc)

Like:

JASS:

private function orderNewMove takes integer parentKey returns nothing
    // loop until a valid point to move was found
    // move unit, change terrain, saved terrain to parentKey ..
endfunction

private function distinationReached takes integer parentKey returns nothing
    // this happens when a unit reached the targeted terrain point
    ...
endfunction

private function onLoop takes nothing returns nothing
    local integer parentKey = GetHandleId(GetExpiredTimer())

    if phase == 1 then
        call orderNewMove(parentKey)

    elseif phase == 2 then
        call distinationReached(parentKey)

    endif
endfunction

With giving the parentKey as parameter, the functions have now full access to all data, because they have the right key in hashtable.

So the different logics can be seperated. It may be useful once there are bigger functions.

The loop just handles the job-distribution, which functionality should happen next, at which state.
the move-functionality logics is encapsulated.
the destination-reached functionality is encapsulated.
etc, to have easier overview and maintenance.

Rigborn · Apr 3, 2019

Actually that makes more sense. I have never thought of it that way. Mine waits for another 0.30 seconds to find a new point and if that point is invalid, another 0.30.

Wanderer - Submission

Rigborn

Rigborn

Resources

Attachments

IcemanBo

IcemanBo

Resources

Rigborn

Rigborn

Resources

Attachments

IcemanBo

IcemanBo

Resources

Rigborn

Rigborn

Resources

IcemanBo

IcemanBo

Resources

Rigborn

Rigborn

Resources

Attachments

IcemanBo

IcemanBo

Resources

Rigborn

Rigborn

Resources

IcemanBo

IcemanBo

Resources

Rigborn

Rigborn

Resources

Similar threads