Bombardment Pack

[Moderator]

Bribe:

If you are going to convert this to JASS, then this needs some serious optimizations.

TONS of leaks as well.

Needs a complete re-write. Rejected.

 

[Archangel678]

Post triggers please.

The spells seem pretty simple. Its damage in a line via bombs. The ultimate lagged on my computers. Probably the fire.

The spell could be useful for certain games...

I won't rate until I see what others have to say.

 

[3yeballz]

Do not use locations. You can get rid of every location by using x/y.
If you use locations: remove them.

You must null every local handle, otherwise it leaks.

Instead of

    call CreateNUnitsAtLoc( 1, 'n000', GetOwningPlayer(a), PolarProjectionBJ(GetUnitLoc(a), 100.00, r2), r ) 
    set Bomber = GetLastCreatedUnit()

just do set Bomber = CreateUnit(GetOwningPlayer(a),...)

The waits are inaccurate but ok. It's anyway MUI.

Please inline your code to make it easier to read (loops).

 

[Adiktuz]

post triggers...

and get rid of those BJssssssssss

 

[Magtheridon96]

For the sake of everyone:

function Trig_Bombardment_Conditions takes nothing returns boolean
    if ( not ( GetSpellAbilityId() == 'A000' ) ) then // 'A000' is the spell
        return false
    endif
    return true
endfunction

function Trig_Bombardment_Actions takes nothing returns nothing
    local unit a = GetSpellAbilityUnit()
    local location s = GetSpellTargetLoc()
    local real r = AngleBetweenPoints(GetUnitLoc(a), s)
    local real r2 = r - 180
    local unit L = null
    local unit Bomber = null
    local integer m = 0
    call CreateNUnitsAtLoc( 1, 'n000', GetOwningPlayer(a), PolarProjectionBJ(GetUnitLoc(a), 100.00, r2), r ) 
    set Bomber = GetLastCreatedUnit()
    call IssuePointOrderLocBJ( Bomber, "move", PolarProjectionBJ(GetUnitLoc(a), 1200.00, r) )
    call TriggerSleepAction(0.75)
    loop
    call CreateNUnitsAtLoc( 1, 'n001', GetOwningPlayer(a), PolarProjectionBJ(GetUnitLoc(Bomber), 155.00, r), r )
    set L = GetLastCreatedUnit()
    call IssuePointOrderLocBJ( L, "attackground", PolarProjectionBJ(GetUnitLoc(Bomber), 245.00, r) )
    call UnitApplyTimedLifeBJ( 1.00, 'BTLF', L )
    call TriggerSleepAction(.50)
    set m = m +1
    exitwhen (m==7)
    endloop
    call TriggerSleepAction(0.60)
    call RemoveUnit( Bomber )
endfunction

//===========================================================================
function InitTrig_Bombardment takes nothing returns nothing
    set gg_trg_Bombardment = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( gg_trg_Bombardment, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( gg_trg_Bombardment, Condition( function Trig_Bombardment_Conditions ) )
    call TriggerAddAction( gg_trg_Bombardment, function Trig_Bombardment_Actions )
endfunction

First of all, u clearly did this in GUI then converted it to JASS and added locals...
I do that too xD xD

You're using way too many BJS for code this small.
Try to use as many natives as possible, but most importantly, try to avoid BJs.
The only good BJs are ones like TriggerRegisterAnyUnitEventBJ(...)

Your condition function should be changed so that it says:

If GetSpellAbilityId() == 'A000' then
    return true
endif
return false

Instead of angle between points, do this:

local unit u = GetTriggerUnit()
local location loc = GetUnitLoc(u)
local real r = bj_RADTODEG * Atan2(GetLocationY(s) - GetLocationY(loc), GetLocationX(s) - GetLocationX(loc)) //This is the Angle between points function

For this:

call CreateNUnitsAtLoc( 1, 'n000', GetOwningPlayer(a), PolarProjectionBJ(GetUnitLoc(a), 100.00, r2), r )

Use CreateUnitAtLoc(....)

This line: set L = GetLastCreatedUnit() is ok

IssuePointOrderLocBJ( L, "attackground", PolarProjectionBJ(GetUnitLoc(Bomber), 245.00, r) ) should be replaced with the function IssuePointOrderLoc

Use UnitApplyTimedLife instead of the BJ form.


I can tell by the style of your code that you don't have JNGP.
I think Anachron posted a tutorial on how to download it and
implement it, but i can't remember the link so you're on your own
buddy

One more thing: For polar projections, this calculation would be better than the function call:

local real x = GetLocationX(source) + dist * Cos(angle * bj_DEGTORAD)
local real y = GetLocationY(source) + dist * Sin(angle * bj_DEGTORAD)
local location loc

set loc = Location(x, y)

 

[Adiktuz]

Wow, that code hurts...

GUI converted to custom script... sigh... not even some effort to remove the BJssss... and it leaks a lot!!!

Therefore: IFVR!

 

[ZEEP]

Aha I though polar projection was a laggy function
Alright that is something i didnt know about BJs and making calculations with locals rather than function
Still I dont understand if using too many BJs why does some things are better off with GUI triggers rather than JASS for instance I tried a loop for timewalk spell and it didng do very well , but thats another thing.

 

[Magtheridon96]

Actually, Gui IS Jass.
GUI uses BJs, making Jass much better.

Jass allows us to use native functions instead of BJs
BJs are bad because they're like functions that call other functions that call other functions that call other functions lol

We just might approve your spell if you remove the BJs, the leaks and give it readability.
The less red we see, the better

 

[Adiktuz]

why does some things are better off with GUI triggers rather than JASS for instance I tried a loop for timewalk spell and it didng do very well , but thats another thing.

I think you made the loop the wrong way, that's why...

@Magtheridon96 - your sig rocks!

 

[ZEEP]

I think you made the loop the wrong way, that's why...

@Magtheridon96 - your sig rocks!

Yea I just read that wait time cannot go under 0.27 and thats why the loop didnt work good

@Magtheridon96 I wanted to say there are many leakless GUI spells and even so complicated ones , but thats another thing.
All above users get 1 rep from me , exept Adiktuz , youl get it next time

 

[Adiktuz]

and oh, don't use waits... they're bad...

 

[ZEEP]

Alright i did this and that learn few stuff removed bjs and tried calculation rather than non-native functions heres what I get and sooo many problems...
Im starting to miss pollar projection :S :S :S
How am i going to make a replacement for 5 pollar projections thats like 15 more lines of code, for the ultimate spell
*EDIT* Angle between points is not right at all...Did i wrote something wrong?

    local unit a = GetSpellAbilityUnit()
    local location s = GetSpellTargetLoc()
    local location Loc = null
    local location tar = null
    local location drops = null
    local real r = bj_RADTODEG * Atan2(GetLocationY(GetUnitLoc(a)) - GetLocationY(s), GetLocationX(GetUnitLoc(a)) - GetLocationX(s))
    local real r2 = r - 180
    local unit L = null
    local unit Bomber = null
    local integer m = 0
    local real x = GetLocationX(GetUnitLoc(a)) + 100 * Cos(r2 * bj_DEGTORAD)
    local real y = GetLocationY(GetUnitLoc(a)) + 100 * Sin(r2 * bj_DEGTORAD)
    local real x1 = GetLocationX(GetUnitLoc(a)) + 1300 * Cos(r * bj_DEGTORAD)
    local real y1 = GetLocationX(GetUnitLoc(a)) + 1300 * Cos(r * bj_DEGTORAD)
    set Loc = Location(x, y)
    set tar = Location(x1 ,y1)
    set Bomber = CreateUnitAtLoc(GetOwningPlayer(a), 'n000' , Loc, r )
    call IssuePointOrderLoc( Bomber, "move", tar )
    call TriggerSleepAction(0.75)
    loop
       set x = GetLocationX(GetUnitLoc(Bomber)) + 150 * Cos(r * bj_DEGTORAD)
       set y = GetLocationY(GetUnitLoc(Bomber)) + 150 * Sin(r * bj_DEGTORAD)
       set Loc = Location(x, y)
       call CreateUnitAtLoc( GetOwningPlayer(a), 'n001', Loc, r )
       set L = GetLastCreatedUnit()
       call IssuePointOrderLoc( L, "attackground", Loc )
       call UnitApplyTimedLife( L, 'BTLF', 1.00 )
       call TriggerSleepAction(.50)
       set m = m +1
       exitwhen (m==7)
    endloop
    call TriggerSleepAction(0.60)
    call RemoveUnit( Bomber )

Do we have some improvement becouse this makes the bomber being created infront of caster flying minus 180 degrees of target point
And this is getting to look like vJASS trigger = million declarations for one stupid spell

 

[3yeballz]

You don't need all those declarations.

Please do not use any location.
GetLocationX(GetUnitLoc(a)) is the same like GetUnitX(a). Same for Y.

Use GetSpellTargetX() and GetSpellTargetY() instead of GetSpellTargetLoc().

Change set Bomber = CreateUnitAtLoc(GetOwningPlayer(a), 'n000' , Loc, r ) to set Bomber = CreateUnit(GetOwningPlayer(a), 'n000' , x, y, r ). Do the same in the loop.

   call CreateUnitAtLoc( GetOwningPlayer(a), 'n001', Loc, r )
   set L = GetLastCreatedUnit()

This doesn't work, because CreateUnitAtLoc doesn't save the last created unit.

Instead of call IssuePointOrderLoc( L, "attackground", Loc ) do call IssuePointOrder( L, "attackground", x, y)

You don't have to convert from degree to radiant and such things. So you don't need bj_RADTODEG and bj_DEGTORAD.
Only time you need bj_RADTODEG is when creating the unit.

 

[ZEEP]

Omg everybody has his own methods and ways
Finnaly the working uber improved jass code and for what?
For 1% less lag ingame but w/e atleast i learnd alot of things
AND STILL angle between points is 180 degrees more but good thing i have r2

function Trig_Bombardment_Actions takes nothing returns nothing
    local unit a = GetSpellAbilityUnit()
    local real x = GetUnitX(a)
    local real y = GetUnitY(a)
    local real x1 = GetSpellTargetX()
    local real y1 = GetSpellTargetY()
    local real r = bj_RADTODEG* Atan2(y-y1,x-x1)
    local real r2 = r - 180
    local unit L = null
    local unit Bomber = null
    local integer m = 0
    set Bomber = CreateUnit(GetOwningPlayer(a), 'n000' , x, y, r2)
    call IssuePointOrder( Bomber, "move",x+1300* Cos(r2* bj_DEGTORAD),y+1300* Sin(r2*bj_DEGTORAD))
    call TriggerSleepAction(0.75)
    loop
       set x = GetUnitX(Bomber) + 150 * Cos(r2* bj_DEGTORAD)
       set y = GetUnitY(Bomber) + 150 * Sin(r2* bj_DEGTORAD)
       set L = CreateUnit( GetOwningPlayer(a), 'n001', x, y, r2 )
       call IssuePointOrder( L, "attackground", x+40* Cos(r2* bj_DEGTORAD),y+40* Sin(r2* bj_DEGTORAD))
       call UnitApplyTimedLife( L, 'BTLF', 1.00 )
       call TriggerSleepAction(.50)
       set m = m +1
       exitwhen (m==7)
    endloop
    call TriggerSleepAction(0.60)
    call RemoveUnit( Bomber )
endfunction

Sry eyebals i cant give you anymore rep

 

[Adiktuz]

I suggest you remove that TSA, it kills the accuracy of the system...

 

[ZEEP]

I suggest you remove that TSA, it kills the accuracy of the system...

What do you sugest for a wait command?

 

[Adiktuz]

what else? ofc a timer loop...

 

[axleion]

Omg everybody has his own methods and ways
Finnaly the working uber improved jass code and for what?
For 1% less lag ingame but w/e atleast i learnd alot of things
AND STILL angle between points is 180 degrees more but good thing i have r2
Sry eyebals i cant give you anymore rep

from your words, i do believe you still don't understand why these people told you to do those blablabla....
i suggest you to read some guides or tutorial about leaks, mainly why leaks are BAD. that really help to encourage good spell making.

 

[Adiktuz]

Its because people have different methods to do things that we have made the what you call "standards"...

and what we're trying to do is to teach you on how to create spells using these standards...

 

[ZEEP]

Lol really? then i better commit suicide