wc3campaigns
WC3C Homepage - www.wc3c.netUser Control Panel (Requires Log-In)Engage in discussions with other users and join contests in the WC3C forums!Read one of our many tutorials, ranging in difficulty from beginner to advanced!Show off your artistic talents in the WC3C Gallery!Download quality models, textures, spells (vJASS/JASS), systems, and scripts!Download maps that have passed through our rigorous approval process!

Go Back   Wc3C.net > Resources > Code Resources > vJass Spells
User Name
Password
Register Rules Get Hosted! Chat Pastebin FAQ and Rules Members List Calendar



Reply
 
Thread Tools Search this Thread
Old 04-19-2008, 01:28 AM   #31
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,898

Submissions (37)

Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)

Hero Contest #3 - 2nd Place

Default

It's official anyways. I am not going to make set integervar = structvalue cause a syntax error, only the opposite.

All structs are integers but not all integers are structs, so the typecast IS a good programming practice there.

Quote:
I was only referring to the "IsUnitInRange" call, no other part of that line. That should be fixed to use direct math to see if the unit is in range since the function does it internally anyways but with a square root. Square root is a very computationally heavy function and should always be avoided in distance comparisons.

Actually, you should always use IsUnitInRange over distance whenever possible, even if it is a comparison.

IsUnitInRange probably is slower than comparing the distances^2 directly ... but ... It considers the unit's collision size as well. So it is more accurate.

..
Moy, I am not going to let a map with a O(n) id search into this database, besides, you can easily make it O(log(n)) not to mention O(1)... Gamecache is faster than a O(n) search on groups of more than 30 units.
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Sponsored Links - Login to hide this ad!
Old 04-19-2008, 01:52 AM   #32
Rising_Dusk
Obscurity, the Art


Projects Director
Project Leader: OD
 
Join Date: Feb 2006
Posts: 9,729

Submissions (27)

Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)

Hero Contest #3 - 1st PlaceApproved Map: Desert of ExileApproved Map: Advent of the ZenithHero Contest #2 - 1st PlaceHero Contest - Third place>

Send a message via AIM to Rising_Dusk Send a message via MSN to Rising_Dusk
Default

Quote:
Originally Posted by moyack
This has nothing to do with good practice or bad practice, it works for me the spell works fine (unless you can prove me I'm wrong) then you should worry about that.
2 + 2 = 4
2 + (5*3) - SquareRoot(16) - (3^2) = 4

Both clearly do the same thing, though anyone with any sense would say the first is obviously better than the second considering basic arithmetic. This extends to code as well; sure, your way works, but it sucks compared to the way I mentioned in at least three ways that I also mentioned above.

Vex agrees, too!

Quote:
Originally Posted by moyack
Man.... don't argue that with me, argue it with Vex, probably with the current version of JassHelper there's no problems, but in later version your spells could generate syntax error for that, so just get used to it.
I'll have to deal with it based on his above points anyways, so arguing is worthless. Le sigh.

Quote:
Originally Posted by Vexorian
IsUnitInRange probably is slower than comparing the distances^2 directly ... but ... It considers the unit's collision size as well. So it is more accurate.
Holy fuck, are you serious? It considers collision? Moyack, ignore everything I said, keep it as IsUnitInRange. *Finishes work faster so he can go home and recode his entire map*
__________________
Rising_Dusk is offline   Reply With Quote
Old 04-19-2008, 01:59 AM   #33
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,898

Submissions (37)

Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)

Hero Contest #3 - 2nd Place

Default

Of course I am serious, if there's anyone out there who should know it's me.
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Old 04-19-2008, 02:02 AM   #34
Rising_Dusk
Obscurity, the Art


Projects Director
Project Leader: OD
 
Join Date: Feb 2006
Posts: 9,729

Submissions (27)

Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)

Hero Contest #3 - 1st PlaceApproved Map: Desert of ExileApproved Map: Advent of the ZenithHero Contest #2 - 1st PlaceHero Contest - Third place>

Send a message via AIM to Rising_Dusk Send a message via MSN to Rising_Dusk
Default

You sir, are my hero.
__________________
Rising_Dusk is offline   Reply With Quote
Old 04-19-2008, 02:21 AM   #35
moyack
Evil Emoticon
 
moyack's Avatar


Respected User
Project Leader: PoC
 
Join Date: Jan 2006
Posts: 3,279

Submissions (17)

moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)

AI Tournament #2 - 2nd PlaceHero Contest - Second place

Send a message via MSN to moyack
Default

Quote:
Originally Posted by Vexorian
Moy, I am not going to let a map with a O(n) id search into this database, besides, you can easily make it O(log(n)) not to mention O(1)... Gamecache is faster than a O(n) search on groups of more than 30 units.
Ok, ok, I defend my looping code because it took me 2 days only thing on it, but if you Vex can link me to a code (or example, whatever) where you use a more improved procedure, I will really appreciate, seriously.

What does my looping method, just to ensure you've understood what my spell does:
  • I have in the globals something like this:
    Collapse JASS:
    globals
       private integer index = 0
       private integer array indexes
    endglobals
  • When I create a struct, I do this in the create method:
    Collapse JASS:
    method create takes... returns Data
       local Data D = Data.allocate()
       ...
       set index = index + 1
       set indexes[index] = integer(D)
       return D
    endmethod

    In this way, I ensure that in the indexes array, there are only listed the index of the struct which are active only, not all the struct array
  • In the looping function, I do this:
    Collapse JASS:
    private function Loop takes nothing returns nothing
       local Data D
       local integer i = 0
       loop
          exitwhen i > index // Loops until the array has been checked
          set D = Data( indexes[i] ) // Here, I'm getting only the active structs, not the whole struct array
          ...
          set i = i + 1
       endloop
  • When I destroy the array, I use the RemoveIndex function of Doom, which removes from the indexes array the dead struct and "resize" the array to the right size

I did it in that way because I needed to ensure that the loop only checks the active structs and avoid any destroyed struct in the middle of the struct array.

Tell me how can it be improved and I'll improve this spell and my project accordingly.
moyack is offline   Reply With Quote
Old 04-19-2008, 02:38 AM   #36
Rising_Dusk
Obscurity, the Art


Projects Director
Project Leader: OD
 
Join Date: Feb 2006
Posts: 9,729

Submissions (27)

Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)

Hero Contest #3 - 1st PlaceApproved Map: Desert of ExileApproved Map: Advent of the ZenithHero Contest #2 - 1st PlaceHero Contest - Third place>

Send a message via AIM to Rising_Dusk Send a message via MSN to Rising_Dusk
Default

You do it correctly there, but take a look here...
Collapse JASS:
    private method RemoveIndex takes nothing returns nothing
        local integer i = 1
        loop
            exitwhen indexes[i] == integer(this) or i > index
            set i = i + 1
        endloop
        if i < index then
            loop
                exitwhen i == index
                set indexes[i] = indexes[i+1]
                set i = i + 1
            endloop
        endif
        set index = index - 1
    endmethod
And here...
Collapse JASS:
    static method start takes unit u, real dur returns cooldown
        local cooldown C
        local integer i = 1
        loop
            exitwhen i > Cindex
            set C = cooldown( Cindexes[i] )
            exitwhen C.u == u
            set i = i + 1
        endloop
        if i > Cindex then
            set C = cooldown.allocate()
            set C.u = u
            set Cindex = Cindex + 1
            set Cindexes[Cindex] = integer(C)
        endif
        set C.dur = dur
        set C.time = 0.
        return C
    endmethod
And here! >_<
Collapse JASS:
    static method IsInCooldown takes unit u returns boolean
        local integer i = 1
        local cooldown C
        loop
            exitwhen i > Cindex
            set C = cooldown( Cindexes[i] )
            exitwhen C.u == u
            set i = i + 1
        endloop
        return i <= Cindex and C.time < C.dur
    endmethod
Those functions are where the O(n) searches exist and need to be eliminated. These searches are slow, functionally inferior to proper indexing techniques (I hate referencing my own spell, but shrapnade's a really good example of the indexing technique I refer to. You even quasi-use it in some areas of the spell, but you just don't in others, which is bad.

You also make it difficult to follow because your variable names are all "index" or "indexes" or "Cindex" and nothing is named in a manner that I can tell what it is. If you named your variables more logically, I'd be able to write the fixed version for you. I can help you out with the conversion if you'd like.

And why in the world do you index the cooldowns and the spears separately? Why are they their own structs? Can you not merge them together and do it all in one fell swoop? That added a lot to the challenge of reading the code, I couldn't figure out why you had so much going on at once!

Quote:
Originally Posted by moyack
Ok, ok, I defend my looping code because it took me 2 days only thing on it, but if you Vex can link me to a code (or example, whatever) where you use a more improved procedure, I will really appreciate, seriously.
Everyone always listens to him when he says it, but whenever I say it people get angry and argue with me. :'(
__________________
Rising_Dusk is offline   Reply With Quote
Old 04-21-2008, 04:41 AM   #37
Pyrogasm
Lackadaisically Absent.
 
Pyrogasm's Avatar


Respected User
 
Join Date: Sep 2006
Posts: 4,523

Submissions (9)

Pyrogasm is a splendid one to behold (638)Pyrogasm is a splendid one to behold (638)Pyrogasm is a splendid one to behold (638)Pyrogasm is a splendid one to behold (638)Pyrogasm is a splendid one to behold (638)Pyrogasm is a splendid one to behold (638)Pyrogasm is a splendid one to behold (638)

Hero Contest - Fourth place

Send a message via ICQ to Pyrogasm Send a message via AIM to Pyrogasm Send a message via MSN to Pyrogasm Send a message via Yahoo to Pyrogasm
Default

Goddamnit; now I've got to go fix all my spells to take collision size into account, too.
__________________
Quote:
Originally posted by Rising_Dusk
Your spells are mostly ignored because they are not very cool so we aren't very excited to review/approve them, but you are incredibly persistent and won't give us an excuse to graveyard it. That is generally what results in a resource being ignored for a long time.

The Spell Request Thread Done for, unless someone else wants to revive it...
It lasted a damn long time.

Please; Ask for Help Appropriately














Quote:
Originally posted by Kyrbi0
Huh. Almost makes me wish I had a girlfriend, to take advantage of today (wait, no, that's not what I meant... I mean, take advantage of the fact that it is international women's day... gah, never mind).
Quote:
Originally posted by Pyrogasm
Rome may not have been built in a day, but the Romans sure as hell didn't say "look at this great city we built guys!" when they had nothing more than a bit of stone and some cottages.
Pyrogasm is offline   Reply With Quote
Old 04-28-2008, 02:09 AM   #38
moyack
Evil Emoticon
 
moyack's Avatar


Respected User
Project Leader: PoC
 
Join Date: Jan 2006
Posts: 3,279

Submissions (17)

moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)

AI Tournament #2 - 2nd PlaceHero Contest - Second place

Send a message via MSN to moyack
Default

Updated 1.2. Now it requires JNGP 5A to work properly.
moyack is offline   Reply With Quote
Old 04-28-2008, 03:45 AM   #39
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,898

Submissions (37)

Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)

Hero Contest #3 - 2nd Place

Default

Your cooldown remove function still has this O(n) search which does not look good to me.

Did you notice that you only call cooldown.destroy() in one place of the script and that such part of your script already knows the index of the object in the cooldown list? This means you can do the index removal in O(1) and very easily.

If it wasn't for that, you could still do it, don't you think? Just store the index in the cooldown struct instance.
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Old 05-06-2008, 09:33 PM   #40
Rising_Dusk
Obscurity, the Art


Projects Director
Project Leader: OD
 
Join Date: Feb 2006
Posts: 9,729

Submissions (27)

Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)Rising_Dusk has a reputation beyond repute (1192)

Hero Contest #3 - 1st PlaceApproved Map: Desert of ExileApproved Map: Advent of the ZenithHero Contest #2 - 1st PlaceHero Contest - Third place>

Send a message via AIM to Rising_Dusk Send a message via MSN to Rising_Dusk
Default

*Poke* Any chance of an update per Vex's most recent suggestions, Moy?
__________________
Rising_Dusk is offline   Reply With Quote
Old 05-06-2008, 09:37 PM   #41
moyack
Evil Emoticon
 
moyack's Avatar


Respected User
Project Leader: PoC
 
Join Date: Jan 2006
Posts: 3,279

Submissions (17)

moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)

AI Tournament #2 - 2nd PlaceHero Contest - Second place

Send a message via MSN to moyack
Default

Hero contest 2 and real life (taxes forms and such) is my priority right now, but when I finish them, I'll give man love to this spell, or I'll kill myself with a banana.

Meanwhile, I'll put in my PoC section...
moyack is offline   Reply With Quote
Old 05-07-2008, 03:43 AM   #42
moyack
Evil Emoticon
 
moyack's Avatar


Respected User
Project Leader: PoC
 
Join Date: Jan 2006
Posts: 3,279

Submissions (17)

moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)

AI Tournament #2 - 2nd PlaceHero Contest - Second place

Send a message via MSN to moyack
Default

Bump!!! version 1.3... no more search when the data is destroyed. Tell me what do you think now.

Last edited by moyack : 05-07-2008 at 12:50 PM.
moyack is offline   Reply With Quote
Old 05-07-2008, 11:52 PM   #43
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,898

Submissions (37)

Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)

Hero Contest #3 - 2nd Place

Default

Collapse JASS:
private function Actions takes nothing returns boolean
    if GetUnitAbilityLevel(GetAttacker(), BuffID) > 0 and GetRandomReal(0., 1.) < Chance(GetUnitAbilityLevel(GetAttacker(), SpellID)) and GetUnitState(GetAttacker(), UNIT_STATE_MANA) >= Mana(GetUnitAbilityLevel(GetAttacker(), SpellID)) and not cooldown.IsInCooldown(GetAttacker()) then
        call cooldown.start(GetAttacker(), Cooldwn)
        call Data.create(GetAttacker(), GetTriggerUnit())
        call SetUnitState(GetAttacker(), UNIT_STATE_MANA, GetUnitState(GetAttacker(), UNIT_STATE_MANA) - Mana(GetUnitAbilityLevel(GetAttacker(), SpellID)))
    endif
    return false
endfunction
Your code would be happier, it would also be more efficient and readable if you used a local for GetAttacker.

That's not required for approval, I guess the code is fine now. So...
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Old 06-15-2008, 03:08 AM   #44
The Grey Knight
User
 
Join Date: May 2008
Posts: 150

The Grey Knight has little to show at this moment (1)

Default

I tried out the spell, and i liked, so i put it in my map. i didnt bother to paste the "berserk" ability, but i did add everything else. i changed the spell id's etc to correspond with the trigger, and i added it to a melee hero. i tested the map, and it didnt work. i tried it on a ranged hero, and it also didnt work. help plz?
__________________
._.
The Grey Knight is offline   Reply With Quote
Old 06-17-2008, 11:23 PM   #45
moyack
Evil Emoticon
 
moyack's Avatar


Respected User
Project Leader: PoC
 
Join Date: Jan 2006
Posts: 3,279

Submissions (17)

moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)moyack is a splendid one to behold (666)

AI Tournament #2 - 2nd PlaceHero Contest - Second place

Send a message via MSN to moyack
Default

Quote:
Originally Posted by The Grey Knight
I tried out the spell, and i liked, so i put it in my map. i didnt bother to paste the "berserk" ability, but i did add everything else. i changed the spell id's etc to correspond with the trigger, and i added it to a melee hero. i tested the map, and it didnt work. i tried it on a ranged hero, and it also didnt work. help plz?
Ohh sorry, I've been busy with other stuff and just until now I saw your post :P

It doesn't work in which way?? it doesn't show the missile?? it doesn't deal damage properly?? Please indicate me how you set in the Trigger Editor and in the Obejct editor the spell, so I can have a clue of your issue.
moyack is offline   Reply With Quote
Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off


All times are GMT. The time now is 03:04 AM.


Affiliates
The Hubb The JASS Vault Clan WEnW Campaign Creations Clan CBS GamesModding Flixreel Videos

Powered by vBulletin (Copyright ©2000 - 2019, Jelsoft Enterprises Ltd).
Hosted by www.OICcam.com
IT Support and Services provided by Executive IT Services