Astonishing Beam [Paladon]

[Moderator]

20:19, 4th Jun 2009
hvo-busterkomo: The effect is awesome, and the scripting is fine, except a few complaints:

1. Your interval should be a constant, really.
2. Inlining TriggerRegisterAnyUnitEventBJ is lame. While a null boolexpr leaks, yours is only leaking 16 times a game, and the leak is too incredibly insignificant for it to matter.
3. You should inline the PolarProjection, and use radians.

 

[baassee]

holy crap, g2 download now!

 

[Paladon]

Well, i haven't tested it already with that terrain, i hope it takes terrain height into account.

 

[hell gate]

a good spell for mass slow. 4,5/5 for this great spell.
i can't say something about the script, because i don't know JASS or vJASS

 

[Yearight]

Really nice spell, gj. Dunno if that means anything to you since all I can make is GUI. 5/5 but the bad thing is that it doesn't take terrain height into account. So the orb floats higher than the beam. (If casted on a higher surface)

 

[Reaper2008]

Don't have time to test right now, but why use a custom model for a dummy? Zone Indicator works just fine.

 

[Paladon]

Really nice spell, gj. Dunno if that means anything to you since all I can make is GUI. 5/5 but the bad thing is that it doesn't take terrain height into account. So the orb floats higher than the beam. (If casted on a higher surface)

Interesting. It takes the height into account for me now ( i just tested it).
There may just be a little difference if you cast it exactly above a rock/tree or whatever.

 

[Yearight]

I will post a picture then 1 min.

 

[nerovesper]

Had fun testing this, specially when the units get small. Not much good like your other ones though. Still seeing your name as the creator of any spell are worth downloading. xD
4.5/5

 

[Paladon]

I am sorry, there was a terrain height bug indeed.
I simply uploaded an older version.
It's fixed, of course.
Feel free to test it again, if you don't mind.

 

[Deuterium]

Creative and original takes terrain height into account and works flawlessly.
Job well-done!

 

[-JonNny]

You switched from GUI to vJass quite fast and your code is really good

Whats this supposed to do ?

loop
    set tempU = FirstOfGroup(d.PickedGroup)
    exitwhen tempU == null
    call GroupRemoveUnit(d.PickedGroup,tempU)
endloop

I dont know it thats really necesarry and call GroupClear(d.PickedGroup) would be way shorter

Wooza , everywhere locationleaks , didnt expect that
set d.light = AddLightningEx(RayModel,true,d.x,d.y,GetLocationZ(Location(d.x,d.y))+FlyHeight,PPX,PPY,GetLocationZ(Location(PPX,PPY)))

=>Location(PPX,PPY) and Location(d.x,d.y) leaks
i gues you didnt know that Location(x,y) leaks

Your dummy has not the desired height because of being a flying unit which gets affected of terrain height and trees ect.-> the beam does not alway start in the dummy
Edit : kk , ive read you fixe that already

Else its fine and looks cool, fix those small things ill rate it [5/5]

 

[Paladon]

Okay, the GroupClear is new for me and definitely a great info
About the locations, that happened as i added the terrain height.
I completely forgot that. Thank you.

 

[Dark_Dragon]

Hi Paladon!

nice to see u again making spells!

so lets get started xD

spells idea is nice and something new here which is already good enough!

is well documented + code is very nice (i mean readable) coz i can hardly find any jasser with nice explained code...

as well code is short which makes it awesome for me to quickly read it xD i am quite lazy sometimes

k so bad things about coding are:
1. has few leaks
2. *suggestion* you should try using dynamic objects (creating and destroing) rather then wasting VM whole the game and slowing start up. but thats only a suggestion

so where are leaks?

call TriggerRegisterPlayerUnitEvent(Trig, Player(i),EVENT_PLAYER_UNIT_SPELL_EFFECT, null)

// is in a loop which allocates memory as null pointer which will nevar again be free for use and takes space for nothing + slows game future objects generation...

globals
      private group    PickGroup = CreateGroup()
endglobals

// you leak this group coz its never destroyed

// that group loaded on map startup is now replaced with this one every 0.01 seconds
set PickGroup = CreateGroup()
this group as well leaks coz its not destroyed (so 100 group leaks per second)


// this group as well leaks once per cast coz its never destroyed but just gets genrated each cast
set d.PickedGroup  = CreateGroup()


// and last is double point leak once per cast
set PPX = GetLocationX(GetSpellTargetLoc())
set PPY = GetLocationY(GetSpellTargetLoc())

this are all i can find xD
well coding is still nice but try to fix this i pointed out!

as well try to optimize this

loop
 set tempU = FirstOfGroup(d.PickedGroup)
 exitwhen tempU == null
 call GroupRemoveUnit(d.PickedGroup,tempU)
endloop

you never destroye that group and you overwrite it with next cast and even if you would use it its best to just use GroupClear(<whichGroup>) native for that

well all other seems to be fine.

Best Regards!
~Dark Dragon

EDIT:
i missed this locations -JonNny pointed xD thanks -JonNny xD +rep

 

[D1000]

Screenshot looks great --> Downloading

 

[Paladon]

globals
      private group    PickGroup = CreateGroup()
endglobals

// you leak this group coz its never destroyed

As soon as i destroy that group, the group doesn't exist anymore. And as soon as the group doesn't exist, the spell won't work properly.

call TriggerRegisterPlayerUnitEvent(Trig, Player(i),EVENT_PLAYER_UNIT_SPELL_EFFECT, null)

// is in a loop which allocates memory as null pointer which will nevar again be free for use and takes space for nothing + slows game future objects generation...

And how may i improve that?

 

[Dark_Dragon]

As soon as i destroy that group, the group doesn't exist anymore. And as soon as the group doesn't exist, the spell won't work properly.



And how may i improve that?

no no xD its coz u generate new groups if u want to use only that one then u must not overwrite it with another one coz CreateGroup() = Location(x, y) both leak. its just that groups are sometimes used as temp variable where u only add / remove units. but in ur code you create groups whole the time and never destroy. problem is that groups leak fare more then locations :S and i know what u want to say, u want to use that global group 'only' but ur code currently does actually never use that group coz it every cast gets changed...
this is how u do it in GUI but remember GUI never uses CreateGroup other then on map startup when group vars are declared.

other then that in GUI groups are created when you use pick every unit matching... and if u remember each of this groups hadded to be destroyed that is what u forgot this time ^^


abot event leaks here i go again xD

private constant function NoFilterLeak takes nothing returns boolean
    return true
endfunction

private function Init takes nothing returns nothing
    local trigger t = CreateTrigger()
    local filterfunc ff = Filter(function NoFilterLeak)
    local integer i = 0
    
    loop
    exitwhen (i >= bj_MAX_PLAYER_SLOTS)
        call TriggerRegisterPlayerUnitEvent(t, Player(i), EVENT_PLAYER_UNIT_SPELL_EFFECT, ff)
        set i = i + 1
    endloop
    ... now add cond and act
    call DestroyFilter(ff)
    set ff =null
endfunction

thats all mate. have fun!
~Dark Dragon

 

[Rmx]

Well well well, another spell from the Legendary GUI maker...... to bad it is in vJASS

Anyway downloading ... i can now rate vJASS coz i learned it .. but still now enough to make my own spells ...

 

[Paladon]

I changed a couple of things now, if you could take a look again Dark Dragon, i'll be quite thankful.

 

[Jack_Sparrow93]

Looks great, but its pretty hard to damage with on a moving target.

 

[Dark_Dragon]

yap i checked it and its really awesome spell. great job is MUI, leakless, has nice performances, great idea and very good documented... seriously i like this a lot coz it requres fare more skils in game then most of spell on hive... so once again i really like the spell and think its quite usefull. as well to say spells main data is quickly editable so what do u expect?

ofc 5/5 xD
Greets!
~Dark Dragon

 

[Deaod]

• I would like to change the timer period easily without changing the effects of the spell
• Spell will bug out when using certain values for "RiseTime"
• I'd cache GetLocationZ(Location(d.x, d.y)) into a struct member
• I'd use GroupUtils for groups in general, and I'd use the the boolexpr of the Enum to do things to all affected units
• I would like to adjust the range in which units get affected (hardcoded to 100)
• I'd inline the whole PolarProjection stuff
• Try looking for a library that recycles dummies (i think xe does that) Since you dont even recycle dummies, why not inline the whole thing?
• ImpactDamage should have its own function
• I'd use BoolexprUtils for the dummy filter, or simply use TriggerRegisterAnyUnitEventBJ. Yes, i know it leaks, but c'mon, its not a big deal. Even destroying groups leaks more than that does.
• I think you could save a bit by using radians instead of degrees

Edit:

My current rating would be 2/5 (lacking).

 

[Aspard]

Can't judge on your coding, but that's pretty funny! =)

 

[Kingz]

Paladon what happened to the spell you talked me about?

 

[Paladon]

Still existent.
But i am planning something else with them before i submit them to the public.

 

[hell gate]

ooop! how to change a rating? xD (4 instead of 5)
ok found it ^^

and Paladon: plz don't stop making GUI spells

 

[Palaslayer]

wow, a new spell by you

i rly gotta make a new testmap for you. gonna start today i think. this one looks kinda ugly.

 

[Deaod]

*poke*

any plans on updating?

 

[Paladon]

Sorry, but this has a low priority for me at the moment.

 

[Palaslayer]

i do hope so

 

[Eccho]

I don't get why you guys are telling Paladon to use dynamic creating and destroying, as the risk of messing up a map increases when destroying a handle, especially (those I am aware of), groups, timers and triggers.

Having a global group is perfectly fine, doing "CreateGroup()" does not leak at all as long as he doesn't create a new one in the code. And GroupEnumUnitsBlablabla always clears/overwrites the units inside the group, while not affecting the handle itself...

If you never did "GroupRemoveUnit" inside a FirstOfGroup loop, then, the loop would stay inifnite, right? The use of ClearGroup is not neccessarily making the script better.

I believe I had some more other things in mind which you should think of Paladon, but since I forgot them now, it will have to wait until tonight or this weekend, if there really was anything to think of

 

[Deuterium]

I don't get why you guys are telling Paladon to use dynamic creating and destroying, as the risk of messing up a map increases when destroying a handle, especially (those I am aware of), groups, timers and triggers.

Having a global group is perfectly fine, doing "CreateGroup()" does not leak at all as long as he doesn't create a new one in the code. And GroupEnumUnitsBlablabla always clears/overwrites the units inside the group, while not affecting the handle itself...

Well, dynamic object works is just a slightly more efficient method of doing the script doesn't affect that much I know...
And theoretically we say that "CreateGroup()" does not leak at all, but it is after all one leak, but it just happens once during the initialization of the trigger so it's not that much of a big deal.
I myself prefer using dynamic object works as it remove the leak when the spell is not being used and then re-create the group when needed, but that's not necessary...

 

[Deaod]

Were not short on RAM space, were short on CPU time. Thats the bottleneck, noone cares about a group staying in the RAM for a few hours. Plus, destroying groups leaks so hes better off using a static group. And i will suggest using GroupUtils for that again.

 

[Deuterium]

Were not short on RAM space, were short on CPU time. Thats the bottleneck, noone cares about a group staying in the RAM for a few hours.

Why then remove leaks at all? I'm 100% sure that even if not one leak is removed in a map my 2GB RAM wouldn't fill up even if the game runs for 3 hours... So why do we remove leaks then?

Plus, destroying groups leaks so hes better off using a static group.

How would destroying a group leak?

 

[Deaod]

Because what you described wasnt fixing a leak. It was doing more than necessary to save some RAM.

Groups allocate a few bytes for each unique unit it ever enumed. Now, recycling a group reuses those few allocated bytes, destroying a group leaks them.

 

[redscores]

We clean leaks because we will enable people with even bad pcs to use this spell flawlessly, clear?

 

[Deuterium]

We clean leaks because we will enable people with even bad pcs to use this spell flawlessly, clear?

Thank you redscores for making my point clear

Now, recycling a group reuses those few allocated bytes, destroying a group leaks them.

Well, I don't understand. So you're saying: destroying a group leaks?

 

[Eccho]

I guess, clearing the group first clears the allocated bytes of stored units. Destroying it afterwards would probably due...
But groups are suspicious just like timers and triggers.
Destroying timers and triggers could lead to consequences for your map.

 

[Deuterium]

I guess, clearing the group first clears the allocated bytes of stored units. Destroying it afterwards would probably due...
But groups are suspicious just like timers and triggers.
Destroying timers and triggers could lead to consequences for your map.

Now this makes more sense man they had to make everything bugged and complicated when they made the game... ouff!

 

[Deaod]

nope, clearing a group wont help.

What im about to tell you is my own theory on how groups leak when destroyed:
I believe every unit ever created has its own ID. Now, when you enum units on a fresh group, WC3 allocates a few bytes for every unit enumed. But after that it reuses those few bytes for units being enumed by that group again. If blizzard hadn't screwed up, groups would deallocate those few bytes a group stored for each unit it ever enumed. But as a matter of fact, blizzard screwed up and lets those few bytes stay in the memory after the group was destroyed.
The key to reuse those allocated bytes is recycling groups. We cant stop the leaking completely, but we can reduce it greatly (that is, if you use a static global group).

 

[dansaDisco]

Question

General Review [TD] : [/TD] [tr][td]The spell works?[/td][td]

[/td][/tr][tr][td]The spell does not lag?[/td][td]

[/td][/tr][tr][td]The spell is MUI?[/td][td]

[/td][/tr][tr][td]The Spell is Original?[/td][td]

[/td][/tr][tr][/tr]

Effects	Very nice effects, it's simple, yet it's very effective. No need for changes here IMO.
Movement	I think the size of the beam's damage applier could be a bit bigger, now it's quite small.
Bugs Found	I noticed if the targets die while still having reduced size; the death animation is still on default size.. looks a bit weird. Also, if the spell is cast while they still have the buff, it just deals damage. (as far as I saw)
Other Comments	A very nice spell. I couldn't check the coding though, since I don't understand vJass. But yeah, it has a few minor flaws..
Usefulness	I see some people might have good use for this, the spell fit's a arcane mage nicely. It's a bit challenging to make the spell hit, but that's all well..
Overall Rating	Idea: 5/5 Execution: 4/5 Effects: 5/5 Fun: 4/5

Detailed Review [TD] : [/TD]

 

[Paladon]

Map updated, it's working with the newest patch now.

 

[Mainywc3sear]

its funy =)