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 08-24-2007, 03:59 PM   #1
Fireeye
Chos
 
Fireeye's Avatar
 
Join Date: Jun 2006
Posts: 495

Submissions (2)

Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)

Default Thunder Whirl

Thunder Whirl v.1.3d

Zoom (requires log in)

Requirement
Jass New Gen Pack v.4b (recommend)

Credits
Vexorian - JassHelper
PipeDream - Grimoire
PitzerMike - PJass

I made this Spell for Spell Making Session #9, well i didn't really continue the spell only some smaller things changed, like moving th entire spell into a scope + the usage of private/public functions and adding the option, that knocked units can destroy destructibles.
However i decided to finish this spell so any CONSTRUCTIVE criticism is welcome.
I hope i didn't forgot anything yet (well, it's my very first submit)

Thunder Whirl is a spell, which grabs an enemy unit and use it as shield in a certain distance, depending on the level.
When the target unit hits another unit both are damaged and the other unit will be knocked away from the hero.
When the cast is canceled/finished the target unit will be released sliding a bit too.

Ability Type: Channeling Spell
Mana Cost: 75 + 10 * Level
Duration: 20 seconds
Cooldown: 30 seconds
Damage: 7 + 2 * Level per hit

Expand Readme:

Expand Spell Code:

---Update v.1.2 ---
fixed/added all things mentioned by Alexander244, hadn't much time yet to test everything, but it should work without any problem.
---Update v.1.3 ---
sorry for the long waiting for an update, were pretty busied, however i hope everything is fine now.
---Update v1.3b ---
Fixed a more or less serious thing in the callback function.
---Update v1.3c ---
Some code lines were removed, also i made the scope change suggested by moyack
---Update v1.3d ---
Hopefully the last update, made the structs private, added the option to set the size of the destructable rect and change some smaller parts.
Attached Images
File Type: jpg WC3ScrnShot_070907_112041_02.jpg (58.9 KB, 977 views)
Attached Files
File Type: w3x ThunderWhirl.w3x (51.6 KB, 303 views)
__________________

Last edited by Fireeye : 11-28-2007 at 05:33 PM.
Fireeye is offline   Reply With Quote
Sponsored Links - Login to hide this ad!
Old 08-24-2007, 04:17 PM   #2
WILL THE ALMIGHTY
. . .
 
WILL THE ALMIGHTY's Avatar
 
Join Date: Oct 2006
Posts: 1,529

Submissions (21)

WILL THE ALMIGHTY is just really nice (284)WILL THE ALMIGHTY is just really nice (284)

Default

Ooh... Looks nice.
__________________
Quote:
[21-10-00] WILLTHEALMIGHTY: Ok I have this weird sound upstairs... BRB gotta go check it out.
[21-10-09] Ash: WILLTHEALMIGHTY, WAIT
[21-10-16] Ash: WILLTHEALMIGHTY, IT IS NOT SAFE OUT THERE, TAKE THIS!
[21-10-16] Ash gives WILL a kitten.
WILL THE ALMIGHTY is offline   Reply With Quote
Old 08-24-2007, 04:56 PM   #3
cohadar
master of fugue
 
cohadar's Avatar
 
Join Date: Jun 2007
Posts: 2,453

Submissions (5)

cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)cohadar is just really nice (250)

Default

Well the code looks great,
I only don't like this:
Collapse JASS:
        set tw_destroytrees = Rect(256.0 , 2400.0 , 448.0 , 2656.0)

hardcoded things are BAD.
either turn this into constants or put a good comment above that line
to explain wtf is that.

Besides from that spell could use a main comment header describing in
general terms how it works.

Good coding is not enough, you have to document it so people would understand it.

And people tend to forget even what their own code was doing after a while.

Comments, comments, comments...

... more comments.
__________________
Omg database crash deleted my signature, as a side effect this immensely improved wc3c.
cohadar is offline   Reply With Quote
Old 08-24-2007, 06:49 PM   #4
Alexander244
Now Kharyb
 
Join Date: Feb 2006
Posts: 602

Submissions (1)

Alexander244 is a jewel in the rough (193)Alexander244 is a jewel in the rough (193)Alexander244 is a jewel in the rough (193)

Default

I liked it a lot in the spell session, and it's even better now :)

Initial feedback:
  1. The unit being spun does not destroy trees, while the units being knocked back do. It would be better if they both did.

  2. Units get knocked back away from the caster, instead of in the direction the unit being spun is moving. As the spell is magical, I don't think this is a big issue. If someone wanted to make a more realistic spell based on this, however, it would look a bit odd.

  3. When a unit finishes being knocked back they will just stand there unless there are other units being damaged - this could be solved by doing 0 damage to them when they finish being knocked back, so they run back into the action.

  4. The attack type and damage type should be configurable.

  5. You should say the spell requires units user data, because this can make it incompatible with some other systems/spells.

    Edit: Minor stuff

  6. You can change your three formula functions from private function to private constant function

  7. Indent your globals

Last edited by Alexander244 : 08-24-2007 at 06:54 PM.
Alexander244 is offline   Reply With Quote
Old 08-24-2007, 07:22 PM   #5
Fireeye
Chos
 
Fireeye's Avatar
 
Join Date: Jun 2006
Posts: 495

Submissions (2)

Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)

Default

1. ok, i'll add the option to configure both seperat
2. I'll add a option for that too
3. Ok, thanks will be fixed next version
4. Not a big deal will be added
5. Ok, i'll write it in the Read Me
6. Ok, no big problem either
7. done

I'll do everything tomorrow and update the first post then.
__________________
Fireeye is offline   Reply With Quote
Old 08-25-2007, 02:24 PM   #6
Alexander244
Now Kharyb
 
Join Date: Feb 2006
Posts: 602

Submissions (1)

Alexander244 is a jewel in the rough (193)Alexander244 is a jewel in the rough (193)Alexander244 is a jewel in the rough (193)

Default

A few more things;
  1. As long as you don't prefix the InitTrig with public/private, you can keep it inside the scope. Then you can have everything it calls as a private (makes the code a bit neater)

  2. Work in radians - in tight loops you want as few calculations as possible

  3. In the Cast_Actions function you use GetUnitX/Y when you already have locals for the target and casters co-oridinates.
    Collapse JASS:
        local real cx = GetUnitX(caster)
        local real cy = GetUnitY(caster)
        local real tx = GetUnitX(target)
        local real ty = GetUnitY(target)
    //...
        set twdat.l = AddLightning(LightningId,true,GetUnitX(caster),GetUnitY(caster),GetUnitX(target),GetUnitY(target))
    //...
        set twdat.angle = bj_RADTODEG*Atan2(GetUnitY(target)-GetUnitY(caster),GetUnitX(target)-GetUnitX(caster))
    


  4. Inline your Knockback Terrain Check (in KBActions):
    Collapse JASS:
        call SetItemPosition(tw_terraincheck,cx,cy)
        if GetItemX(tw_terraincheck)==cx and GetItemY(tw_terraincheck)==cy then
            call SetUnitX(u,cx)
            call SetUnitY(u,cy)
        endif


    Edit:

  5. You need to have some form of wait between unpausing the unit and making the caster damage it, or the unit will not be ordered to attack the caster.

Last edited by Alexander244 : 08-25-2007 at 03:02 PM.
Alexander244 is offline   Reply With Quote
Old 08-26-2007, 11:57 AM   #7
Fireeye
Chos
 
Fireeye's Avatar
 
Join Date: Jun 2006
Posts: 495

Submissions (2)

Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)

Default

1. i know, but it won't do it due it doesn't have any affect of the performance
2. i gonna change that
3. omg, thank you very much, i really hate it when such a thing happens
4. ok, will be inlined
5. fixed it with an attack command instead of doing damage, no nasty work around required then

Don't know if i'll update the spell today, maybe tomorrow.

Offtopic:
+rep for reviewing and suggestions
__________________
Fireeye is offline   Reply With Quote
Old 08-27-2007, 12:09 PM   #8
blu_da_noob
Nonchalant
 
blu_da_noob's Avatar


Respected User
 
Join Date: Mar 2006
Posts: 1,933

Submissions (2)

blu_da_noob is just really nice (398)blu_da_noob is just really nice (398)blu_da_noob is just really nice (398)blu_da_noob is just really nice (398)blu_da_noob is just really nice (398)blu_da_noob is just really nice (398)

[Quicksilver #2] - 2nd Place[Quicksilver#1] 1st place

Send a message via MSN to blu_da_noob
Default

Haven't read the other posts so I may repeat others' points.
  • Incorrect executefunc usage (when using scopes for spell distribution you must use SCOPE_PREFIX instead of naming it statically).
  • Not strictly a problem, but as a development note: private-ing your globals removes the need to prefix them to make them unique (so the 'tw_' 's aren't really necessary).
  • To make it easier to customise, I recomend moving all constant spell data variables (along with the functions) to the top in a clearly indicated configuration section. Just so you don't have people changing some important stuff and screwing up the spell. On that note you should explain what each one does a bit better; "//The increament/decreament of the speed" isn't very helpful.
  • In function Cast_Actions you create variables for caster and target coordinates and then neglect to use them in a couple of instances.
  • Replace bj_DEGTORAD/RADTODEG with literal constants
  • May want to make tree destruction area configurable
  • You need to at the very least make a huge note saying that you use UnitUserData for knockbacks. It can conflict with other people's code.
  • function CreateKB has useless radian->degree->radian conversions
  • Pausing units that are knocked back isn't always what people want. Also, you pause the units and remove their pathing on every iteration of knockback. Just do it when it starts.
  • Your terrain checking function is bad, other items will get in the way and terrain may be pathable but the item won't be exactly on the spot. Account for these.
  • It is good practice to initialise all struct members to null.
__________________

Last edited by blu_da_noob : 08-27-2007 at 12:10 PM.
blu_da_noob is offline   Reply With Quote
Old 09-04-2007, 01:34 AM   #9
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

Other comments:
  • Man!! you have a bunch of constant values... that's not a problem, but it would be a good idea to make some classification with comment lines, so people can find easily where to modify the data. I suggest classify in sound section, constants used by cast struct and knockback struct.

  • It's not very versatile to have predefined the order ID, it would be better to avoid that private constant and directly get the order with GetUnitCurrentOrder(caster).

  • In the InitTrig function, you make an ExecuteFunc("ThunderWhirl_preload"). Why not just do call ThunderWhirl_preload()??

Last edited by moyack : 09-04-2007 at 01:34 AM.
moyack is offline   Reply With Quote
Old 09-25-2007, 01:02 AM   #10
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

Nearly three weeks after the fact... Will this be seeing an update in the near future? If not, I fear I may be toasting it.
__________________
Rising_Dusk is offline   Reply With Quote
Old 09-30-2007, 06:49 AM   #11
Malf
I LIKE PIZZA! | >
 
Malf's Avatar
 
Join Date: Sep 2007
Posts: 625

Submissions (2)

Malf has a spectacular aura about (92)Malf has a spectacular aura about (92)Malf has a spectacular aura about (92)Malf has a spectacular aura about (92)

Send a message via AIM to Malf
Default

Oh it looks good, I'm gonna try it :D

Btw, Mr.Dusk, when's the next spell session? I've browsed previous spell sessions before and they were great.
Malf is offline   Reply With Quote
Old 10-01-2007, 08:35 PM   #12
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:
Btw, Mr.Dusk, when's the next spell session? I've browsed previous spell sessions before and they were great.
Please, just Dusk. :p
And whenever I can think up a decent theme for it and I sort out the results of the last one. I may end up just forfeiting my win so that people don't think I cheated or hacked or some shit. See my signature for more information, I covered most of it there.

And just so other mods know -- He updated this and didn't post to bump it. (He actually PMed me that he did) So don't graveyard this until it gets at least a few more rounds of review.
__________________
Rising_Dusk is offline   Reply With Quote
Old 10-09-2007, 01:05 AM   #13
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

@Fireeye: Please post when you have a new version or improvement please, Rising_Dusk is not the only moderator here.

I've checked Fireeye's spell, and here's what I've found thus far:
  • you should make the scope to include the Inittrig function, with that, you can put the code in the Preload public function into the inittrig function, and, at the same time, remove those prefixes.
  • In the Cast_Actions function, you have this line of code set glob[Integer] = twdat. It must be changed to set glob[Integer] = integer(twdat) because it will generate incompatibilities with JassHelper greater than 0.9.8.8.
  • You can remove the debug message line in the orderid detection.

Last edited by moyack : 10-09-2007 at 01:08 AM.
moyack is offline   Reply With Quote
Old 10-09-2007, 12:47 PM   #14
Fireeye
Chos
 
Fireeye's Avatar
 
Join Date: Jun 2006
Posts: 495

Submissions (2)

Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)Fireeye has a spectacular aura about (113)

Default

Ok, i gonna post when i'm updating the spell.
However i found a serious but dumb mistake in my own Spell...
These 2 lines cause a problem, which will let the target unit orbit TOO fast around the caster, don't know why i wrote there a RADTODEG, i already fixed it (Removing the *57.29577 part).
Collapse JASS:
set x2 = x1+twdat.cdistance*Cos(twdat.angle*57.29577)
set y2 = y1+twdat.cdistance*Sin(twdat.angle*57.29577)
@moyack:
Do you mean using something like this?
Collapse JASS:
    public function Init takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CHANNEL )
        call TriggerAddCondition( t, Condition( function Cast_Conditions ) )
        call TriggerAddAction( t, function Cast_Actions )
        call Preload(LightningId)
        call Preload(CasterEffect)
        call Preload(HitEffect)
        call Preload(HitEffectAdd)
        call Preload(SlideEffect)
        set Bex = Condition(function KBFilter)
        set TreeRect = Rect(256.0 , 2400.0 , 448.0 , 2656.0)
        set TerrainCheck = CreateItem(TerrainItem,0.00,0.00)
        call SetItemVisible(TerrainCheck,false)
    endfunction
The other 2 things are already done, well i never got any problem with
set glob[Integer] = twdat
but i'm too lazy to make test right now so i changed it
Any other constructive critiques are welcome.
__________________

Last edited by Fireeye : 10-09-2007 at 12:55 PM.
Fireeye is offline   Reply With Quote
Old 10-10-2007, 05:46 PM   #15
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 Fireeye
@moyack:
Do you mean using something like this?
Collapse JASS:
    public function Init takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterAnyUnitEventBJ( t, EVENT_PLAYER_UNIT_SPELL_CHANNEL )
        call TriggerAddCondition( t, Condition( function Cast_Conditions ) )
        call TriggerAddAction( t, function Cast_Actions )
        call Preload(LightningId)
        call Preload(CasterEffect)
        call Preload(HitEffect)
        call Preload(HitEffectAdd)
        call Preload(SlideEffect)
        set Bex = Condition(function KBFilter)
        set TreeRect = Rect(256.0 , 2400.0 , 448.0 , 2656.0)
        set TerrainCheck = CreateItem(TerrainItem,0.00,0.00)
        call SetItemVisible(TerrainCheck,false)
    endfunction
The other 2 things are already done, well i never got any problem with
set glob[Integer] = twdat
but i'm too lazy to make test right now so i changed it
Any other constructive critiques are welcome.

I mean this:
Expand JASS:

About this line of code set glob[Integer] = integer(twdat), it is mandatory in order to guarantee compatibility with newer versions of JassHelper, because this will generate a serious syntax error.

EDIT: Here's the thread related with the integer() stuff: http://www.wc3campaigns.net/showthread.php?t=96463

Last edited by moyack : 10-10-2007 at 05:51 PM.
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 11:51 PM.


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