RPG Threat System v3.2c

[Moderator]

IcemanBo:
- date: 3th March 2015
- submission: Special Creep Engine v3.2b

The system allows to define unique unit-type behaviours
for creeps to improve the enemy's AI creep controle.

All in all the code is efficient, leakless and works fine.
I have not found nor bugs nor major flaws in the system.

• 2*r <= 360 -> r <= 180.
• It might be a bit confusing for user that the creep sleeping is not the same as the normal wc3 unit sleeping.
  It might be helpful if it was clarified.
• Arithmetic like "SECONDS_IN_A_DAY/86400" could be calculated once onInit.
• The term "generator unit" that is used is not really explained. (?)

Good job. Approved.

Older

IcemanBo: http://www.hiveworkshop.com/forums/...p-engine-v3-2b-248858/index5.html#post2640438

General

Coding

Concept

Design

Misc

Docs

Required Changes

Purgeandfire :: 29 October 2014

Criteria (Systems):

• Coding (20): efficiency and potential improvement.
• Concept (10): originality and features.
• Design (20): modularity, importability, ease of use, and method of attack.
• Misc (10): test map, image, hive submission, misc.
• Docs (10): documentation

Score:

• Coding: 10 / 20
• Concept: 7 / 10
• Design: 14 / 20
• Misc: 8 / 10
• Docs: 5 / 10
• Total Score: 44 / 70

Needs Fix :: see Required Changes for approval.

3 / 5

Score Distribution

• 65 - 70 becomes 5 / 5
• 55 - 65 becomes 4 / 5
• 40 - 55 becomes 3 / 5
• 30 - 40 becomes 2 / 5
• 0 - 30 becmes 1 / 5

Past Reviews

1. http://www.hiveworkshop.com/forums/2589556-post54.html

GetWorldBounds() leaks. You need to store it in a variable and then remove it manually (2 occurrences). [-3] Just a note, adding [13] does not do anything for the following line: private trigger array Handler[13] It does not change anything unless you use a size > 8192. filter2 should have a better name. [-1] In pickHelper, if I'm not mistaken, it seems like you are trying to choose the closest units and order them to help. In this case, it may be better to take advantage of Spinnaker's system and use: GetClosestNUnitsInRange It is designed to do what you need to do, and it will result in a significantly lighter computation. Note: You will have to change your entire loop to adapt to this change. [-2] This: set Real[R_TEMP_FACE] = GetRandomReal(0, 359.9) * bj_DEGTORAD Is equivalent to: sett Real[R_TEMP_FACE] = GetRandomReal(0, 2*bj_PI) Or you can use 6.28319 instead of 2*bj_PI. In onHit, you can move the assignment of 'dex' outside the blocks. (saves 1 line) In onDeath, you need to set u to null. [-1] If you really wanted to be extreme, you could redo this entire system using structs instead of hashtables. > [-2] Why use a trigger action for initUnit? (use a condition) [-1] Score: 10 / 20

1. Cool concept! I love the diverse features! It really shows effort.
2. Creep engines are quite common though.
3. It could be cool to have creep-specific modularity, e.g. creep
   specific sleep durations, so forth.

Score: 8 / 10

The module design is not very good. Generally, the best design is where a user does not have to touch the system at all, and only needs to know its functions. Generally, it is better to separate example code from the library itself (usually in a different trigger). A second design flaw is that the data is all expected upon initialization. The setup functions have too generic names, e.g. ResetTimer or LowBound To fix issues 1-3, I recommend translating your setup over to a struct and adding a .register() method so the user can invoke registering a creep. struct SpecialCreep extends array private static group cg = CreateGroup() private static integer uid = 0 method operator tamed= takes boolean tame returns nothing call SaveBoolean(Hashtable, P_TAMED_BOOL, this, tame) endmethod method operator sleepDuration= takes real duration returns nothing call SaveReal(Hashtable, P_SLEEP_DUR, this, duration) endmethod // etc... private static method registerCreeps takes nothing returns boolean local unit f = GetFilterUnit() if GetUnitTypeId(f) == thistype.uid then // ... actions to register creeps ... // uid -> rawcode of creep being registered // f -> creep being registered endif set f = null return false endmethod method update takes nothing returns nothing local rect r = GetWorldBounds() set uid = this call GroupEnumUnitsInRect(cg, r, Filter(function thistype.registerCreeps)) call RemoveRect(r) set r = null endmethod static method get takes integer raw returns thistype return raw endmethod endstruct This may be a little advanced. In this situation, we are treating the raw ID as the struct itself. With this, we do not even need to map the rawcodes to integers 1, 2, 3... etc. We can just use the rawcode for everything! In case this was confusing, maybe this will help: local SpecialCreep murloc = SpecialCreep.get('nmrl') set murloc.tamed = false set murloc.sleepDuration = 7.25 set murloc.sleepTime = 21.0 // etc. call murloc.update() Note: my code above would change a lot of the system. I recommend making these changes only if you understand the code 100%. But the advantage is it would allow you to update unit stats at any time by using .update()! That is awesome modularity. When you order a unit to patrol (via the MOVE order), won't it ignore enemy targets? Score: 14 / 20

1. The test map should have instructions on what to do, or some
   form of debriefing. It can either be in a message, a quest, etc. [-2]

Score: 8 / 10

Needs documentation. It is preferrable to have it at the top of the code, and have a list of the functions that should be used. Ideally, a user should not have to look through your system at all in order to use your system. Add some descriptions to functions and such. See the JASS section for examples. It will make your code a lot more user-friendly. [-5] Score: 5 / 10

• Coding: #1, #4 (or explain if I read your code wrong), #7, #9
• Design: #5 (need a response)
• Misc: #1

 

[Kazeon]

forgot to mention
this system is not friendly for GUI user because you need a little basic JASS knowledge about filtering unit.

 

[Daffa]

forgot to mention
this system is not friendly for GUI user because you need a little basic JASS knowledge about filtering unit.

Found some bugs:
- dead units still ordered to attack
- unit is attacked event is buggy, I recommend everybody to change the event with any DDS. I can do nothing about this.

Bug Patching Help :
- check if unit is alive, if it's not, removed it from list.
- I recommend create several versions that suits all renown DDS (LFH and Bribe GUI DDS' are top priority since they're used a lot).

 

[Kazeon]

Bug Patching Help :
- check if unit is alive, if it's not, removed it from list.
- I recommend create several versions that suits all renown DDS (LFH and Bribe GUI DDS' are top priority since they're used a lot).

I will just use lfh's.. bcose it's super lovely and easy to use!

 

[Daffa]

It's recommended that CBS_Loop Value shouldn't go below integer 0.3125 or so, that's the base value of Jass spells loop.

 

[TheGoldenGoblin]

You should give examples on how to use dds. Also if you use spells on the units they won't attack you.

 

[BPower]

The big con is the tremendous ForGroup loop, which runs every 0.03125 seconds.

I would appreciate it, if you would outline your code with comments.
Makes it much easier for me and others to read.

 

[Kazeon]

You should give examples on how to use dds. Also if you use spells on the units they won't attack you.

you are right.. but there are many choices of DDS, that's why I got a feeling to left it optional (using basic unit is attacked event)

The big con is the tremendous ForGroup loop, which runs every 0.03125 seconds.

I would appreciate it, if you would outline your code with comments.
Makes it much easier for me and others to read.

yeah, actualy using high accuracy doesn't really affects the performance lol, event using 0.5 is not noticeable..

300th post!!!

EDIT:

Updated to v1.2 with the following:
- Accuracy changed to a reasonable value
- Added some outlines
- Added with DDS version

credit's going to looking_for_help

 

[TriggerHappy]

The documentation should be included in the code, so when people copy it to their maps or to another website, it's there.

Also, inside CBS_InitLibrary I think there should be a wrapper forSaveIntegerto improve readability and not require people to specify a parent key (custom allocation).
This will also prevent users from using a different hashtable.

By default this system should probably require a DDS, because of how inaccurate the attack event is.

 

[Malhorne]

function DistanceBetweenCoords takes real x,real y,real xt,real yt returns real
        local real dx = xt - x
        local real dy = yt - y
        
        return SquareRoot(dx * dx + dy * dy)
    endfunction

->

function DistanceBetweenCoords takes real x,real y,real xt,real yt returns real
        return SquareRoot((xt-x)*(xt-x) + (yt-y)*(yt-y))
    endfunction

if angle > 360 then
            loop
                set angle = angle - 360
                exitwhen (angle <= 360)
            endloop
        elseif angle < 0 then
            loop
                set angle = angle + 360
                exitwhen angle >= 0
            endloop
        endif

->

if angle > 360. then
            set angle = ModuloReal(angle, 360)
        elseif angle < 0. then
            set angle = ModuloReal(-angle, 360)
        endif

function CBS_Filter2 takes nothing returns boolean
        local unit u = GetFilterUnit()
        local unit c = LoadUnitHandle(udg_CBS_Hash, 0, 0)
        local real a1
        local real a2
        local real rad
        local integer dex
        local player pa = Player(CBS_PassivePlayer())
        local player ag = Player(CBS_AggressivePlayer())
        local player p = GetOwningPlayer(u)
        
        set dex = LoadInteger(udg_CBS_Hash, 1911, GetUnitTypeId(c))
        set a1 = GetUnitFacing(c) + 360.0
        set a2 = AngleBetweenCoords(GetUnitX(c),GetUnitY(c),GetUnitX(u),GetUnitY(u)) + 360.0
        // Load sight radius
        set rad = LoadReal(udg_CBS_Hash, 5511, dex) * 0.5
        // Filter out target not in sight radius
        if (a2 <= a1 - rad or a2 >= a1 + rad) and (a2 <= a1 - rad - 360.0 or a2 >= a1 + rad - 360.0) then
            return false
        endif
        if p == pa or p == ag then
            return false
        endif
        return IsUnitVisible(u, pa) and IsUnitVisible(u, ag) and not IsUnitType(u, UNIT_TYPE_DEAD)
    endfunction

This function leak ^^'
I suggest storing the result into a boolean, null every local handle and at least return the boolean.

function CBS_HelpFilter takes nothing returns boolean
        local unit u = GetFilterUnit()
        local integer ut = GetUnitTypeId(u)
        local integer dex = LoadInteger(udg_CBS_Hash, 1911, ut)
        local integer ht = LoadInteger(udg_CBS_Hash, 7550, dex)
        
        if ht == 1 then
            if ut != udg_CBS_DuTy then
                return false
            endif
        else
            if ht < 1 then
                return false
            endif
        endif
        return GetOwningPlayer(u) == Player(CBS_PassivePlayer()) and not LoadBoolean(udg_CBS_Hash, 2343, dex) 
    endfunction

Same there.

if GetWidgetLife(u) > .405 then isn't enough to check if a unit is dead :
if GetWidgetLife(u) > .405 and not IsUnitType(u, UNIT_TYPE_DEAD)

local player pa = Player(CBS_PassivePlayer())
        
        if GetOwningPlayer(a) == Player(CBS_AggressivePlayer()) then

->

local player pa = Player(CBS_PassivePlayer())
        
        if GetOwningPlayer(a) == pa then

else
                if not CBS_ColorBool() then
                    call SetUnitColor(u, GetPlayerColor(ag))
                endif

->

elseif not CBS_ColorBool() then
    ....
endif

call SetPlayerAllianceStateBJ(ag, p, bj_ALLIANCE_UNALLIED)
                // Passive player threats other players as allies
                call SetPlayerAllianceStateBJ(pa, p, bj_ALLIANCE_ALLIED)

Bjs needed ?


call TriggerRegisterEnterRectSimple(t, GetWorldBounds())
This one can be avoid and it leaks :/

 

[IcemanBo]

private function AngleBetweenCoords takes real x, real y, real xt, real yt returns real
        local real angle = Atan2((yt - y), (xt - x)) * bj_RADTODEG
       
        if angle > 360 then
            loop
                set angle = angle - 360
                exitwhen (angle <= 360)
            endloop
        elseif angle < 0 then
            loop
                set angle = angle + 360
                exitwhen angle >= 0
            endloop
        endif
        return angle
    endfunction

This is ok, but I would just do:

private function AngleBetweenCoords takes real x, real y, real xt, real yt returns real
    return Atan2((yt - y), (xt - x)) * bj_RADTODEG
endfunction

If you keep yours (not wrong) change the first check "if angle > 360 then" to if equal or bigger. (because if you have 360 you want to get it 0)

private function filter2 takes nothing returns boolean
        local unit u = GetFilterUnit()
        local unit c = LoadUnitHandle(HASH, 0, 0)
        local real a1
        local real a2
        local real rad
        local integer dex
        local player pa = Player(PASSIVE)
        local player ag = Player(AGGRESSIVE)
        local player p = GetOwningPlayer(u)
        local boolean b = true
       
        set dex = LoadInteger(HASH, 1911, GetUnitTypeId(c))
        set a1 = GetUnitFacing(c) + 360.0
        set a2 = AngleBetweenCoords(GetUnitX(c),GetUnitY(c),GetUnitX(u),GetUnitY(u)) + 360.0
        set rad = LoadReal(HASH, 5511, dex) * 0.5
        if (a2 <= a1 - rad or a2 >= a1 + rad) and (a2 <= a1 - rad - 360.0 or a2 >= a1 + rad - 360.0) then
            set b = false
        endif
        if (p == pa or p == ag) or IsUnitType(u, UNIT_TYPE_DEAD) then
            set b = false
        endif
        if not IsUnitVisible(u, pa) and not IsUnitVisible(u, ag) then
            set b = false
        endif
        set u = null
        set c = null
        return b
    endfunction

Use locols for UnitX/Y since you make 2 function calls.
Atm you do 3 checks, even b would be FALSE already after first one. You could put further checks in 'else' of first 'if' to prevent useless checks.

Same with move further 'if's' in 'else' of first 'if' to prevent needless checks for

private function helpfilter

Then soon stopped reading code carefully because Im too lazy to read this all :d

But in "private function ini takes nothing returns nothing" you forgot to null local region.

And does your system work if I want to have more than 1 AGGRESIVE and 1 PASSIVE player?

And for me idc, but you have to change variable names. Only capital letters --> only constants. Start normal variables with lowcase and each new word with capital letter.

 

[Kazeon]

And does your system work if I want to have more than 1 AGGRESIVE and 1 PASSIVE player?

you want it for what? C:

Use locols for UnitX/Y since you make 2 function calls.

I use it only once

If you keep yours (not wrong) change the first check "if angle > 360 then" to if equal or bigger. (because if you have 360 you want to get it 0)
But in "private function ini takes nothing returns nothing" you forgot to null local region.

okay, thnks!

And for me idc, but you have to change variable names. Only capital letters --> only constants. Start normal variables with lowcase and each new word with capital letter.

can you give me example? I don't understand that part
thnks for the review..

 

[IcemanBo]

you want it for what? C:

I dont need it, just wanted to know

I use it only once

Oh yo, one time 'c' and other one is 'u'. You're right

can you give me example? I don't understand that part
thnks for the review..

private constant string NAME : good
private constant string name : not good

private string MAPNAME : not good
private string mapName : good

 

[IcemanBo]

In my system BPower told me not to use _
So if it's constant then maybe NAMENAME else nameName.
http://www.hiveworkshop.com/forums/2494473-post3.html

Edit:

CONSTANT_NAMES are all capital letters and each new word
is indicated by being seperated from the rest by an underscore.

MAP_NAME was correct for constant ^^

 

[Malhorne]

Remember that if you use only one time the value in the function there is no need to use a local I didn't check the code on this criteria but seemed good.

set dex = LoadInteger(HASH, 1911, GetUnitTypeId(c))
        set a1 = GetUnitFacing(c) + 360.0
        set a2 = AngleBetweenCoords(GetUnitX(c),GetUnitY(c),GetUnitX(u),GetUnitY(u)) + 360.0
        set rad = LoadReal(HASH, 5511, dex) * 0.5

Could be done on local declaration

local unit u = gettarget()
        local unit a = getsource()

Set the values of those local inside the if block and put the nulling of those local in the if block too

local player pa = Player(PASSIVE)
        
        if GetOwningPlayer(a) == Player(AGGRESSIVE) then

->

local player pa = Player(PASSIVE)
        
        if GetOwningPlayer(a) == pa then

You forgot to use call RemoveRect(r) in the init function

if GetUnitCurrentOrder(u) != OrderId("move") and GetUnitX(u) == LoadReal(HASH, hand, 9) and GetUnitY(u) == LoadReal(HASH, hand, 10) then
            return false
        endif
        return true

->
return not(GetUnitCurrentOrder(u) != OrderId("move") and GetUnitX(u) == LoadReal(HASH, hand, 9) and GetUnitY(u) == LoadReal(HASH, hand, 10))

 

[Kazeon]

Roger that!

local player pa = Player(PASSIVE)

if GetOwningPlayer(a) == Player(AGGRESSIVE) then

there is PASSIVE and AGGRESIVE, they are not the same

        local unit u = gettarget()
        local unit a = getsource()

I think that part just stay at local declaration is okay

 

[Malhorne]

Holy xD !
My eagle eye didn't see this ^^

 

[Kazeon]

call RemoveRect(r) result in complie error? :/ or maybe you meant call RemoveRegion(r) ? because r is region

 

[Malhorne]

Oh yes you're right ^^

 

[Xonok]

I recommend not using ForGroup at all. Use a FirstOfGroup loop or put units into an indexed array.
This is because ForGroup gets crazily expensive with lots of units. Tests show that it's about on par with FirstOfGroup with around 3-4 units. After that the performance has dropped drastically.

 

[Malhorne]

Yep you're right...
If you don't mind destroying the group I suggest using a FoG loop

 

[Kazeon]

I dont understand what are you gays talking about x.X

what's FoG? I never use it before

 

[Malhorne]

local unit u
loop
    set u = FirstOfGroup(g, u)
    exitwhen u == null
    call GroupRemoveUnit(g, u)
    //Do actions with the unit
endloop

 

[Xonok]

I dont understand what are you gays talking about x.X

what's FoG? I never use it before

Basically FirstOfGroup (FoG) is just a function. It gives you 1 unit from a unit group.
If you wish to use groups, then do it like this:

function stuff takes group g returns nothing
    local group backup = CreateGroup()
    local unit u
    loop
        set u = FirstOfGroup(g)
        exitwhen u == null
        call GroupAddUnit(backup,u)
        //Do something with unit
        call GroupRemoveUnit(g,u)
    endloop
    call DestroyGroup(g)
    set g = backup
    set backup = null
    set u = null
endfunction

This piece of code assumes that you want to keep the group intact.

 

[Kazeon]

will that improve the performance? seems like it's just the same if we have many units :/

 

[Xonok]

will that improve the performance? seems like it's just the same if we have many units :/

It will. For some reason the ForGroup function spends excessive amounts of processing power with lots of units.
Due to this, a FoG loop is much faster. It's stupid, but that's how it is.

 

[Kazeon]

thanks, I will change it then
wait, are you sure this works
set g = backup
?

EDIT:
it works!
but strange, when I destroy the backup it stops working
call DestroyGroup(bu)
if I dont destroy it, will that causes leak?

Updated to v1.9
- Added 2 new APIs
- Improved loops
- Fixed another sleep bug
- Many improvements (credit's going to Malhorne and xonok)

 

[Xonok]

thanks, I will change it then
wait, are you sure this works
set g = backup
?

EDIT:
it works!
but strange, when I destroy the backup it stops working
call DestroyGroup(bu)
if I dont destroy it, will that causes leak?

Updated to v1.9
- Added 2 new APIs
- Improved loops
- Fixed another sleep bug
- Many improvements (credit's going to Malhorne and xonok)

Well, you have to keep in mind that you're not destroying a variable, but an object.
A unit group is an object, so if you do something like:
local group a = CreateGroup
local group b = a
local group c = b

Then you have 1 group and 3 variables.
Thus, to clear them you have to destroy the group once (through any of them, but only 1) and null all 3 of them.

 

[Malhorne]

It is a reference to the object to be precise.

 

[Malhorne]

You're totally welcome !

EDIT :

looking_for_help for GUI damage detection (actually it's Jass, he was drunk)

<3-ed this

 

[Adiktuz]

It seems to look useful... Though sadly I think most systems like this gets not much exposure from the users it was actually intended for...

Hidden

Heck, my super now useless, inferior, maybe not so optimized missile system has more than 2x downloads than my really useful systems...

and maybe because less and less people mod nowadays...

 

[Kazeon]

that's all I want to know, is this system actually useful or not? :3 thnks adikutz!

 

[Adiktuz]

It is useful...

As for usability, IDK... haven't look closely at the code so...

For example, you can make a really useful script, but then it's not easy for people to implement, so while it is useful, it's not that much usable

 

[Kazeon]

it's not that hard to implement Imo
just copy all the codes and give 2 empty player slot, then configure everything (yes, this part need little more understanding actually )

 

[Adiktuz]

It was just an example of usable vs useful... Never said your system was hard to implement

 

[Kazeon]

It is useful...

As for usability, IDK... haven't look closely at the code so...

For example, you can make a really useful script, but then it's not easy for people to implement, so while it is useful, it's not that much usable

there you said it..

 

[Adiktuz]

If you read the first part of the greened text, it says, FOR EXAMPLE... IDK what part of the word EXAMPLE you don't understand...

and the next one too, YOU CAN

So you see, I said EXAMPLE and CAN... I never said YOU DID

and in case you missed my second line: "As for usability, IDK... haven't look closely at the code so..."
Logically, how can I say that your system is hard to implement when I haven't looked closely at it?

 

[Malhorne]

Please guys xD !

 

[Adiktuz]

but really, maybe the mods are just busy...

 

[A Void]

Shame that this is not in GUI. I am pro-GUI. This would be very useful in GUI maps. Since most maps out there are GUI.

 

[Malhorne]

I don't see the point of doing maps without JNGP....
Please guys stop this narrow minded way ><

 

[A Void]

I am using Jass New Gen Pack. But I'm not into vJass nor Jass.

 

[Malhorne]

And you say you're pro-GUI ?
You can't without JASS knowledge...

And btw vJASS is easier to configure than GUI :/

 

[A Void]

I don't like writing codes. I like doing things with interface.

 

[Malhorne]

Oh that's sure that when you code nowadays interface is like : https://www.youtube.com/watch?v=nUreKfeBGWk
The title was supposed to be a part of the sentence but oh anyway, arguing is close

 

[msongyboi]

sir this is excellent, maybe you can add some spells ai funcs.

spells
targetable (disable, nukes, ally buff, aoe disable, aoe buff, aoe disable, aoe damage)
immediate (self buff, aoe damage, aoe buff, aoe disable)
point (etc..)

like
function addTargetableSpell takes integer unitID, integer abilityID, integer castType, real time, integer chance returns nothing

let the time proc and when it hits 0, there's a chance for a unit to cast a spell with different controls per castType or something like

function addRetaliationSpell takes blah blah balh

every time the unit is damaged, there's a chance to proc this spell blah blah blah.

once again lovely script +rep

 

[Malhorne]

Infact it is the definition of AI XD

 

[A Void]

So any plans on this engine to actually be in GUI? I suggest on doing that as I mentioned. More people will then use this.

 

[Kazeon]

there is no point in recoding it to GUI, unless user wish to modify the core, which is impossible in the most cases, mostly user just want to use not modify.. so I refuse
and it's currently easy for GUI ers just to use it..