[vJASS]Rupture [v1.2f] by baassee

This bundle is marked as approved. It works and satisfies the submission rules.
*Requirements For The Spell*
*xebasic by Vexorian
*xedamage by Vexorian
*T32 by Jesus4Lyf
*JASSHelper version 0.A.2.B or later by Vexorian
*Optional*
if the NOSTACK constant is set to true, this will be needed, else not.
*SimError by Vexorian

Links:
xe 0.8
T32
JASSHelper
SimError

Rupture, bloodseekers ultimate from DOTA. I have no idea if it works as the DOTA one but my version is how I thought it worked. I wont change it in anyway.

Description:

Deals a mighty blow to the enemy causing any movement to result in bleeding and loss of life.

Level 1 - 150 initial damage. Deals 20% of the distance moved as damage.
Level 2 - 250 initial damage. Deals 40% of the distance moved as damage.
Level 3 - 350 initial damage. Deals 60% of the distance moved as damage.

DO NOT COMPLAIN ABOUT ITS UNORIGINALITY, IT DIDNT EXIST IN THW SPELL SECTION SO THATS WHY I CREATED IT! ENJOY!

More information about the spell and how to import is inside the code below

JASS:
scope Rupture // requires xebasic, xedamage, T32 optional SimError
//
//
//      *Rupture*
//          by baassee
//
//      *Requirements*
//          *xebasic by Vexorian
//          *xedamage by Vexorian
//          *T32 by Jesus4Lyf
//          *JASSHelper version 0.A.2.B or later by Vexorian
//      *Optional*
//          if the NOSTACK constant is set to true, this will be needed, else not.
//          *SimError by Vexorian
//
//          http://www.wc3c.net/showthread.php?t=101150 - xe 0.8
//          http://www.thehelper.net/forums/showthread.php/132538-Timer32 - T32
//          http://www.wc3c.net/showthread.php?t=88142  - JASSHelper
//          http://www.wc3c.net/showthread.php?t=101260 - SimError
//
//          *WARNING AND NOTICE!*
//              This map uses a renamed dummy model by Vexorian
//              please download the proper one or just export this
//              one, if you already have xe implented then you
//              wont have to worry about this.
//
//              Also the spell uses the dummy id of the xedummy so
//              remember to correct it!
//
//      *Changelog*
//          v1.0 - Released
//          v1.1 - -Fixed spelling errors Rupture instead of Rapture.
//                 -Fixed preloading.
//                 -Added a struct member lvl to the struct.
//                 -Fixed bug with adding unit to group when nostack == false.
//                 -Changed ApplyLife with RemoveUnit
//                 -.count is initially 0.
//                 -Removed creating effect and added it on damage with the xe.
//          v1.2 - -Removed the showunit command
//                 -Now the trigger doesn't initialize 2 triggers but creates one more with NOSTACK
//                 -Added another check to check if the unit is dead according to TH
//                 -Added locals to the conditions according to TH
//          v1.2b  -Fixed nulling -.-
//                 -Removed a action.
//          v1.2c  -Fixed an unnecessary function call
//                 -Fixed Error message.
//                 -Now when error message shows up the caster can choose a new target.
//          v1.2d  -Removed a static if.
//          v1.2e  -Fixed another static if with the globals. nostack is now set at the init.
//          v1.2f  -Made the spell run with T32. Fixed a few lines.
//
//
//      *Standard Information*
//          This spell was totally inspired by the spell Rupture in DOTA.
//          It probably dont function the same way as in DOTA but this is
//          my own version of it. I made it as the tooltip says at
//          http://www.playdota.com/heroes/bloodseeker/
//          
//          Please don't beg me to do other DOTA spells and don't complain
//          about its not-originality. I didn't find any Rapture at THW when
//          searching so I thought it might have been useful to someone.
//
//          Implent it as the how to import tutorial says and enjoy the spell!
//          Please give proper credit to Vexorian for the libraries and me for
//          this spell.
//
//          Yes I use GetWidgetLife() instead of the JASSHelper native UnitAlive
//          because UnitAlive bugs the optimizer by the same author so to prevent
//          that bug I use GetWidgetLife(), also it's faster.
//
//
//      *How to import*
//
//          1. Copy the dummy unit or just copy the one from the xe with the model included!
//
//          2. Copy the abilities, Rupture and Rupture Buff.
//
//          3. Copy the buff Rupture. Remember to set the buff of the spell Rupture Buff to
//             this one.
//
//          4. Make sure you have xebasic and damage in your map with proper dummy
//             and dummy model. Also import T32 or just copy the T32 Trigger.
//
//          5. Copy over this trigger called Rupture.
//
//          6. Follow the setup part which will explain all values and correct the ability
//             ids and the buff id.
//     
//          7. Add the spell to a unit, credit proper authors and enjoy!
//
//
//      *SETUP PART*
//
//

    globals
//
//      The ability ID of the main spell. Select it in the object editor, press
//      ctrl + d and you can see the ID.
//
        private constant integer ABILID         = 'A000'
//
//      The ability id of the buff adding spell. How to get the id, check above.
//
        private constant integer DUMMYID        = 'A001'
//
//      The buff id. Do as with the main ABILID but in the buff section.
//
        private constant integer BUFFID         = 'B000'
//
//      The blood effect when a unit takes damage.
//
        private constant string BLOODFX         = "Objects\\Spawnmodels\\Human\\HumanBlood\\HumanBloodPeasant.mdl"
//
//      The attachment point where the blood fx will be attached too.
//      I would recommend chest, hands or head. Leave it as it is, no
//      need to change.
//
        private constant string ATTACH          = "chest"
//
//      If you want to preload the blood fx and the ability to remove
//      first cast lagg set this to true. You might suffer a longer
//      loading time if you do but it will prevent first cast lagg as
//      said. I would recommend it set to true.
//
        private constant boolean PRELOAD        = true
//
//      The error message that shows up with NOSTACK (below) is true
//
        private constant string MESSAGE         = "Target Unit has already Rupture"
//
//      The hotkey of the spell
//
        private constant string HOTKEY          = "R"
//      
//      If the damage should be 100% pure then set this to true
//      else if you want to make your own damage stuff, set it to false.
//
        
        private constant boolean PURE           = true
//
//      Do you want the ability to stack at the same unit?
//      This can bug the spell if set to false. Use at your own risk
//      The bug it can make is that the buff wont show but it will damage
//      proper.
//
        
        private constant boolean NOSTACK        = true
        
    endglobals
//
//      *FUNCTIONS* (Part of the setup part)
//
//
//      The duration function, takes the level of the spell.
//      The buff will last as long as the duration. Also
//      the movement damage.
//
    private function dur takes integer lvl returns real
        return 3. + 2. * lvl
    endfunction
//
//      The initial hit damage. Set as you want.
//
    
    private function fdmg takes integer lvl returns real
        return 50. + 100. * lvl
    endfunction
//
//      The movement damage in percent. As it is now it will
//      damage 20% * the level of the ability * the movement
//      during the period in damage.
//
    
    private function mdmg takes integer lvl returns real
        return 0.2 * lvl
    endfunction
//
//      The max damage function. Functions with the interval
//      function below the max damage function.
//
    
    private function maxdmg takes integer lvl returns real
        return 200. + 0 * lvl
    endfunction
//
//      The interval for the maximum damage. It will only
//      be possible to make above damage during the interval.
//      If the unit moves so much that it will take more damage,
//      this will prevent it. Refreshes every set amount.
//
    
    private function maxdmginterval takes integer lvl returns real
        return 0.25 + 0 * lvl
    endfunction
//
//      The damage options, they're explained inside the function.
//      This function is useless if you have set the PURE constant
//      to true. Use the function below this one for the PURE constant
//      true stuff.
//

    private function xeoptions takes xedamage d returns nothing
        // the damage type of the spell
        // this wont be affected if forcedamage
        // is set to true.
        set d.dtype         = DAMAGE_TYPE_UNIVERSAL
        // the attack type of the spell
        // this wont be affected if forcedamage
        // is set to true.
        set d.atype         = ATTACK_TYPE_CHAOS
        //
        // the weapon type the spell will use, affilcts the
        // sound when damaged, in this case, no sound
        // when damaged but I made it configurable anyway
        set d.wtype         = WEAPON_TYPE_WHOKNOWS
        // if the damage should be ranged,
        // works different when AI's hit,
        // usally makes the unit move as
        // it got attacked. Perfect for
        // Rapture.
        set d.ranged        = true
        // if the spell should damage enemies
        // quite obvious
        set d.damageEnemies = true
        // if the spell should damage neutrals if
        // they are targeted.
        set d.damageNeutral = true
        // this one below is quite special
        // I use the feature of xedamage to
        // try to make the pure damage as close
        // as possible to the amount.
        // if set to true, it will be pure
        // damage, if false then the damage
        // type and attack type will matter
        // if this is set to false and
        // damage type to universal and
        // attack type to chaos, it will act
        // as pure damage, if you havent mixed
        // with the gameplay constants.
        set d.forceDamage   = true
        
        call d.useSpecialEffect(BLOODFX, ATTACH)
    endfunction
//
//      The second xedamage options, just basic the same as above so
//      wont explain them further but these are used when the PURE
//      constant is set to true.
//
    private function xepureoptions takes xedamage d returns nothing
        set d.wtype         = WEAPON_TYPE_WHOKNOWS
        set d.ranged        = true
        set d.damageEnemies = true
        set d.damageNeutral = true
        call d.useSpecialEffect(BLOODFX, ATTACH)
    endfunction
    
//
//      *END OF SETUP*
//
    
    globals
        private xedamage xed // the xedamage instance
        private group nostack //nostack group, only declared
    endglobals

    private struct Rupture // the struct
        
        unit    c //casting unit
        unit    u //target unit
        integer lvl
        real    dmg//the movement percent
        real    maxdmg//the max damage per interval
        real    check//the interval check
        real    count = 0.//counter to count the interval
        real    X//current x of the target
        real    Y//current y of the target
        real    dur//duration of the spell
        //
        // creation method takes unit caster, unit target, real x of target, real y of target
        //
        static method create takes unit c, unit u, real x, real y returns Rupture
            local Rupture this = Rupture.allocate()
            set .c = c
            set .u = u
            set .lvl = GetUnitAbilityLevel(c, ABILID)
            set .dmg = mdmg(.lvl)
            set .maxdmg = maxdmg(.lvl)
            set .check = maxdmginterval(.lvl)
            set .X = x
            set .Y = y
            set .dur = dur(.lvl)
            static if PURE then
                call xed.damageTargetForceValue(c, u, fdmg(.lvl))
            else
                call xed.damageTarget(c, u, fdmg(.lvl))
            endif
            call UnitAddAbility(u, DUMMYID)//adds the buff spell
            call UnitMakeAbilityPermanent(u, true, DUMMYID)
            static if NOSTACK then
                call GroupAddUnit(nostack, u)
            endif
            call .startPeriodic()
            return this
        endmethod
        //
        // the method loop
        //
        private method periodic takes nothing returns nothing
            local real dx = GetUnitX(.u) - .X //these are distance stuff
            local real dy = GetUnitY(.u) - .Y //to see if the unit has moved
            local real sq = SquareRoot(dx * dx + dy * dy) //distance function
            if .dur > 0. and GetWidgetLife(.u) > 0.405 and not IsUnitType(.u, UNIT_TYPE_DEAD)then //check if the spell has ended
            //or the target is already dead
                set .dur = .dur - T32_PERIOD //reduce the duration
                if sq > 0. then //check if the unit has moved any distance
                    //next if is to check if the interval has been reached and
                    //the max damage hasn't been reached and also that the
                    //damage that is about to happen isn't greater than the maxdmg
                    if .count < .check and .maxdmg > 0. and (sq * .dmg) <= .maxdmg then
                        // we do the damage
                        static if PURE then
                            call xed.damageTargetForceValue(.c, .u, (sq * .dmg))
                        else
                            call xed.damageTarget(.c, .u, (sq * .dmg))
                        endif
                        set .maxdmg = .maxdmg - (sq * .dmg)
                        set .count = .count + T32_PERIOD
                        set .X = GetUnitX(.u)//set the new coordinates of the unit
                        set .Y = GetUnitY(.u)//same as above
                        if .count >= .check then
                            set .count = 0.
                            set .maxdmg = maxdmg(.lvl)
                        endif
                    endif
                endif
            else
                //instance have ended then we clear leaks and removes the ability and also the buff
                call .stopPeriodic()
                call UnitMakeAbilityPermanent(.u, false, DUMMYID)
                call UnitRemoveAbility(.u, DUMMYID)
                call UnitRemoveAbility(.u, BUFFID)
                static if NOSTACK then
                    call GroupRemoveUnit(nostack, .u)
                endif
                call .destroy()
            endif
        endmethod
    
        static method Conditions takes nothing returns boolean
            local real x
            local real y
            local unit c
            local unit u
            if GetSpellAbilityId() == ABILID then
                set c = GetTriggerUnit()
                set u = GetSpellTargetUnit()
                set x = GetUnitX(u)
                set y = GetUnitY(u)
                call Rupture.create(c, u, x, y)
            endif
            set c = null
            set u = null
            return false
        endmethod
        
        static method Conditions2 takes nothing returns boolean
            local unit c
            local unit u
            if GetSpellAbilityId() == ABILID then
                set c = GetTriggerUnit()
                set u = GetSpellTargetUnit()
                if IsUnitInGroup(u, nostack) == true then
                    call IssueImmediateOrder(c, "stop")
                    call SimError(GetTriggerPlayer(), MESSAGE)
                    if GetLocalPlayer() == GetTriggerPlayer() then
                        call ForceUIKey(HOTKEY)
                    endif
                endif
            endif
            set c = null
            set u = null
            return false
        endmethod
        
        static method onInit takes nothing returns nothing
            //start of the trigger
            local trigger t = CreateTrigger()
            local trigger trg
            call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
            // to prevent mana and cooldown loss if the constant NOSTACK is on
            static if NOSTACK then
                set trg = CreateTrigger()
                set nostack = CreateGroup()
                call TriggerRegisterAnyUnitEventBJ(trg, EVENT_PLAYER_UNIT_SPELL_CAST)
                call TriggerAddCondition(trg, Condition(function Rupture.Conditions2))
            endif
            //conditions are faster than actions? so I've heard
            call TriggerAddCondition(t, Condition(function Rupture.Conditions))
            //xe damage stuff
            set xed = xedamage.create()
            static if PURE then //checks the pure constant
                call xepureoptions(xed)
            else
                call xeoptions(xed)
            endif
            //preload
            static if PRELOAD then
                call Preload(BLOODFX)
                set bj_lastCreatedUnit = CreateUnit(Player(15), XE_DUMMY_UNITID, 0., 0., 0.)
                call UnitAddAbility(bj_lastCreatedUnit, DUMMYID)
                call RemoveUnit(bj_lastCreatedUnit)
            endif
        endmethod
        implement T32x
    endstruct
endscope



v1.0 Released.

v1.1
-Fixed spelling errors Rupture instead of Rapture.
-Fixed preloading.
-Added a struct member lvl to the struct.
-Fixed bug with adding unit to group when nostack == false.
-Changed ApplyLife with RemoveUnit
-.count is initially 0.
-Removed creating effect and added it on damage with the xe.

v1.2
-Removed the showunit command
-Now the trigger doesn't initialize 2 triggers but creates one more with NOSTACK
-Added another check to check if the unit is dead according to TH
-Added locals to the conditions according to TH
v1.2b
-Fixed nulling
-Removed removeability.
v1.2c
-Fixed an unnecessary check.
-Fixed error message.
-Now error message is among the constants.
-Now when error shows up, you can choose a new target instantly. (thanks xBlackRose)
v1.2d -Removed a static if.
v1.2e -Fixed another static if with the globals. nostack is now set at the init.
v1.2f -Made the spell run with T32. Fixed a few lines.


Credits goes to:

Vexorian for required stuff

-BZK- for the test map.

Credit me, vexorian and Jesus4Lyf if used.

Enjoy the spell!

Keywords:
stygwyr, bloodseeker, rapture, ultimate, dota, baassee, movement, damage, vjass, mui
Contents

Rupture v1.2f (Map)

Reviews
09:28, 2nd Oct 2010 TriggerHappy: Thanks for updating your spell.

Moderator

M

Moderator

09:28, 2nd Oct 2010
TriggerHappy:

Thanks for updating your spell.
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
rap·ture [ rápchər ]
noun
Definition: overwhelming happiness: a euphoric transcendent state in which somebody is overwhelmed by happiness or delight and unaware of anything else
I think you mean rupture. :p

Anyway short code review:

  • Probably would be a good idea to store level of the ability in the a local variable in the create method. (Or even as a struct member) You could use local variables for integer and reals if you find you're using them more than two times since it can make your code like nicer.
  • Bit nit-picky but I would release the timer before .destroy is called so you won't need to use GetExpiredTimer
  • Instead of destroying the trigger if safety is not needed, you could just do it all in the static if.
  • Your preloading is wrong; you need to set bj_lastCreatedUnit to CreateUnit. You could also just call RemoveUnit instead of UnitApplyTimedLife
  • I don't think you really need a different configuration function if PURE is on.
  • .count can be initially set to 0 like globals.
  • xedamage has a field that allows it to deal damage with a special effect on the unit. I'm not sure if it lets you specify the attachment point though.
Otherwise, nice coding and very good documentation. :D
 
I think you mean rupture. :p

Anyway short code review:

  • Probably would be a good idea to store level of the ability in the a local variable in the create method. (Or even as a struct member) You could use local variables for integer and reals if you find you're using them more than two times since it can make your code like nicer.
  • Bit nit-picky but I would release the timer before .destroy is called so you won't need to use GetExpiredTimer
  • Instead of destroying the trigger if safety is not needed, you could just do it all in the static if.
  • Your preloading is wrong; you need to set bj_lastCreatedUnit to CreateUnit. You could also just call RemoveUnit instead of UnitApplyTimedLife
  • I don't think you really need a different configuration function if PURE is on.
  • .count can be initially set to 0 like globals.
  • xedamage has a field that allows it to deal damage with a special effect on the unit. I'm not sure if it lets you specify the attachment point though.
Otherwise, nice coding and very good documentation. :D

yeah, you can specify the attachment point of the damage special effect in xedamage...

anyway, rupture... nice spell...
 
Level 15
Joined
Jul 6, 2009
Messages
888
:) - I just realised some of the points I made have been mentioned by watermelon O_O

1)GetWidgetX/Y is faster than GetUnitX/Y ~ or so I read. I just wanted to bring that up, and hope someone can confirm it.

2) Maybe it should damage units whose Z change as well as X/Y. So like. A jump on the spot spell would cause damage.

3) Does CreateUnit return bj_lastCreatedUnit? I didn't think it did. I'm probably wrong but.

4) You use GetUnitAbilityLevel 6 times in the create method, I think you know what to do :)

5) I always disliked xedamage o_O What's wrong with normal JASS functions for damage?

6) local Rapture this could be local thistype this.
 
Level 18
Joined
Feb 4, 2009
Messages
1,315
1)GetWidgetX/Y is faster than GetUnitX/Y ~ or so I read. I just wanted to bring that up, and hope someone can confirm it.

I can confirm that GetWidgetX is slower than GetUnitX and that GetWidgetLife is slower faster than GetUnitState (the difference is smaller though)
some dude suggestet that it might be because of typecasting
 
Last edited:
Level 21
Joined
Nov 14, 2008
Messages
3,256
I think you mean rupture. :p

Anyway short code review:

  • Probably would be a good idea to store level of the ability in the a local variable in the create method. (Or even as a struct member) You could use local variables for integer and reals if you find you're using them more than two times since it can make your code like nicer.
  • Bit nit-picky but I would release the timer before .destroy is called so you won't need to use GetExpiredTimer
  • Instead of destroying the trigger if safety is not needed, you could just do it all in the static if.
  • Your preloading is wrong; you need to set bj_lastCreatedUnit to CreateUnit. You could also just call RemoveUnit instead of UnitApplyTimedLife
  • I don't think you really need a different configuration function if PURE is on.
  • .count can be initially set to 0 like globals.
  • xedamage has a field that allows it to deal damage with a special effect on the unit. I'm not sure if it lets you specify the attachment point though.
Otherwise, nice coding and very good documentation. :D

LOL what a mistake XD well fuck it, I dont care :D

1. Yeah I probably should, didnt notice that lol I hate configuration functions :p I'll just make it as a member.

2. So I should release the member .t instead?

3. Am I wrong or cant locals be put elsewhere except top of function? ^^

4. Oh nice miss, I'll fix it.

5. damageTargetForceValue doesn't need damagetype and attack type, that's why and neither does it need force damage :p

6. Fixed.

7. F*ck ...
JASS:
// method .useSpecialEffect("effect\\path.mdl", "origin")
my bad, fixed.

Thanks for the reply, will work on it :D

:) - I just realised some of the points I made have been mentioned by watermelon O_O

1)GetWidgetX/Y is faster than GetUnitX/Y ~ or so I read. I just wanted to bring that up, and hope someone can confirm it.

2) Maybe it should damage units whose Z change as well as X/Y. So like. A jump on the spot spell would cause damage.

3) Does CreateUnit return bj_lastCreatedUnit? I didn't think it did. I'm probably wrong but.

4) You use GetUnitAbilityLevel 6 times in the create method, I think you know what to do :)

5) I always disliked xedamage o_O What's wrong with normal JASS functions for damage?

6) local Rapture this could be local thistype this.

1. I don't think it wont matter? As I know, isn't getunitx/y fast enough? Different with GetUnitState I think

2. So I should add some kind of z damage factor movement? That would have been cool I guess :)

3. heh fixed.

4. Yes I do :D

5. I just wanted xe's force value stuff but I agree, it is possible by yourself

6. is thistype faster or? I thought it just was so you didnt have to type every struct name?

Thanks for reply :D

I can confirm that GetWidgetX is slower than GetUnitX and that GetWidgetLife is slower than GetUnitState (the difference is smaller though)
some dude suggestet that it might be because of typecasting

What I always heard that getwidgetlife/setwidgetlife are faster than getunitstate/setunitstate? Thanks anyways :D

yeah, you can specify the attachment point of the damage special effect in xedamage...

anyway, rupture... nice spell...

thanks :D
 
Level 18
Joined
Feb 4, 2009
Messages
1,315
just tested again and the GetUnitState thing was wrong
however GetWidgetX is defenitly slower than GetUnitX

Empty trigger: <1 sec
if true then endif: <1 sec
GetUnitX: 11 sec
GetWidgetX: 13 sec
GetWidgetLife: 13 sec
GetUnitState: 15 sec
GUI Life of Unit: 25+ sec and game quit
SetUnitX: 21 sec

  • Speedtest
    • Events
      • Player - Player 1 (Red) skips a cinematic sequence
    • Conditions
    • Actions
      • For each (Integer i) from 1 to 2000, do (Actions)
        • Loop - Actions
          • -------- Code 10 times --------
      • Set i2 = (i2 + 1)
      • Custom script: if udg_i2 < 1000 then
      • Trigger - Run (This trigger) (ignoring conditions)
      • Custom script: else
      • Set i2 = 0
      • Custom script: endif
measured with a manual stop watch on a pc with core 2 duo 8500E, 4 gb ram on windows 7 64 bit (geforce 8800 gt which does not really matter I think) on fullscreen with world editor and firefox running in background

2) Maybe it should damage units whose Z change as well as X/Y. So like. A jump on the spot spell would cause damage.

just told some guy that changing units flying height to 999999 is an easy way to hide them without fucking with selection
if you are going to implement z-damage this would be an instant kill oO
also handeling the GetLocationZ together with the flying height might turn out as a little bit challenging
 
Last edited:
Level 14
Joined
Nov 18, 2007
Messages
1,084
Yes, that's what I meant for the timer. Not too drastic but yeah. :p

You could declare the local but not set it to anything unless it passes the static if.
Ex:
JASS:
local trigger t
static if true then
    set t = CreateTrigger()
    ...
It's not necessary to pass the spell target unit's coordinates in Rupture's create method and it might even be better if you didn't so you don't have to keep calling [/icode]GetSpellTargetUnit in the Conditions method. Instead, just do [code=jass]set .X = GetUnitX(u) set .Y = GetUnitY(u)[/code]Not too important though. In your preloading, [icode=jass]ShowUnit and UnitRemoveAbility is not necessary.
1) No reason to hide the unit since it'll get instantly removed.
2) No need to remove the ability since adding it will be enough to preload it.

thistype basically equals the struct name so I don't think it will be any faster than writing Rupture manually. There are some good uses for thistype but I can't think of any right now.
 
Level 38
Joined
Sep 26, 2009
Messages
8,456
Timers still don't have to have their own entire arrays from what I can tell ;)

thistype is good in modules, textmacros and situations where your struct name has more characters in it than "thistype" does. "data" is a good name for a private struct. Fast to type and very easy to remember.

I hate the keyword "this" in vJass because you can't have a struct member with the same name as a local-variable without wondering if you are setting the right variable at all. Instead of saying: local data this I go with local data dat because it forces me to type dat.x, dat.y instead of leaving it ambiguous and having to rely on JassHelper's bugged compile-time process.

You can thank Blackrose for getting me to use data as a structname.
 
Level 21
Joined
Nov 14, 2008
Messages
3,256
Timers still don't have to have their own entire arrays from what I can tell ;)

thistype is good in modules, textmacros and situations where your struct name has more characters in it than "thistype" does. "data" is a good name for a private struct. Fast to type and very easy to remember.

I hate the keyword "this" in vJass because you can't have a struct member with the same name as a local-variable without wondering if you are setting the right variable at all. Instead of saying: local data this I go with local data dat because it forces me to type dat.x, dat.y instead of leaving it ambiguous and having to rely on JassHelper's bugged compile-time process.

You can thank Blackrose for getting me to use data as a structname.

I'll fix the timers here too :D

Well I used to use dat (thx to hell gate and Deaod) but then I saw everybody using "this" and when watermelon used this, I wanted to start to use this. Oo lots of thisses :p
 
Level 15
Joined
Jul 6, 2009
Messages
888
What's th?

1) The order of the code in your onInit method is strange.
JASS:
            local trigger t = CreateTrigger()
            local trigger trg
            call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
            // to prevent mana and cooldown loss if the constant NOSTACK is on
            static if NOSTACK then
                set trg = CreateTrigger()
                call TriggerRegisterAnyUnitEventBJ(trg, EVENT_PLAYER_UNIT_SPELL_CAST)
                call TriggerAddCondition(trg, Condition(function Rupture.Conditions2))
            endif
            //conditions are faster than actions? so I've heard
            call TriggerAddCondition(t, Condition(function Rupture.Conditions))

Why is call TriggerAddCondition(t, Condition(function Rupture.Conditions)) not with it's corresponding event registration? If you move that above, trg becomes useless, as you can reuse t.

2) function Rupture.Conditions/2 - Not very informative. I thought the spells have two conditions they need to be parsed through. Something such as OnCast and OnEffect tells me more. People would know that those functions would take place on EVENT_PLAYER_UNIT_SPELL_EFFECT/CAST.

3) Why have a second check of NOSTACK?

JASS:
                static if NOSTACK then
                    if IsUnitInGroup(u, nostack) == true then
                        call IssueImmediateOrder(c, "stop")
                        call SimError(GetOwningPlayer(c), "Target Unit has already Rapture")
                    endif
                endif

You have this.

JASS:
            static if NOSTACK then
                set trg = CreateTrigger()
                call TriggerRegisterAnyUnitEventBJ(trg, EVENT_PLAYER_UNIT_SPELL_CAST)
                call TriggerAddCondition(trg, Condition(function Rupture.Conditions2))
            endif

4) "Target Unit has already Rapture" should be configurable. Also what is Rapture? Perhaps try to stick with Blizzard-like error messages, like "Target is already ruptured.".

5) Rather than just cancelling the order, how about you order the player to press the hotkey of the ability so you can choose your target again.

6) What's the point in this in Conditions?

JASS:
                static if NOSTACK then
                    if IsUnitInGroup(u, nostack) == false then
                        call Rupture.create(c, u, x, y)
                    endif
                else

If the unit is already being ordered to stop casting, it won't ever get to this point of code?

7) You can also inline every single local used in Conditions. You're using a static if. Things like GetUnitX will only be called once. Also eliminating the needs of local variable in the condition is good :)

8) Variable names like c and u don't tell me anything. WTF is c and u? I know by checking your code, but on bigger codes, do you really want to use names like c and u instead of sourceUnit and targetUnit or something?

9) Why does create take the x/y of the target? Just set them in the create method.

10) Blerh. Lost interest in this :thumbs_down: There's probably more I could say, but nah!
 
Level 21
Joined
Nov 14, 2008
Messages
3,256
What's th?

1) The order of the code in your onInit method is strange.
JASS:
            local trigger t = CreateTrigger()
            local trigger trg
            call TriggerRegisterAnyUnitEventBJ(t, EVENT_PLAYER_UNIT_SPELL_EFFECT)
            // to prevent mana and cooldown loss if the constant NOSTACK is on
            static if NOSTACK then
                set trg = CreateTrigger()
                call TriggerRegisterAnyUnitEventBJ(trg, EVENT_PLAYER_UNIT_SPELL_CAST)
                call TriggerAddCondition(trg, Condition(function Rupture.Conditions2))
            endif
            //conditions are faster than actions? so I've heard
            call TriggerAddCondition(t, Condition(function Rupture.Conditions))

Why is call TriggerAddCondition(t, Condition(function Rupture.Conditions)) not with it's corresponding event registration? If you move that above, trg becomes useless, as you can reuse t.

2) function Rupture.Conditions/2 - Not very informative. I thought the spells have two conditions they need to be parsed through. Something such as OnCast and OnEffect tells me more. People would know that those functions would take place on EVENT_PLAYER_UNIT_SPELL_EFFECT/CAST.

3) Why have a second check of NOSTACK?

JASS:
                static if NOSTACK then
                    if IsUnitInGroup(u, nostack) == true then
                        call IssueImmediateOrder(c, "stop")
                        call SimError(GetOwningPlayer(c), "Target Unit has already Rapture")
                    endif
                endif

You have this.

JASS:
            static if NOSTACK then
                set trg = CreateTrigger()
                call TriggerRegisterAnyUnitEventBJ(trg, EVENT_PLAYER_UNIT_SPELL_CAST)
                call TriggerAddCondition(trg, Condition(function Rupture.Conditions2))
            endif

4) "Target Unit has already Rapture" should be configurable. Also what is Rapture? Perhaps try to stick with Blizzard-like error messages, like "Target is already ruptured.".

5) Rather than just cancelling the order, how about you order the player to press the hotkey of the ability so you can choose your target again.

6) What's the point in this in Conditions?

JASS:
                static if NOSTACK then
                    if IsUnitInGroup(u, nostack) == false then
                        call Rupture.create(c, u, x, y)
                    endif
                else

If the unit is already being ordered to stop casting, it won't ever get to this point of code?

7) You can also inline every single local used in Conditions. You're using a static if. Things like GetUnitX will only be called once. Also eliminating the needs of local variable in the condition is good :)

8) Variable names like c and u don't tell me anything. WTF is c and u? I know by checking your code, but on bigger codes, do you really want to use names like c and u instead of sourceUnit and targetUnit or something?

9) Why does create take the x/y of the target? Just set them in the create method.

10) Blerh. Lost interest in this :thumbs_down: There's probably more I could say, but nah!

1. Triggerhappy :D

1b. It was before Trigger complained about it. Before I created both triggers and I wanted them to be in pairs like, two triggers, two events, two conditions, that's why. Doesn't really matter does it? :p

2. kinda agrees

3. it's a part of the "prevent casting"

JASS:
static if NOSTACK then
    if IsUnitInGroup(u, nostack) == true then
        call IssueImmediateOrder(c, "stop")
        call SimError(GetOwningPlayer(c), "Target Unit has already Rapture")
    endif
endif

4. Didn't actually know the message correctly. Oh thought the find and replace function fixed all these :( fix in a sec

5. I rather stop the unit but ofc this wont be that hard to implent thanks :)

6. fixed

7. TH told me that I should use locals in my conditions for better reading, from the beginning I didn't have them there :p

8. this, I used to have t or targ for target unit and cast/caster for c but I was lazy :D

JASS:
    private struct Rupture // the struct
        
        unit    c //casting unit
        unit    u //target unit
        integer lvl
        real    dmg//the movement percent
        real    maxdmg//the max damage per interval
        real    check//the interval check
        real    count = 0.//counter to count the interval
        real    X//current x of the target
        real    Y//current y of the target
        real    dur//duration of the spell
        timer   t//the timers

9. it doesn't really matter as you say I would do it in the create if I didn't do it in the cond

10 :(
 
Level 15
Joined
Jul 6, 2009
Messages
888
Ergh. Why do you quote the entire thing like that? Break it up X_X I don't remember everything of what I said.

1b. It was before Trigger complained about it. Before I created both triggers and I wanted them to be in pairs like, two triggers, two events, two conditions, that's why. Doesn't really matter does it? :p

You could save the need of having trg.

3. it's a part of the "prevent casting"

Oh. Yeah. o_o Although NOSTACK instead of ALLOW_STACKING or something confused me....-shrugs-

7. TH told me that I should use locals in my conditions for better reading, from the beginning I didn't have them there :p

Actually, you need c and u, but not x and y.

8. this, I used to have t or targ for target unit and cast/caster for c but I was lazy :D

Those variables don't really tell anything. What if you had t for time?

9. it doesn't really matter as you say I would do it in the create if I didn't do it in the cond

If you take the unit in the arguments, why would you take it's x/y when you would take them in the create method? It doesn't really matter in THIS case... but still I guess.

10. Why xedamage?
 
Level 21
Joined
Nov 14, 2008
Messages
3,256
Oh. Yeah. o_o Although NOSTACK instead of ALLOW_STACKING or something confused me....-shrugs-

Yeah the spell bugs with the buff if you allow stacking though the damage is still proper

Those variables don't really tell anything. What if you had t for time?

I have t for timer :p

10. Why xedamage?

I was playing with its damageTargetForceValue that's why :p also I was learning to use it :D
 
Level 14
Joined
Nov 18, 2007
Messages
1,084
Have you tried download it and not open it, just play it directly? Else you really need JNGP with latest JASSHelper to compile it although I compiled it correctly before I uploaded it :$
I downloaded and tested the map; it didn't work, even though I didn't open it with anything.
Perhaps you didn't save normally with JNGP? Using only Save As doesn't seem to compile the map correctly.
 
Top