Feedback on AoS Spawn system

luorax

Invasion in Duskwood
Reaction score
67
Well, we've got tons of these already, but most of them is outdated, or overcomplicated. I tried to mix simplicity and efficiency. I got this:

JASS:
library SpawnSystem uses TimerUtils,Alloc, optional AIDS, optional UnitIndexer

/*************************************************************************************
*
*            Spawn System
*                by Luorax
*
*            DESCRIPTION
*                >> AoS Spawn System is an basic and simple system that eases the creation
*                   of spawns in AoS or any other genre. You only have to create one AoSSpawn struct per
*                   Spawn and add your waypoints. After that call .finish() to make the whole stuff work.
*
*            FEATURES
*                 > Easy to setup alot of spawns which would create a bunch of triggers normally
*                 > You can have nearly an unlimited number of spawns
*                 > Simple struct syntax should be easy to use
*
*            FUNCTIONS LIST
*                >> struct Spawn
*                    .create takes integer rawCode, integer count,player owner,real interval,string effectPath returns thistype
*                    .addWaypoint takes rect whichRect returns nothing
*                    .removeWayoint takes rect whichRect returns nothing
*                    .method addSpawnFunction takes onSpawn spw returns nothing
*                    .method addCheckFunction takes onCheck spw returns nothing
*                    .method addUnitSpawnFunction takes onUnitSpawn spw returns nothing
*            
*            CREDITS
*                > YourName for the original system and this library header
*                > Magtheridon96 for TimerUtils
*                > Sevion for Alloc
*                > Vexorian for JASSHelper and original TimerUtils
*            
*            MISC
*
*                 >> Function Interfaces
*                    > function interface onSpawn takes nothing returns nothing
*                    > function interface onCheck takes nothing returns boolean
*                    > function interface onUnitSpawn takes unit u returns nothing
*************************************************************************************/

    //: =================================================================
    //:                        Config
    //: =================================================================
    globals
        // The basic interval which is used when you set -1 as interval in the create function.
        private constant real DEFAULT_INTERVAL=10
    endglobals
    private function defaultSpawnCheck takes nothing returns boolean
        return true
    endfunction
                
    //: =================================================================
    //:                       System Code
    //: =================================================================

    function interface onSpawn takes nothing returns nothing
    function interface onCheck takes nothing returns boolean
    function interface onUnitSpawn takes unit u returns nothing
    
    globals
        private integer array WayPointData
    endglobals
    
    private struct RectList extends array
        implement Alloc
        
        readonly rect r
        readonly real x
        readonly real y
        readonly boolean isList
        readonly thistype next
        readonly thistype prev
        readonly thistype list
        readonly integer size
        static method create takes nothing returns thistype
            local thistype this=thistype.allocate()
            set this.isList=true
            set this.list=this
            set this.prev=0
            set this.next=0
            set this.size=0
            return this
        endmethod
        method remove takes nothing returns thistype
            local thistype next=this.next
            set this.isList=false
            call this.deallocate()
            return next
        endmethod
        method destroy takes nothing returns nothing
            loop
                set this=this.remove()
                exitwhen this==0
            endloop
        endmethod
        method link takes rect r returns nothing
            local thistype next=thistype.allocate()
            set next.r=r
            set next.x=GetRectCenterX(next.r)
            set next.y=GetRectCenterY(next.r)
            set next.list=this
            
            set this.prev.next=next
            set next.prev=this.prev
            set this.prev=next
            set this.size=this.size+1
            
            if this.next==0 then
                set this.next=next
            endif
        endmethod
        method detachTail takes nothing returns nothing
            local thistype tail=this.prev
            
            set this.prev=this.prev.prev
            set this.prev.next=0
            set this.size=this.size-1
            
            set tail.next=0
            set tail.prev=0
            set tail.list=0
            
            if this.next==tail then
                set this.next=0
            endif
        endmethod
        method detachHead takes nothing returns nothing
            local thistype head=this.next
            
            set this.next=this.next.next
            set this.next.prev=0
            set this.size=this.size-1
            
            set head.next=0
            set head.prev=0
            set head.list=0
            
            if this.prev==head then
                set this.prev=0
            endif
        endmethod
        private method detach takes nothing returns nothing
            if this.list.next==this then
                call this.list.detachHead()
            elseif this.list.prev==this then
                call this.list.detachTail()
            else
                set this.prev.next=this.next
                set this.next.prev=this.prev
                set this.list.size=this.list.size-1
                
                set this.next=0
                set this.prev=0
                set this.list=0
            endif
        endmethod
        method unlink takes rect r returns nothing
            loop
                set this=this.next
                if this.r==r then
                    call this.detach()
                endif
                exitwhen this==0
            endloop
        endmethod
        
        method operator head takes nothing returns thistype
            return this.next
        endmethod
        method operator hasNext takes nothing returns boolean
            return this.next!=0
        endmethod
        method operator tail takes nothing returns thistype
            return this.prev
        endmethod
        method operator hasPrev takes nothing returns boolean
            return this.prev!=0
        endmethod
        method operator empty takes nothing returns boolean
            return this.size==0
        endmethod
    endstruct
    struct Spawn extends array
        implement Alloc
        private player owner
        private integer rawCode
        private integer count
        private timer timer
        
        private RectList wayPoints
        private trigger onEnter
        
        private string effectPath
        private onSpawn onSpawn
        private onCheck spawnCheck
        private onUnitSpawn onUnitSpawn
        
        //---== Privates ==---
        private static method periodic takes nothing returns nothing
            local thistype this=GetTimerData(GetExpiredTimer())
            local integer i=0
            local unit u
            
            if this.spawnCheck.evaluate() then
                call this.onSpawn.execute()
                loop
                    exitwhen i>=this.count
                    set u=CreateUnit(this.owner,this.rawCode,this.wayPoints.head.x,this.wayPoints.head.y,0)
                    if this.effectPath!="" then
                        call DestroyEffect(AddSpecialEffectTarget(this.effectPath,u,"origin"))
                    endif
                    call this.onUnitSpawn.execute(u)
                    call IssuePointOrder(u,"attack",this.wayPoints.head.next.x,this.wayPoints.head.next.y)
                    
                    set WayPointData[GetUnitUserData(u)]=this.wayPoints.head.next
                    set i=i+1
                endloop
            endif

            set u=null
        endmethod
        private static method enter takes nothing returns boolean
            local RectList list=RectList(WayPointData[GetUnitUserData(GetTriggerUnit())])
            
            if list.hasNext then
                set list=list.next
                call IssuePointOrder(GetTriggerUnit(),"attack",list.x,list.y)
                set WayPointData[GetUnitUserData(GetTriggerUnit())]=list
            endif
            return false
        endmethod
        //---== SetUps ==---
        method addSpawnFunction takes onSpawn spw returns nothing
            set this.onSpawn=spw
        endmethod
        method addCheckFunction takes onCheck spw returns nothing
            set this.spawnCheck=spw
        endmethod
        method addUnitSpawnFunction takes onUnitSpawn spw returns nothing
            set this.onUnitSpawn=spw
        endmethod
        method addWaypoint takes rect r returns nothing
            local region reg=CreateRegion()
            call RegionAddRect(reg,r)
            call TriggerRegisterEnterRegion(this.onEnter,reg,null)
            call this.wayPoints.link(r)
            set reg=null
        endmethod
        method removeWaypoint takes rect r returns nothing
            call this.wayPoints.unlink(r)
        endmethod
        //---== Allocation/Deallocation ==---
        static method create takes integer rawCode, integer count,player owner,real interval,string effectPath returns thistype
            local thistype this=thistype.allocate()

            set this.rawCode=rawCode
            set this.count=count
            set this.owner=owner
            set this.effectPath=effectPath
            set this.timer=NewTimer()
            set this.spawnCheck=defaultSpawnCheck
            set this.onEnter=CreateTrigger()
            set this.wayPoints=RectList.create()
            
            call SetTimerData(this.timer,this)
            call TriggerAddCondition(this.onEnter,Filter(function thistype.enter))
            if interval<0 then
                call TimerStart(this.timer,DEFAULT_INTERVAL,true,function thistype.periodic)
            else
                call TimerStart(this.timer,interval,true,function thistype.periodic)
            endif
            return this
        endmethod
        method destroy takes nothing returns nothing
            call ReleaseTimer(this.timer)
            call this.wayPoints.destroy()
            call DestroyTrigger(this.onEnter)
            call this.deallocate()
        endmethod
    endstruct
endlibrary


Please share your thought with me. (Except those "too long variable names", "you should compare 0 to your variable and not the opposite" kind of comments; my spaghetti should be on my plate and my code should be on my monitor. So I'm not interested in reading 'em)
 

Dirac

22710180
Reaction score
147
Observations
RectList destroy method should be static
The current way you use to link rects toguether is a bit more complicated than it should be
This dosn't require table, most likely some unit indexing system, would also increase the performance.
There is no .finish method, i guess you're talking about the Spawn start method, which should be static, and i really don't see why this is needed, you could just create those triggers for the rects along with the waypoint creating
Function interfaces? Why? they are so confusing!
this.addSpawnFunction(onSpawn(funcName))
I advise you to use Event instead. (And not even J4L, use Nes's)
Please write the all of the public functions at the code's header.
 

Sgqvur

FullOfUltimateTruthsAndEt ernalPrinciples, i.e shi
Reaction score
62
struct Spawn extends array
.
.
.
static method create ...
local thistype this=thistype.allocate()

So basically you write your script and ask people for feedback without making the effort to actually test it?
 

luorax

Invasion in Duskwood
Reaction score
67
Okay Dirac, you had some good points there :)

RectList destroy method should be static

I don't get why would it be different.

The current way you use to link rects toguether is a bit more complicated than it should be

Yes, actually it has some extra data. I added it for future updates (I had some fancy stuff in plan)

This dosn't require table, most likely some unit indexing system, would also increase the performance.

True, I've replaced it with an integer array and [ljass]GetUnitUserData[/ljass]

There is no .finish method, i guess you're talking about the Spawn start method, which should be static, and i really don't see why this is needed, you could just create those triggers for the rects along with the waypoint creating

True, again. I've rewritten an old script, forgot that I don't even need them.

I advise you to use Event instead. (And not even J4L, use Nes's)
Well, my map is built on J4L's event, and I'm not going to change it. However I'm not sure if I need it, I'm fine with the function interfaces.

So basically you write your script and ask people for feedback without making the effort to actually test it?
I don't get your post. It's working very well both on the test map and the real map. I just wanted to get some feedback. Dirac had some good points, for example.
 

Sgqvur

FullOfUltimateTruthsAndEt ernalPrinciples, i.e shi
Reaction score
62
luorax:
1. I don't get your post. It's working very well both on the test map and the real map.

1. What I meant was that it shouldn't be able to compile thus my guess that you've never tested it, so upload the test map?
 

luorax

Invasion in Duskwood
Reaction score
67
It should. Why? Because it has Alloc.

"So basically you write your post and ask me for a test map without making the effort to actually check the required libraries?"
 

Attachments

  • AoSSpawnSystem.w3x
    51.6 KB · Views: 185

Sgqvur

FullOfUltimateTruthsAndEt ernalPrinciples, i.e shi
Reaction score
62
luorax:

1. "So basically you write your post and ask me for a test map without making the effort to actually check the required libraries?"

1. Yes =). I don't know why you bother with custom allocators instead of using the jasshelper's generated ones, when not dealing with inheritance/specialization.

Okay, about the library:

1. I think the Spaw.create should take an argument for example initial delay, and delay between respawns. I think you would agree on that one.

2. Also say I want to change the creep spawning type, I wouldn't be able to do so using an instance of the Spawn struct.
You can expose a method to do so.

3. Instead of the Spawn.create taking an integer for the "rawCode" of the unit, it should take a list (maybe a string like this: "hfoo:hpea:hgry:ebal:etc.".
This I think would save a lot of typing.

4. I am not sure but wouldn't it be better if the Spawn.create could take real x, real y, real radius, bool is_rect (true = rect, false - a circular waypoint detection)
I think it's easier to point at the terrain location you want units to move to instead of opening the region palette and making a bunch of rects.
 

luorax

Invasion in Duskwood
Reaction score
67
No more useless double free protection, increasing its speed. The difference is not that bug, but since I only have to write [ljass]implement Alloc[/ljass] to get it, it's definitely worth it.
 

Sgqvur

FullOfUltimateTruthsAndEt ernalPrinciples, i.e shi
Reaction score
62
luorax:
1. but since I only have to write implement Alloc to get it, it's definitely worth it.

1. you forgot the "extends array" part, which is kinda cryptic.
 

Sgqvur

FullOfUltimateTruthsAndEt ernalPrinciples, i.e shi
Reaction score
62
luorax:

1. No more useless double free protection, increasing its speed.

2. No, array structs aren't cryptic.

1. You actually think that a signle if statement will slow down your map? And btw the double free is only in debug mode.

2. If you really think about it "struct mystruct extends array", now what is that supposed to mean? Well probably everyone will
think that mystruct is trying to specialize the array object type in some way, but "extends array" is used by vJass users
more like "uses custom allocator/deallocator" manner.

Laiev:
1. Do you know Alloc?

1. A useless module written in a hideous manner, which doesn't know how big jass arrays are (instance id of 8190 is perfectly valid).
 

Dirac

22710180
Reaction score
147
"extends array" is used by vJass users
more like "uses custom allocator/deallocator" manner.
I've never thought of it that way, remember that structs are nothing but a system of array variables, like a way to encapsulate them. An struct should extend array if you're working with, for example, player indexes, as you don't need allocators or deallocators because each player has already its own id, same thing goes to units with custom values. Also if you're trying to hint that the default struct allocator is better than alloc you're wrong.
You're not apporting anything to this thread. Looks to me all you want to do is fall into pointless arguments that in which my point of view, you're wrong in almost every single thing.
 

Laiev

Hey Listen!!
Reaction score
188
@Sgqvur

If you dislike something, it's ok... Is your opinion, but don't blame it if you're not sure about it.

vJASS style, 5fps for 1500 instances (create/destroy)
JASS:
globals
    integer a = 0
    integer array b
    trigger t = CreateTrigger()
endglobals

function D takes nothing returns boolean
    return false
endfunction
function E takes nothing returns boolean
    return false
endfunction

function B takes nothing returns nothing
    local integer c
    local integer x = 100
    local integer k
    loop
        exitwhen x == 0
        set c = b[0]
        if (c == 0) then
            set c = a + 1
            set a = c
        else
            set b[0] = b[c]
        endif
        set k = c
        set b[c] = b[0]
        set b[0] = c
        call TriggerEvaluate(t)
        set x = x - 1
    endloop
endfunction

struct tester extends array
    private static method onInit takes nothing returns nothing
        local integer x = 15
        call TriggerAddCondition(t, Condition(function D))
        call TriggerAddCondition(t, Condition(function E))
        loop
            exitwhen x == 0
            call TimerStart(CreateTimer(), .031250000, true, function B)
            set x = x - 1
        endloop
    endmethod
endstruct


Alloc style, 15 fps for 10,000 instances create/destroy
JASS:
globals
    integer a = 0
    integer array b
endglobals

function B takes nothing returns nothing
    local integer c
    local integer x = 500
    loop
        exitwhen x == 0
        set c = b[0]
        if (c == 0) then
            set c = a + 1
            set a = c
        else
            set b[0] = b[c]
        endif
        set b[c] = b[0]
        set b[0] = c
        set x = x - 1
    endloop
endfunction

struct tester extends array
    private static method onInit takes nothing returns nothing
        local integer x = 20
        loop
            exitwhen x == 0
            call TimerStart(CreateTimer(), .031250000, true, function B)
            set x = x - 1
        endloop
    endmethod
endstruct

5 fps for 1,500 instances vs 15 fps for 10,000 instances
 

Sgqvur

FullOfUltimateTruthsAndEt ernalPrinciples, i.e shi
Reaction score
62
Laiev:
1. If you dislike something, it's ok... Is your opinion, but don't blame it if you're not sure about it.

1. I've never written that I dislike it, I wrote that it was useless if inheritance/specialization is not used.

And seriously what's that script you posted supposed to demonstrate? And since when vJass's default instance deallocators have TriggerEvaluations to DoNothing?

Edit:

Dirac:

1. that in which my point of view, you're wrong in almost every single thing.

1. Which things exactly you think are wrong?
 

Dirac

22710180
Reaction score
147
And since when vJass's default instance deallocators have TriggerEvaluations to DoNothing?
I'm thinking since vexorian wrote it, but you tell me.

Which things exactly you think are wrong?
In this thread IMO the only statement that stands right before my eyes is "Also say I want to change the creep spawning type, I wouldn't be able to do so using an instance of the Spawn struct."

Example of wrong statement "useless if inheritance/specialization is not used." Obviously alloc is better than common struct allocation methods, so it's the opposite from useless, it should be the new common allocation method. Most of the times i write my own alloc in the structs i design since i get more control of those variables, and i preffer to work with static arrays rather than struct members.
 

Laiev

Hey Listen!!
Reaction score
188
@Sgqvur

You should click on to understand what the script is supposed to demonstrate, or read the thread of Alloc ^_^

You should know that [ljass]struct[/ljass] should always [ljass]extends array[/ljass], no matter if you use or not 'custom allocator/deallocator' as you said.

If you want discuss this, you should use Alloc thread, not this. I'll not discuss this here anymore.

@Luorax

I really sorry for all this OT on your thread |:
 

Sgqvur

FullOfUltimateTruthsAndEt ernalPrinciples, i.e shi
Reaction score
62
Dirac:
1. Most of the times i write my own alloc in the structs i design since i get more control of those variables, and i preffer to work with static arrays rather than struct members.

1. And you need more control of "those variables" because? Just use the default allocator & deallocator or maybe even Alloc ...

Laiev:

1. You should know that struct should always extends array, no matter if you use or not 'custom allocator/deallocator' as you said.
2. I'll not discuss this here anymore.

1. You can think/believe what you want, but it won't change the fact that what you wrote is a pointless/senseless statement without a proof.
2. Guess my question "And since when vJass's default instance deallocators have TriggerEvaluations to DoNothing?" won't be answered then.
 

Dirac

22710180
Reaction score
147
1. You can think/believe what you want, but it won't change the fact that what you wrote is a pointless/senseless statement without a proof.
He provided a benchmark and you still think hes wrong or getting information out of nowhere, don't you read what other people post?

2. Guess my question "And since when vJass's default instance deallocators have TriggerEvaluations to DoNothing?" won't be answered then.
I did answer "I'm thinking since vexorian wrote it, but you tell me." on the post above.
 

Sgqvur

FullOfUltimateTruthsAndEt ernalPrinciples, i.e shi
Reaction score
62
Dirac:
1. He provided a benchmark and you still think hes wrong or getting information out of nowhere, don't you read what other people post?
2. I did answer "I'm thinking since vexorian wrote it, but you tell me." on the post above.

1. fps is a benchmark?
2. I am telling you that the default vJass deallocators have no TriggerEvaluations! Hence what Laiev pasted is absolutly irrelevant. It's irrelevant even if there were TriggerEvaluatios. And what is your answer supposed to mean ("I'm thinking since vexorian wrote it")?
 
General chit-chat
Help Users
  • No one is chatting at the moment.

      The Helper Discord

      Members online

      No members online now.

      Affiliates

      Hive Workshop NUON Dome World Editor Tutorials

      Network Sponsors

      Apex Steel Pipe - Buys and sells Steel Pipe.
      Top