• 🏆 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!

[vJASS] Outbreak!

Status
Not open for further replies.
Level 29
Joined
Mar 10, 2009
Messages
5,016
Hello hivers!
1) I want to ask why if I enable the .flush(), it doesnt work well...
2) Does it leak when Im not flushing?, (in my understanding, no if it's not a handle)...
3) Should I use call cast.flush() and call out.flush() only if condition (global integer index) is met?, like index==0?...
3) Id like suggestion of code pls (not nullings, use TU)...

Description:
Spawns units from the XY of caster, the spawned units patrol around the
caster, if the caster moves all spawned units dies. Qty of spawn units
depends on the level of the caster.


JASS:
/*
REQUIRES:
- Jass New Gen Pack (JNGP) by Vexorian
- RegisterPlayerUnitEvent by Magtheridon96
- T32 by Jesus4Lyf
- Table by Bribe
*/

scope Outbreak

globals
    private constant integer      SPELL_ID = 'A00C' //starfall
    private constant integer       UNIT_ID = 'hfoo'
    private constant integer      ORDER_ID = 852183 //starfall
    private constant integer       MIN_AOE = 400
    private constant integer       MAX_AOE = 1100
    private Table out
    private Table cast
endglobals

private function GetUnitQty takes integer i returns integer
    return 0 + i * 10
endfunction

private function GetUnitLife takes integer i returns real
    return 10 + i * 10.
endfunction

private function GetDuration takes integer i returns real
    return 0 + i * 15.
endfunction

private interface Use
    unit caster
    real interval
    real xLoc
    real yLoc
endinterface

private struct Attacking extends Use
    unit spawn
    
    method periodic takes nothing returns nothing
        local real xEn
        local real yEn
        local integer distx
        local real angle
        if not IsUnitType(.spawn, UNIT_TYPE_DEAD) and GetUnitCurrentOrder(.caster)==ORDER_ID then
            set .interval = .interval + T32_PERIOD
            if .interval > 3 then
                set .interval = 0
                if GetUnitCurrentOrder(.spawn)!=851990 then //patrol
                    set distx = GetRandomInt(MIN_AOE, MAX_AOE)
                    set angle = GetRandomReal(-5,5)
                    call IssuePointOrderById(.spawn, 851990, .xLoc + distx * Cos(angle), .yLoc + distx * Sin(angle))
                endif
            endif        
        else
            call KillUnit(.spawn)
            call .stopPeriodic()
            call .deallocate()
        endif    
    endmethod
    
    implement T32x   
    
    static method create takes unit c,unit u, real x, real y returns thistype
        local thistype this = thistype.allocate()
        set .caster = c
        set .spawn = u
        set .xLoc = x
        set .yLoc = y
        set .interval = 0
        call .startPeriodic()
        return this
    endmethod
endstruct

private struct Outbreak extends Use
    real duration
    real life
    integer maxunit
    
    method periodic takes nothing returns nothing
        local unit d
        local integer count
        local integer casterID
        local integer dID
        if .duration > 0 and GetUnitCurrentOrder(.caster)==ORDER_ID and not IsUnitType(.caster, UNIT_TYPE_DEAD) then
            set .interval = .interval + T32_PERIOD
            set .duration = .duration - T32_PERIOD
            if .interval > 1 then
                set .interval = 0
                if .maxunit > (cast[GetHandleId(.caster)]) then
                    set casterID = GetHandleId(.caster)
                    set cast[casterID] = cast[casterID] + 1
                    set d = CreateUnit(GetOwningPlayer(.caster), UNIT_ID, .xLoc, .yLoc, 0)
                    set dID = GetHandleId(d)
                    call UnitApplyTimedLife(d, 'BTLF', .life)
                    set out[dID] = 1
                    set out.unit[dID] = .caster 
                    call Attacking.create(.caster, d, .xLoc, .yLoc)
                    set d = null
                    //call BJDebugMsg(I2S(cast[casterID]))
                endif
            endif
        else
            //call out.flush()
            //call cast.flush()
            call IssueImmediateOrder(.caster, "stop")
            call .stopPeriodic()
            call .deallocate()
        endif 
    endmethod
    
    implement T32x
    
    static method create takes nothing returns thistype
        local thistype this = thistype.allocate()
        local integer level = GetUnitAbilityLevel(GetTriggerUnit(), SPELL_ID)        
        set .caster = GetTriggerUnit()
        set .xLoc = GetUnitX(.caster)
        set .yLoc = GetUnitY(.caster)
        set .interval = 0.
        set .duration = GetDuration(level)
        set .maxunit = GetUnitQty(level)
        set .life = GetUnitLife(level)
        call .startPeriodic()       
        return this
    endmethod

    private static method cond takes nothing returns nothing
        if GetSpellAbilityId()==SPELL_ID then
            call thistype.create()       
        endif 
    endmethod
    
    private static method death takes nothing returns nothing
        local integer casterID
        local integer id
        local unit u
        local unit caster
        if GetUnitTypeId(GetTriggerUnit())==UNIT_ID then
            set u = GetTriggerUnit()
            set id = GetHandleId(u)
            if out[id]==1 then
                set caster = out.unit[id]
                set casterID = GetHandleId(caster)
                set cast[casterID] = cast[casterID] - 1
                set caster = null
                //call out.flush()
            endif            
            set u = null
        endif    
    endmethod 
    
    private static method onInit takes nothing returns nothing
        call RegisterPlayerUnitEvent(EVENT_PLAYER_UNIT_SPELL_EFFECT, function thistype.cond) 
        call RegisterPlayerUnitEvent(EVENT_PLAYER_UNIT_DEATH, function thistype.death)
        set out = Table.create()
        set cast = Table.create()
    endmethod
endstruct

endscope
 
Last edited:
Level 7
Joined
Apr 30, 2011
Messages
359
well . . . .
i think flushes remove all saved things inside the table
you'll get some bug with operator .has if you don't flush it

it can be wrong too ~
 
Level 7
Joined
Apr 30, 2011
Messages
359
aren't UNIT_ID supposed to be SUMMONED_UNIT_ID ?
for readability reasons

use textmacros instead of interfaces just for those CnP things

JASS:
    private static method death takes nothing returns nothing
        local integer casterID
        local integer id
        local unit u
        local unit caster
        if GetUnitTypeId(GetTriggerUnit())==UNIT_ID then
            set u = GetTriggerUnit()
            set id = GetHandleId(u)
            if out[id]==1 then
                set caster = out.unit[id]
                set casterID = GetHandleId(caster)
                set cast[casterID] = cast[casterID] - 1
                set caster = null
                //call out.flush()
            endif            
            set u = null
        endif    
    endmethod
for extra simplicity use this:
JASS:
    private static method death takes nothing returns nothing
        if GetUnitTypeId(GetTriggerUnit())==UNIT_ID then
            if out[GetHandleId(GetTriggerUnit())]==1 then
                set cast[GetHandleId(out.unit[GetHandleId(GetTriggerUnit())])] = cast[GetHandleId(out.unit[GetHandleId(GetTriggerUnit())])] - 1
                //call out.flush()
            endif
        endif
    endmethod
long . . . you can still use that casterID, for a bit shortness . . .
 
Level 7
Joined
Apr 30, 2011
Messages
359
you make some . . .
JASS:
    static method create takes unit c,unit u, real x, real y returns thistype
        local thistype this = thistype.allocate()
        set .caster = c
        set .spawn = u
        set .xLoc = x
        set .yLoc = y
        set .interval = 0
        call .startPeriodic()
        return this
    endmethod

interfaces duplicate codes, you should use textmacros . . .

JASS:
    //! textmacro VARIABLES
        real x
        real y
        real z
        real a
    //! endtextmacro
    
    struct ThisIsXYZ
        //! runtextmacro VARIABLES()
        
        method operator This takes nothing returns real
            return this.x * this.y * this.z
        endmethod
    endstruct
    
    struct ThisIsA
        //! runtextmacro VARIABLES()
        
        method operator This takes nothing returns real
            return this.a
        endmethod
    endstruct
    
    // interfaces should be used like this:
    private interface Point
        private real x
        private real y
        private real z
    endinterface
    
    //! textmacro BASE
        method operator x= takes real x returns nothing
            set this.x = x
        endmethod
        
        method operator x takes nothing returns real
            return this.x
        endmethod
        
        method operator y= takes real y returns nothing
            set this.y = y
        endmethod
        
        method operator y takes nothing returns real
            return this.y
        endmethod
    //! endtextmacro
    
    struct 2D extends Point
        //! runtextmacro BASE()
    endstruct
    
    struct 3D extends Point
        //! runtextmacro BASE()
        
        method operator z= takes real z returns nothing
            set this.z = z
        endmethod
        
        method operator z takes nothing returns real
            return this.z
        endmethod
    endstruct
    
    function GetDistance takes Point p1, Point p2 returns real
        if p1.getType() == 2D.typeid then
            // blablabla
        elseif p1.getType() == 3D.typeid then
            // blablabla
        endif
        
        if p2.getType() == 2D.typeid then
            // blablabla
        elseif p2.getType() == 3D.typeid then
            // blablabla
        endif
        
        return // blablabla
    endfunction
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
Is there a particular reason why you set the random angle like set angle = GetRandomReal(-5,5)? If you wanted every angle to have an equal chance it should be something like set angle = GetRandomReal(0, bj_PI*2).

You could make an if-statement for call KillUnit(.spawn) so it won't be called again if the spawn had died.

You should be using Table's remove method to get rid of the stored data, not flush (You're using Bribe's Table, right?).

I would have just combined Attacking into Outbreak, sacrificing organization for avoiding the use of the tables and unit death event. You would have a unit array member that would store the spawns created for the caster and another real member for the interval giving patrol orders.

Those local unit variables in static method death are pretty much useless since they only get referred to once.
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
- I did the angle to avoid extra calculations, it should have been (-1,1) but it
only patrols in front or side of the caster...

- Well noted for the KillUnit...

- flush works fine now, so there's no need to adjust that...

- I cant really combine the two coz Outbreak is the creation while Attacking is
the periodic engaging, except if I make the set d = CreateUnit(,
into an arrayed member unit and index it within the Outbreak periodic loop...

- Yeah, the 'u' and 'caster' is useless, not the integers, but 'death' method also
is also useless since the unit dies anyway in the Attacking struct...

Thanks! and congtarz for your 1000 posts :)...
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
- I did the angle to avoid extra calculations, it should have been (-1,1)
No, it shouldn't have been (-1, 1). It's supposed to be (0, bj_PI * 2).

Cos and Sin uses radians.

So, -1 and 1 would have given you a random angle in this range:
For the sake of the visualization, let's just say two PI is 6.2
attachment.php


hence
but it only patrols in front or side of the caster...
-5 to 5 "works" because you manage to cover all the angles, but it skews the probability to favor angles on the left side more.
attachment.php

A range from 0 to two pi covers all the angles without skewing probability like that.
- flush works fine now, so there's no need to adjust that...
remove is a much better option. All you have to do is remove the unit's handle id from the table which frees up storage for that case only, rather than trying to empty the entire table.

OT:
Thanks! and congtarz for your 1000 posts :)...
Thanks! ^_^
 

Attachments

  • Radian Range of -5 to 5.jpg
    Radian Range of -5 to 5.jpg
    64.7 KB · Views: 205
  • Radian Range of -1 to 1.jpg
    Radian Range of -1 to 1.jpg
    71.8 KB · Views: 134
The thing is instead of Cos(GetRandomReal(-bj_PI, bj_PI)) Sin(GetRandomReal(-bj_PI, bj_PI)), he would get the exact same results replacing cos & sin with simply "GetRandomReal(-1, 1)" (because cos and sin return values between -1 and 1).

However, if someone wants to get a random point in a circle, it is essential to store the value of GetRandomReal(-bj_PI, bj_PI), and then reference that angle variable for both cos and sin.

Does this make sense?

This is what mckill2009 is doing:

JASS:
Cos(GetRandomReal(-bj_PI, bj_PI))
Sin(GetRandomReal(-bj_PI, bj_PI))

This is what he could do if he simplified it:

JASS:
GetRandomReal(-1, 1)
GetRandomReal(-1, 1)

Both would return the same range of values when calculated, but both solutions would be going to a random point in a square, not a circle. If you want a circle, you have to do this:

JASS:
set angle = GetRandomReal(-bj_PI, bj_PI)
Cos(angle)
Sin(angle)
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
^Yes, but that doesn't really have any relevance to what I was trying to explain.
JASS:
                    @set angle = GetRandomReal(-5,5)@
                    call IssuePointOrderById(.spawn, 851990, .xLoc + distx * @Cos(angle)@, .yLoc + distx * @Sin(angle)@)
He was getting a random angle with (-5, 5), which "works" but has its problems like I mentioned earlier.
I was saying that it was better to get the random angle with (0, bj_PI *2), but a range of (-bj_PI, bj_PI) is probably more appealing to mckill2009 since it doesn't have that multiplication.


You could just avoid using Cos/Sin and getting a random angle if you just make the units move in a square around the caster instead of a circle.
 
Instead of using Sin and Cos and that random real, you can use:

GetRandomReal(0, 0.28366) instead of the Cos
GetRandomReal(-0.95892, 0.95892) instead of the Sin.

Don't worry, I took into account that you're using GetRandomReal(-5, 5)

My solution will work exactly like your current setup, but mine uses one less function call.


-----

Now that that's settled, let's move on.

1) I know that the interface isn't going to generate that much code in it's current setup, but ... it terrifies me D:

2) You should indent everything after scope and stop at endscope
It's cleaner for some odd reason :3

3) Use SpellEffectEvent for a more efficient execution of the spell.

4) When checking if a unit is dead or not, you should also make sure it's ID isn't 0 (GetUnitTypeId(unit) != 0)
 
Level 29
Joined
Mar 10, 2009
Messages
5,016
Both would return the same range of values when calculated, but both solutions would be going to a random point in a square, not a circle.
that's the problem that's why I keep calling Cos & Sin coz as what the description says, "patrol around the caster"...
and I already did that (-5,5) in the script...

@Mags
1) I already explained it in post#5 why I made the interface...
2) I have no idea what you're talking about indention, imo, it's properly indented...
3) The peoblem of SpellEffect event is that it requires your library, I know
that its more efficient coz I dont need to write the method cond
for the spell, I just really dont like that it has a requirement, although I use
it sometimes in my map...
4) I havent tested that, but seems without that its working really fine...

anyway, I've removed the death method and Table out, and its working well also...
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
Instead of using Sin and Cos and that random real, you can use:

GetRandomReal(0, 0.28366) instead of the Cos
GetRandomReal(-0.95892, 0.95892) instead of the Sin.
This is wrong. (-1, 1) for both would be more correct though it would result in a square, but it really shouldn't matter anyway when you're just issuing orders to a unit anyway.

but seems without that its working really fine...
Just because it seems that it's working fine doesn't mean that it is working fine, mostly in part thanks to WC3's weird way of doing mostly everything. IsUnitType(u, UNIT_TYPE_DEAD) doesn't account correctly for removed units or some weird case like that.
 
UNIT_TYPE_DEAD returns false for removed units because "UNIT_TYPE_DEAD" is a flag, and flags are reset to false when the handle is removed. Just like getting the X/Y of a removed location will return 0 for both.

Obviously it would have been smarter to make a UNIT_TYPE_ALIVE and to included it in the GUI list or to make UnitAlive part of the common.j API or for Vexorian to not have made custom natives break the optimizer so that UnitAlive can actually work on an optimized map.
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
But it doesn't which is why I said it was wrong.
(-5, 5) will cover -1 to 1, since it covers (-bj_PI, bj_PI).

This is what a thousand points would look like with the range you described:
The red dot is the center point
attachment.php

This is his way with (-5, 5):
attachment.php
 

Attachments

  • Mag.jpg
    Mag.jpg
    12.5 KB · Views: 126
Last edited:
Status
Not open for further replies.
Top