Arrow System v1.5

[Moderator]

This system could be useful.
Suggested changes

• Use coordinates instead of locations
• Use an integer instead of GroupCount founction and ForGroup
• Player variable does not need to be nulled
• You could pause/resume the timer instead of using a trigger

Why does CreateArrow have destructable as function parameter, but it is not used?

 

[maddeem]

Questions anyone?

 

[-Kobas-]

Try to use coordinates instead of locations.
Also there is no need for BJ functions.

Btw why you split those 2 scripts, if you use library then put merge them together.

 

[Existas]

Your arrow system works, however there are few things that I dislike.

1. Do not make your arrow destroy itself when the unit reaches its destination, rather make sure the unit follows the arrows guidance to perform an action which should destroy the arrow guiding (since he accomplished something already and doesn't need to be guided).
2. You should consider including some eyecandy on arrow's disappearance. It looks very static right now. Do it with an effect or arrow animation (which you may now be short on).
3. Reconsider using a unit instead of a destructable. This provides you with following capabilities: vertex coloring, animation time scaling, scaling and moving, Vexorian projectile dummy model usage (making your arrow system able to point at high point or low ground), but provides and few disadvantages: GetLocalPlayer() usage exacerbation (in this case you would need to use vertex coloring to hide the arrow from other players) and additional hard work.
4. Your codding part is rather messy.

private integer Arrow = 'B000'
public real ArrowTimeOut = 0.03
public real DestroyDistance = 200.

Consider making these constant
Changes:

private constant integer Arrow = 'B000'
public constant real ArrowTimeOut = 0.03
public constant real DestroyDistance = 200.

As -Kobas- mentioned you should merge your triggers into one.
Consider changing these public members to private and provide the library with public functions to get those members in case of emergency (which I honestly hope wont happen in a lifetime)

public hashtable ArrowHash = InitHashtable( )
public group ArrowGroup = CreateGroup( )

Help yourself - create a GetArrow function which takes a unit and returns a destructable (obviously get unit's arrow)

if IsUnitGroupEmptyBJ( ArrowGroup ) == true then

Can be changed to:

if FirstOfGroup( ArrowGroup ) == null then

Which is so much faster

and

        call ShowDestructable( d, false )
        if GetLocalPlayer( ) == p then
            call ShowDestructable( d, true )
        endif

Can be changed to (I know people can get messed up, but this is quite simple):

call ShowDestructable( d, GetLocalPlayer( ) == p )

On top of that: get used to using coordinates instead of locations (they can get really messy in a big project and leave a huge memory leak if you don't know what you're doing). Practicing coordinates now gives your further coding life an excellent contribution.

If you just practice, it's good you're doing so, but if you need a good arrow system you should make my mentioned changes (2 and 3 parts are optional) or I can create such library for your use (no credits, just rep).

Wish you best luck!

 

[maddeem]

I wanted to use locations to make this gui friendly, also I tried merging them... proved to be quite difficult
Edit: Got rid of the BJs >;p

 

[Pharaoh_]

Use a timer instead of a timed trigger then; either way, the timer's expiration is linked with a function.

 

[maddeem]

I dunno how to use timers :S

 

[Existas]

Then you got a long way to go.

Timer usage:

function TimerFunction takes nothing returns nothing
    call BJDebugMsg("timer function call")
    //TimerFunction actions here, depending on what you want to do
    //There are so many bonus posibilties what timers can do including but not limited to struct attachment.
endfunction

function InitTrig_Timer takes nothing returns nothing
    local timer t = CreateTimer()
    call TimerStart(t, 0.03, true, function TimerFunction)
endfunction

What timers do is absolutely simple, they call a function everytime their time run out (that's called a time out) and refreshes it or not based on the third parameter (which is true here and makes the timer keep going forever)

I wanted to use locations to make this gui friendly, also I tried merging them... proved to be quite difficult

And merging them isn't that difficult as you would have thought. Simple.

Make your library an initializer:

library ArrowSys initializer Startup

    private function Startup takes nothing returns nothing
        //Here goes the code for startup: timer definition and timer start
    endfunction
endlibrary

 

[Magtheridon96]

The Timer API is extremely simple.

CreateTimer()
DestroyTimer(timer)
TimerStart(timer, timeout, isPeriodic, code)
PauseTimer(timer)
ResumeTimer(timer)
TimerGetTimeout(timer)
TimerGetElapsed(timer)
TimerGetRemaining(timer)

All you need here is CreateTimer, TimerStart and PauseTimer.

Examples of usage:

local timer t = CreateTimer()
call TimerStart(t, 0.03125, true, function onRun)
call PauseTimer(t)

 

[maddeem]

Blarg it is too troublesome to use timers >:S

 

[Existas]

I'm terribly sorry that I have to be this harsh but...

Give Credit if used!

This line is pointless if you won't update your system as friendly hive members offered you to.
Well unless people use GUI and don't give a single fuck about it's performance.

I can create such a system from scratch in around 10...20 minutes (including my level of experience) you should be able to do the same in about 5x more time, which has to be 50...100 minutes or harshly around an hour or two.

Just to give you a push... I'll give +rep if you manage to update it into a perfect state.

Do not get my post as a must do and do whatever you think is right.

Happy coding!

 

[Magtheridon96]

They're still way faster.

 

[maddeem]

I really did try to use them, it kept crashing my map, I cant even debug that crap. I'm not going to use timers.

 

[timothyhuth]

Try to use coordinates instead of locations.
Also there is no need for BJ functions.

Btw why you split those 2 scripts, if you use library then put merge them together.

Bj functions.

 

[maddeem]

Wut? I removed those...

 

[-Kobas-]

May I suggest:

 private integer Arrow = 'B000'
        public real ArrowTimeOut = 0.03
        public real DestroyDistance = 200.
        //End Configurables
        public hashtable ArrowHash = InitHashtable( )
        public group ArrowGroup = CreateGroup( )
        private boolean boob

to be changed into

    globals
        //Configurables
        private integer    ARROW_ID       = 'B000'
        private real       LOOP           = 0.03
        private real       MIN_DISTANCE   = 200.
        //End Configurables
        
        private hashtable  HASH          = InitHashtable( )
        private group      G             = CreateGroup( )
        private boolean    B             = false
        private trigger    T             = CreateTrigger()
    endglobals

There is no need for long names in this small script.
Also there is no need for public variables you can merge scripts.

Also function names are horrible
Even variables are bad:
function ArrowU2U takes real size, real Height, unit AttachedUnit, unit TargetUnit returns nothing
can be
function ArrowU2U takes real size, real z, unit u, unit target returns nothing

 

[Pharaoh_]

I dunno how to use timers :S

Mhm, so you tell everyone how dumb their coding is and you can't use timers.. Now that's something I did not expect.
Anyway, apart from my poisonous comment, timers will save your life. You won't need to use a trigger, but a simple function!

 

[Magtheridon96]

private boolean boob

That's my favorite bit-sized data-type

 

[maddeem]

Mhm, so you tell everyone how dumb their coding is and you can't use timers..

What?

Kobas, most of those must be public, so I had to have realistic names.

Jesus, this is my first vJass script or any scripting language at all. You have some high expectations.

 

[-Kobas-]

What?

Kobas, most of those must be public, so I had to have realistic names.

Jesus, this is my first vJass script or any scripting language at all. You have some high expectations.

You don't need public variables for that, everything can be crated with private ones, use public functions to link this system with any other.

 

[maddeem]

Combined the code

 

[-Kobas-]

Function names are bad, variables as well.
Once again you should use coordinates instead of locations, global functions can be declared later that will allow you to do same shit with locations.
Use TAB to inline code.

And check the best tutorial there is http://www.wc3c.net/vexorian/jasshelpermanual.html (actually it's not tutorial but it will do just fine).

 

[Ruke]

Function names are bad, variables as well.

They are not bad, they just have long names e__e.

Once again you should use coordinates instead of locations

Locations seems to be more faster than coordinates.

Player variable does not need to be nulled

But is a good practise.

That you (maddeem) should really use are structs. In this case, they would make your code more easy to read and "easy to program" it.

Also, try to use Table by Bribe instead of the hashtable .

Greetings .

 

[Iconer]

lol he said he ar bliz staf!

 

[Cokemonkey11]

Should read this http://www.wc3c.net/vexorian/jasshelpermanual.html#lib

Learn to use the

initializer

keyword. Your script might work, but it's ugly.

People have high expectations here because you posted it in the "spells" section. This section is for examples of good code for others to use and learn from.

 

[maddeem]

Updated the code, hope its better

 

[Cokemonkey11]

I'd say at this point it's quite usable, and probably useful for someone. But it's certainly not perfect.

Your script only attaches to units. As a general rule, if only one object can be attached to a unit at a time (and therefore have a maximum number of instances equal to the number of units) you should not use hashtables, but instead use a linear stack.

This will significantly speed up your code. If you'd like an example I'd be happy to provide.

At this point I'd vote for the script to be approved with 3 stars.

***

 

[maddeem]

Linear stack? Sure, I'd like to see an example

 

[Cokemonkey11]

Here's an example of an MUI spell using a linear stack.

scope warSlam initializer i
    private struct unpauseDat
        unit u
        real timeLeft
        boolean attacked
    endstruct
    
    globals
        private constant integer SHOUTID='A00R'
        private constant integer IMPALEID='A00F'
        private constant real AVERAGEUNITHEIGHT=60.
        private constant real FIDELITY=.05
        private constant real MAXSHOUTPOWER=40.
        private constant real PAUSETIME=.45
        private constant real FOLLOWTHROUGHTIME=.2
        private constant real SHOUTRANGE=600.
        private constant string SLAMMODEL="Abilities\\Spells\\Human\\MarkOfChaos\\MarkOfChaosTarget.mdl"
        private constant string SLAMSOUND="Units\\Orc\\HeroTaurenChieftain\\WarStompBirth1.wav"
        private group grp=CreateGroup()
        private integer dbIndex=-1
        private timer time=CreateTimer()
        private unit tempCaster
        private unpauseDat array unpauseDB
        private real tempCasterX
        private real tempCasterY
        private real tempCasterZ
    endglobals

    private function f takes nothing returns boolean
        local real distX
        local real distY
        local real distZ
        local real fX
        local real fY
        local real fZ
        local real power
        local real distXY
        local unit filter=GetFilterUnit()
        if IsUnitEnemy(filter,GetOwningPlayer(tempCaster)) and GetUnitState(filter,UNIT_STATE_LIFE)>=1 then
            set fX=GetUnitX(filter)
            set fY=GetUnitY(filter)
            set fZ=GetUnitFlyHeight(filter)+helper_getZ(fX,fY)+AVERAGEUNITHEIGHT
            set distZ=fZ-tempCasterZ
            set distX=fX-tempCasterX
            set distY=fY-tempCasterY
            set power=(SHOUTRANGE-SquareRoot(distX*distX+distY*distY+distZ*distZ))/SHOUTRANGE*MAXSHOUTPOWER
            set distXY=SquareRoot(distX*distX+distY*distY)
            call Knockback_add(filter,power,Atan2(fY-tempCasterY,fX-tempCasterX),Atan2(distZ,distXY))
        endif
        set filter=null
        return false
    endfunction
    
    private function p takes nothing returns nothing
        local unpauseDat tempDat
        local integer index=0
        loop
            exitwhen index>dbIndex
            set tempDat=unpauseDB[index]
            set tempDat.timeLeft=tempDat.timeLeft-FIDELITY
            if tempDat.timeLeft<=0 and tempDat.attacked==false then
                call PauseUnit(tempDat.u,false)
                call SetUnitTimeScale(tempDat.u,1)
                set tempDat.attacked=true
                set tempCasterX=GetUnitX(tempDat.u)
                set tempCasterY=GetUnitY(tempDat.u)
                set tempCasterZ=GetUnitFlyHeight(tempDat.u)+helper_getZ(tempCasterX,tempCasterY)
                call helper_sound3d(SLAMSOUND,tempCasterX,tempCasterY,GetUnitFlyHeight(tempDat.u))
                set tempCaster=tempDat.u
                call GroupEnumUnitsInRange(grp,tempCasterX,tempCasterY,SHOUTRANGE,Filter(function f))
            endif
            if tempDat.timeLeft<=(0-FOLLOWTHROUGHTIME) and tempDat.attacked then
                call UnitAddAbility(tempDat.u,IMPALEID)
                set unpauseDB[index]=unpauseDB[dbIndex]
                set dbIndex=dbIndex-1
                if dbIndex==-1 then
                    call PauseTimer(time)
                endif
                call tempDat.destroy()
            endif
            set index=index+1
        endloop
    endfunction
    
    private function c takes nothing returns boolean
        local effect fx
        local unpauseDat tempDat
        if GetSpellAbilityId()==SHOUTID then
            set tempDat=unpauseDat.create()
            set tempDat.u=GetTriggerUnit()
            set tempDat.attacked=false
            set tempDat.timeLeft=PAUSETIME
            set dbIndex=dbIndex+1
            set unpauseDB[dbIndex]=tempDat
            call UnitRemoveAbility(tempDat.u,IMPALEID)
            if dbIndex==0 then
                call TimerStart(time,FIDELITY,true,function p)
            endif
            set fx=AddSpecialEffect(SLAMMODEL,GetUnitX(tempDat.u),GetUnitY(tempDat.u))
            call DestroyEffect(fx)
            call SetUnitTimeScale(tempDat.u,2)
            call IssueImmediateOrder(tempDat.u,"stop")
            call PauseUnit(tempDat.u,true)
            call SetUnitAnimationByIndex(tempDat.u,3)
        endif
        set fx=null
        return false
    endfunction
    
    private function i takes nothing returns nothing
        local trigger t=CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ(t,EVENT_PLAYER_UNIT_SPELL_EFFECT)
        call TriggerAddCondition(t,Condition(function c))
        set t=null
    endfunction
endscope

Sorry I don't have time to make a more fitting example of one for a system like this, but if you still don't understand what I mean I can write something custom later.


Edit: I ended up having time now Here's a simple "arrow system" made with a stack.

library stackSystemExample
    private struct arrowData
        unit arrow
        unit attachment
        unit target
    endstruct
    
    globals
        private constant real FIDELITY=1./30.
        private timer time=CreateTimer()
        private arrowData array arrowDB
        private integer dbIndex=-1
    endglobals
    
    private function p takes nothing returns nothing
        local integer index=0
        local arrowData tempDat
        loop
            exitwhen index>dbIndex
            set tempDat=arrowDB[index]
            call SetUnitX(tempDat.arrow,GetUnitX(tempDat.attachment))
            call SetUnitY(tempDat.arrow,GetUnitY(tempDat.attachment))
            call SetUnitFacing(tempDat.arrow,Atan2(GetUnitY(tempDat.target)-GetUnitY(tempDat.attachment),GetUnitX(tempDat.target)-GetUnitX(tempDat.attachment))*bj_RADTODEG)
            set index=index+1
        endloop
    endfunction
    
    public function removeArrow takes unit attachment returns nothing
        local integer index=0
        local boolean found=false
        local arrowData tempDat
        loop
            exitwhen index>dbIndex or found
            set tempDat=arrowDB[index]
            if tempDat.attachment=attachment then
                set found=true
                call RemoveUnit(tempDat.arrow)
                call tempDat.destroy()
                set arrowDB[index]=arrowDB[dbIndex]
                set dbIndex=dbIndex-1
                if dbIndex==-1 then
                    call PauseTimer(time)
                endif
            endif
            set index=index+1
        endloop
    endfunction
    
    public function addArrow takes unit attachment, unit target returns nothing
        local arrowData tempDat=arrowData.create()
        set arrowData.arrow=CreateUnit(...)
        set arrowData.target=target
        set arrowData.attachment=attachment
        set dbIndex=dbIndex+1
        set arrowDB[dbIndex]=tempDat
        if dbIndex==0 then
            call TimerStart(time,FIDELITY,true,function p)
        endif
    endfunction
endlibrary

 

[maddeem]

That can be achieved with hashtables. I don't have it stack because that is a useless feature.. If someone were to need it, I would add it.

 

[Cokemonkey11]

This will significantly speed up your code.

I'm not sure why I said that to be honest. Hashtables aren't that slow that it matters.