1. Are you planning to upload your awesome spell or system to Hive? Please review the rules here.
    Dismiss Notice
  2. Melee Mapping contest #3 - Poll is up! Vote for the best 4v4 melee maps!
    Dismiss Notice
  3. The 30th edition of the Modeling Contest is finally up! The Portable Buildings need your attention, so come along and have a blast!
    Dismiss Notice
  4. We have a new contest going on right now! Join the 11th Music Contest! You are to make a Cinematic modern sound-track for this contest, so come and compete with other people for fun.
    Dismiss Notice

Railgun

Submitted by Insanity_AI
This bundle is marked as pending. It has not been reviewed by a staff member yet.
Railgun spell - used to shoot a beam and damage units in a line.
Does damage depending on target's max health and depending on how close the target was to the center of the beam.

Code (vJASS):
library Railgun requires TimerUtils
//==================================================================================================
//---------------------------------Railgun-----by-Insanity_AI---------------------------------------
//==================================================================================================
/*/*    DISCLAIMER:         This spell visually works better on 1.30 PTR                        */*/
/*/*                  Primarily because of BlzGetLocalUnitZ() working properly there...         */*/
//
//  Setup:  Create a Railgun ability and modify trigger spell condition.
//          Create a dummy unit for trajectory visualizaton.
//          Make sure to match those objectIDs with configuration settings in here:
//          Primarily in SpellCondition() and in the Visuals category; VISUALIZATION_ID
/*          Make sure that you have a /*/*Unit Indexer*/*/ in your map.                      
*/

//  A bit of configuration info:
//          You can edit in the Configuration Section just down below.
//          Each configurable function or variable has a little comment describing what it does.
//          There are also functions that are responsible for damaging units, killing destructables,
//          And blacklisting them, those are all at your disposal.
//          I added a lot of visual configuration, I'm pretty sure you can use things like GetTriggerUnit()
//          in there, so I'm pretty sure that alone opens a lot of doors and posibilities.
//
//  About the damage:
//          At the moment it deals percentage of target's max health.
//          The damage is stacked on the target depending on how close they're to the beam,
//          which is determined by the RADIUS.
//          It goes up from 0% to 90%(in configuration) depending on how close the target is
//          to the beam.
//
//  Limitation:
//          It will shoot through terrain hills, so I'd suggest using some sort of pathingblockers
//          ... that is, unless you don't want this intended interaction (or lack of.)
//
//  Modifications:
//          Cast time, cooldown and mana cost is edited in the object editor,
//          as for the other things, they're editable right here.
//
//==================================================================================================
    globals
        private real        array   angle
        private real        array   offsetZ
        private group       array   visualizers
        private lightning   array   beam
        private real        array   fadeOutDuration
        private real        array   fadeOutRate
        private real        array   fadeOutPeriod
        private boolean     array   stopPoint
 
        private trigger     cast                        = CreateTrigger()
        private trigger     stop                        = CreateTrigger()
        private trigger     fire                        = CreateTrigger()
        private boolexpr    destructFilter                = null
        private boolexpr    unitFilter                    = null
        private boolexpr    castFunction                = null
        private boolexpr    stopFunction                = null
        private boolexpr    fireFunction                = null
    endglobals
 
    /*-----------------------------------------------------------------------------*/
    /*========================Configuration Section================================*/
    /*-----------------------------------------------------------------------------*/
 
    //A little foreword; All of these functions have access to GetTriggerUnit() and GetSpellAbilityId(),
    //and with such, opens many customization options.
    //No need for have the library twice to make 2 different spells, is what I am saying.
    private function SpellCondition takes nothing returns boolean
        return (GetSpellAbilityId() == 'A000')
    endfunction
     
    private function GetRange takes nothing returns real
        //Range to where the railgun shoots
        return 4000.00
    endfunction

    private function GetRadius takes nothing returns real
        //Railgun's visualizer area check for destructables and units
        return 100.00
    endfunction

    private function GetBeamStepDistance takes nothing returns real
        //Distance-step to check for beam's object blockers
        //Eg. A tree.
        return 40.00
    endfunction
 
    /*/*/*----------------------------------------------------------------------*/*/*/
    /*/*/*===========================Visuals====================================*/*/*/
    /*/*/*----------------------------------------------------------------------*/*/*/
    globals
        //UnitID of Railgun Trajectory visualization
        private integer     VISUALIZATION_ID        = 'n000'    
        private integer     VISUALIZATION_PLAYER    = PLAYER_NEUTRAL_PASSIVE
        private string      VISUAL_HIT_MODEL        = "Objects\\Spawnmodels\\NightElf\\NECancelDeath\\NECancelDeath.mdl"
        //Aka, how many lightnings does 1 beam contain, stacked up on 1 another.
        private integer     MAXLIGHTNINGCOUNT       = 5
            //- After a lot of thinking, I decided to put this back into variable form, because having multiple spells
            //with different lightningcounts, would cause interference in the lightning array.
    endglobals
 
    private function FadeOut takes nothing returns boolean
        //Would you like the railgun beam to fade out rather than just dissapear
        return true
    endfunction
 
    private function GetBeamDuration takes nothing returns real
        //Time before the railgun beam is removed from the map
        //I should note that you shouldn't cast another beam from the same unit, until this time has passed
        //So, set spell cooldown accordingly
        return 0.2
    endfunction
 
    private function GetFadeOutPeriod takes nothing returns real
        //In what intervals will it apply the fade out rate
        return 0.04
    endfunction
 
    private function GetFadeOutRate takes nothing returns real
        //By how much does the beam fade out per period,
        //(remember lightning colour, including alpha channel, goes from 0.00 to 1.00)
        //also if duration is lower than period, the beam will just dissapear instead, regardless if you have FadeOut as true.
        return 0.2
    endfunction
 
    private function GetVisualizationDistance takes nothing returns real
        //Distance between each of the visualization lights
        return 266.00
    endfunction
 
    private function GetLightningName takes nothing returns string
        // Change beam's lightning model, would suggest vanilla DRAM, DRAL, DRAB and LEAS models
        //Other models don't retain a "beam" sort of structure.
        return "LEAS"
    endfunction
 
    private function SetBeamColor takes lightning beam returns nothing
        //The real values are by order: Red, Green, Blue, Alpha
        //And take values from 0.00  to 1.00, apparently.
        //Now this won't change the beam to actual color, but just reduce the amount of colour the lightning
        //model already has. If you're unsatisfied with the results this gives you, try a different model.
        call SetLightningColor(beam,1.00,1.00,0.75,1.00)
        set beam                                    = null
    endfunction

    private function GetLightningVisualHeightOffset takes nothing returns real
        //Visual Z offset for the lightning.
        return 60.00
    endfunction
 
    private function GetCastAnimation takes nothing returns string
    //Used when the caster is prepearing to fire
        return "stand ready"
    endfunction
 
    private function GetFireAnimation takes nothing returns string
    //Used when caster is firing
        return "spell"
    endfunction
 
    private function GetStandAnimation takes nothing returns string
    //Queued after firing
        return "stand"
    endfunction
 
    /*/*/*----------------------------------------------------------------------*/*/*/
    /*/*/*=====================End of Visuals===================================*/*/*/
    /*/*/*----------------------------------------------------------------------*/*/*/
 
    private function UnitBlacklist takes nothing returns boolean
        //Any unit or unit-type you want it to be immune to this? You can add it here.
        return true
    endfunction
     
    private function DamageVictims takes real squaredDistance returns nothing
    //Function responsible for damaging units, you can edit this however you'd like.
    //squaredDistance is the distance of the unit from the beam... squared.
        local unit target                       = GetEnumUnit()
        local unit caster                       = GetTriggerUnit()
        local real maxRangeSquared              = Pow(GetRadius(),2)
        local real damage
        if(target !=  caster)then
            set damage                          = (maxRangeSquared - squaredDistance)/maxRangeSquared
            if damage > 0.90 then
                set damage                      = 0.90
            endif
            set damage                          = damage * GetUnitState(target,UNIT_STATE_MAX_LIFE)
            call UnitDamageTarget(caster,target,damage,true,false,ATTACK_TYPE_CHAOS,DAMAGE_TYPE_UNKNOWN,WEAPON_TYPE_WHOKNOWS)
            call DestroyEffect(AddSpecialEffectTarget(VISUAL_HIT_MODEL,target,"chest"))
        endif
     
        set target                              = null
        set caster                              = null
    endfunction

    private function DestructableBlacklist takes nothing returns boolean
    //Any additions you'd like to add here?
        local integer   destructType            = GetDestructableTypeId(GetFilterDestructable())
     
        //if the destructable is alive and is not a pathblocker of any kind.
        if GetDestructableLife(GetFilterDestructable()) <= 0 then
            return false
        endif
        set stopPoint[GetUnitUserData(GetTriggerUnit())]          = true
        return not((destructType == 'YTlb') or (destructType == 'YTab') or (destructType == 'YTpb') or (destructType == 'YTfb'))
    endfunction

    private function DestructSearchAndDestroy takes nothing returns nothing
        //How destructables are being destroyed.
        call SetDestructableLife(GetEnumDestructable(), 0.00)
    endfunction
    /*-----------------------------------------------------------------------------*/
    /*===================End of configuration section==============================*/
    /*-----------------------------------------------------------------------------*/
 
      private function CheckUnitTarget takes nothing returns nothing
        local unit target                       = GetEnumUnit()
        local real targetX                      = GetUnitX(target)
        local real targetY                      = GetUnitY(target)
        local unit caster                       = GetTriggerUnit()
        local real casterX                      = GetUnitX(caster)
        local real casterY                      = GetUnitY(caster)
        local integer casterData                = GetUnitUserData(caster)
        local real maxRangeSquared              = Pow(GetRadius(),2)
        local real angleT                       = Atan2(targetY-casterY,targetX-casterX) - angle[casterData]
        local real targetDistance
     
        //Ignore units that are behind the caster.
        if not(angleT < bj_PI/2 and angleT > -bj_PI/2) then
            set target                          = null
            set caster                          = null
            return
        endif
     
        //A slightly modified Distance between a line and a point formula
        //Ax + By + C = 0 => -ax + y + b = 0 from y = ax - b, where a = tan(angle) and b = a(x1) - y1
        //Formula: |Ax + By + C|/Sqrt(A^2 + B^2)=> ((-ax + y + b)^2)/(a^2 + 1)
        set angleT                              = Tan(angle[casterData])
        set targetDistance                      = Pow(-angleT*targetX + targetY + angleT*casterX - casterY,2)/(Pow(angleT,2) + 1)
        if targetDistance > maxRangeSquared then
            set target                          = null
            set caster                          = null
            return
        endif
     
        //If it checks out, damage it
        call DamageVictims(targetDistance)
        set target                          = null
        set caster                          = null
    endfunction
 
    private function GetVisualizationStopIndex takes nothing returns integer
        return R2I(GetRange()/GetVisualizationDistance())
    endfunction
 
    private function GetBeamStopIndex takes nothing returns integer
        return R2I(GetRange()/GetBeamStepDistance())
    endfunction
     
    private function Cast takes nothing returns nothing
        local unit      caster                  = GetTriggerUnit()      
        local rect      destructCheckRect       = Rect(0,0,0,0)
        local unit      visualizer              = null
        local group     visualizerGroup         = CreateGroup()
        local real      casterX                 = GetUnitX(caster)
        local real      casterY                 = GetUnitY(caster)
        local real      casterZ                 = BlzGetLocalUnitZ(caster)
        local real      targetX                 = GetSpellTargetX()
        local real      targetY                 = GetSpellTargetY()
        local real      targetZ            
     
        local real      range                   = GetRange()
        local real      visualDistance          = GetVisualizationDistance()
        local real      radius                  = GetRadius()
     
        local integer   iteration               = 1
        local integer   iterationEnd            = GetVisualizationStopIndex()
        local integer   casterData              = GetUnitUserData(caster)
     
        local real      offsetX  
        local real      offsetY
     
        call SetUnitAnimation(caster, GetCastAnimation())
        set angle[casterData]                   = Atan2(targetY-casterY,targetX-casterX)
        set offsetX                             = visualDistance*Cos(angle[casterData])
        set offsetY                             = visualDistance*Sin(angle[casterData])
     
        set visualizers[casterData]             = CreateGroup()
        set stopPoint[casterData]               = false
        loop
            exitwhen iteration > iterationEnd
            set targetX                         = casterX + offsetX*I2R(iteration)
            set targetY                         = casterY + offsetY*I2R(iteration)
         
            set visualizer                      = CreateUnit(Player(VISUALIZATION_PLAYER),VISUALIZATION_ID,targetX,targetY,angle[casterData])
            call GroupAddUnit(visualizerGroup,visualizer)
         
            call SetRect(destructCheckRect,targetX-radius,targetY-radius,targetX+radius,targetY+radius)
            call EnumDestructablesInRect(destructCheckRect,destructFilter, null)
         
            exitwhen stopPoint[casterData]
            set iteration                       = iteration +1
        endloop
         
        set targetZ                             = BlzGetLocalUnitZ(visualizer)
        set offsetZ[casterData]                 = (targetZ - casterZ)/range
     
        set iteration                           = 1
        loop
            set visualizer                      = FirstOfGroup(visualizerGroup)
            exitwhen visualizer == null
         
            call UnitAddAbility(visualizer,'Amrf')
            call SetUnitFlyHeight(visualizer,offsetZ[casterData] * I2R(iteration) + casterZ - BlzGetLocalUnitZ(visualizer) ,0.00)
            call UnitRemoveAbility(visualizer,'Amrf')
         
            set iteration                       = iteration + 1
            call GroupRemoveUnit(visualizerGroup,visualizer)
            call GroupAddUnit(visualizers[casterData],visualizer)
        endloop
         
        call RemoveRect(destructCheckRect)
        call DestroyGroup(visualizerGroup)
     
        set visualizerGroup                     = null
        set visualizer                          = null
        set caster                              = null
        set destructCheckRect                   = null
    endfunction

    private function Stop takes nothing returns nothing
        //in case the caster stops casting the railgun
        local integer   casterData              = GetUnitUserData(GetTriggerUnit())
        local unit      dummy                   = null
     
        loop
            set dummy                           = FirstOfGroup(visualizers[casterData])
            exitwhen (dummy == null)
            call GroupRemoveUnit(visualizers[casterData],dummy)
            call RemoveUnit(dummy)
        endloop
        call DestroyGroup(visualizers[casterData])
        set dummy                               = null
        set visualizers[casterData]             = null
    endfunction

    private function FireCleanup takes nothing returns nothing
        local timer     t                       = GetExpiredTimer()
        local integer   casterData              = GetTimerData(t)
        local integer   iteration               = 0
        local integer   endIteration            = MAXLIGHTNINGCOUNT
     
        loop
            exitwhen iteration >= endIteration
            call DestroyLightning(beam[casterData*endIteration + iteration])
            set iteration                       = iteration + 1
        endloop
     
        call ReleaseTimer(t)
        set t                                   = null
    endfunction
 
    private function FireFadeOut takes nothing returns nothing
        local timer     t                       = GetExpiredTimer()
        local integer   casterData              = GetTimerData(t)
        local integer   iteration               = 0
        local integer   endIteration            = MAXLIGHTNINGCOUNT
        local lightning localBeam               = null
     
        set fadeOutDuration[casterData]         = fadeOutDuration[casterData] - fadeOutPeriod[casterData]
     
        if fadeOutDuration[casterData] <= 0.00 then
            call PauseTimer(t)
            call FireCleanup()
        endif
     
        loop
            exitwhen iteration >= endIteration
            set localBeam                       = beam[casterData*endIteration+iteration]
            call SetLightningColor(localBeam, GetLightningColorR(localBeam),GetLightningColorG(localBeam),GetLightningColorB(localBeam),GetLightningColorA(localBeam)- fadeOutRate[casterData])
            set iteration                       = iteration + 1
        endloop
     
        set t                                   = null
        set localBeam                           = null
    endfunction

    private function Fire takes nothing returns nothing
        local unit      caster                  = GetTriggerUnit()
        local integer   casterData              = GetUnitUserData(caster)
        local timer     t                       = NewTimerEx(casterData)
        local rect      multipurposeRect        = Rect(0,0,0,0)
        local group     victims                 = CreateGroup()
     
        local real      lightningZOffset        = GetLightningVisualHeightOffset()
     
        local real      casterX                 = GetUnitX(caster)
        local real      casterY                 = GetUnitY(caster)
        local real      casterZ                 = BlzGetLocalUnitZ(caster) + lightningZOffset
        local real      stepDistance            = GetBeamStepDistance()
        local real      radius                  = GetRadius()
        local string    lightningName           = GetLightningName()
        local real      offsetX                 = stepDistance*Cos(angle[casterData])
        local real      offsetY                 = stepDistance*Sin(angle[casterData])
     
        local real      rectX                
        local real      rectY
     
        local integer   iteration               = 0
        local integer   endIteration            = MAXLIGHTNINGCOUNT
     
        local real      targetX
        local real      targetY
        local real      targetZ
     
        set stopPoint[casterData]               = false
     
        //visual appeal
        call SetUnitAnimation(caster, GetFireAnimation())
        call QueueUnitAnimation(caster, GetStandAnimation())
     
        loop
            exitwhen iteration >= endIteration
            set beam[casterData*endIteration + iteration] = AddLightningEx(lightningName ,true ,casterX,casterY ,0.00 ,casterX + 1.00,casterY + 1.00,0.00)
         
            set iteration                       = iteration + 1
        endloop
     
        set iteration                           = 1
        set endIteration                        = GetBeamStopIndex()
        loop
            exitwhen iteration >= endIteration
            set targetX                         = casterX + offsetX*I2R(iteration)
            set targetY                         = casterY + offsetY*I2R(iteration)
         
            call SetRect(multipurposeRect,targetX-radius,targetY-radius,targetX+radius,targetY+radius)
            call EnumDestructablesInRect(multipurposeRect,destructFilter, function DestructSearchAndDestroy)
         
            exitwhen stopPoint[casterData]
            set iteration                       = iteration +1
        endloop
     
        //OffsetX/Y is no longer used, so I'll repurpose them here!
        set offsetX                             = targetX
        set offsetY                             = targetY
        set rectX                               = casterX
        set rectY                               = casterY
     
        if targetX > casterX then
            set offsetX                         = casterX
            set rectX                           = targetX
        endif
     
        if targetY > casterY then
            set offsetY                         = casterY
            set rectY                           = targetY
        endif
     
        call SetRect(multipurposeRect,offsetX-radius,offsetY-radius,rectX+radius,rectY+radius)
        call GroupEnumUnitsInRect(victims,multipurposeRect,unitFilter)
        call ForGroup(victims, function CheckUnitTarget)
     
        set targetZ                             = casterZ + offsetZ[casterData]*R2I(iteration)*stepDistance
        set iteration                           = 0
        set endIteration                        = MAXLIGHTNINGCOUNT
     
        loop
            exitwhen iteration >= endIteration
            call MoveLightningEx(beam[casterData*endIteration + iteration],true,targetX,targetY,targetZ,casterX,casterY,casterZ)
            call SetBeamColor(beam[casterData*endIteration + iteration])
            set iteration                       = iteration + 1
        endloop
     
        if FadeOut() then
            set fadeOutDuration[casterData]     = GetBeamDuration()
            set fadeOutRate[casterData]         = GetFadeOutRate()
            set fadeOutPeriod[casterData]       = GetFadeOutPeriod()
            call TimerStart(t,GetFadeOutPeriod(),true,function FireFadeOut)
        else
            call TimerStart(t,GetBeamDuration(),false,function FireCleanup)
        endif
     
        call DestroyGroup(victims)
        call RemoveRect(multipurposeRect)
        set caster                              = null
        set multipurposeRect                    = null
        set victims                             = null
        set t                                   = null
    endfunction
 
    private function Cast_Wrapper takes nothing returns boolean
        if SpellCondition() then
            call Cast()
            return true
        endif
        return false
    endfunction

    private function Stop_Wrapper takes nothing returns boolean
        if SpellCondition() then
            call Stop()
            return true
        endif
        return false
    endfunction

    private function Fire_Wrapper takes nothing returns boolean
        if SpellCondition() then
            call Fire()
            return true
        endif
        return false
    endfunction
    //================================================================================
    function InitTrig_Railgun takes nothing returns nothing
     
        //this is so that the system knows when to stop the railgun. Value depends on 3 pre-set constants.
        set destructFilter                      = Filter(function DestructableBlacklist)
        set unitFilter                          = Filter(function UnitBlacklist)
     
        set castFunction                        = Condition(function Cast_Wrapper)
        set stopFunction                        = Condition(function Stop_Wrapper)
        set fireFunction                        = Condition(function Fire_Wrapper)
     
        //Cast the Railgun
        call TriggerRegisterAnyUnitEventBJ(cast, EVENT_PLAYER_UNIT_SPELL_CHANNEL)
        call TriggerAddCondition(cast, castFunction)
     
        //Stop the Railgun
        call TriggerRegisterAnyUnitEventBJ(stop, EVENT_PLAYER_UNIT_SPELL_ENDCAST )
        call TriggerAddCondition(stop, stopFunction)
     
        //Be the... no wait, Fire the Railgun
        call TriggerRegisterAnyUnitEventBJ(fire, EVENT_PLAYER_UNIT_SPELL_FINISH)
        call TriggerAddCondition(fire, fireFunction)
     
        call Preload(VISUAL_HIT_MODEL)
    endfunction
endlibrary
 


11.9.2018:
  • Now no longer deals damage 5 times, but will deal damage only once, depending on target's distance from the beam
  • No longer spams GroupEnumUnitsInRange, but rather picks up all units in rect from caster to target (the rect is expanded in all 4 directions by Radius() )
  • System now uses a modified "Distance between point and line" equation, without square root
  • Now deals percentage damage from 0% to 90% depending on distance between target and the beam. (Special case: Ignores units that are caught by GroupEnumUnitsInRect(), but are behind the caster.)
  • Removed GetPointZ and GetUnitZ functions in favour to use BlzGetLocalUnitZ() native (doesn't work in 1.29)
  • Attempted to make the railgun's visualization dummies in a somewhat straight line(in terms of height) , will consider using SFX instead..
  • The system now no longer keeps creating and destroying rects in a loop, but rather creates 1 rect and re-uses it till the loop is done, get's removed at the end of the trigger.
3.9.2018:
  • Nulled a leftover local variable
  • Changes some function names to make more sense
  • Added more comments
2.9.2018:
  • Stumbled upon an idea; added fade-out functionality and made it an option.
  • Changed GetMaxLightningCount() function into a global constant, to prevent lightning array problems.
  • Slightly changed the beam's color to have a bit of an orange hue.
25.8.2018:
  • Removed hashtable usage
  • Added more configuration functions
  • Changed the lightning model to Magic Leash lightning effect
  • Changed scope into library, now requires TimerUtils and a UnitIndexer
  • Removed some leaks from the test map
  • Removed Railgun_ prefixes

  • Adjust unit group in Fire function, now it doesn't create and destroy multiple groups, but only creates one and destroys it.
  • Fixed test map's Attribute Bonus ability.

  • Forgot to edit comments.. there they are.

23.8.2018:
  • Placed everything under a scope
  • Moved most of globals to be functions
  • Replaced TriggerAddAction with TriggerAddCondition
  • Made a configurable hashtable section for those who may want to incorporate this into 1 hashtable
  • Cleaned up some leftover not-nulled variables

19.7.2018:
  • Removed vJASS code,
  • Added/Changed/Removed some configuration
  • Removed excessive over-use of GetLocationZ
  • Changed the code, now spawns only 5 beams that are gradually getting longer with distance

27.5.2018:
  • Added some newlines in the code for more readability
  • Changed most, if not, all the text to the JPAG standard
  • Added an option for the end-user to either use LocationZ or not
  • nulled some variables
  • Added a spellshield effect to the caster when firing the railgun

2.3.2018:
  • Indented some words
  • Added some blank lines between functions
  • Replaced the eye-bleed-inducing structures with arrays
  • Privatized globals and removed their prefixes
Previews
Contents

Railgun (Map)

Reviews
MyPad
This bundle has been updated. Thus, it is only proper to leave behind another review. This review is more rigorous than the last. Nitpicks: Constants which are configurable could be moved to functions instead. Adding some more effects to the...
Bribe
Instead of prefixing everything with Railgun_, use the "private" keyword. This doesn't need a hashtable. A unit indexer, which many maps already have, would serve better (and faster). Work towards implementing more of these kinds of changes (such as...
  1. Clanzion

    Clanzion

    Joined:
    Jul 4, 2016
    Messages:
    223
    Resources:
    0
    Resources:
    0
    Going to test it out now that it is publicly available.
     
  2. Chaosy

    Chaosy

    Joined:
    Jun 9, 2011
    Messages:
    10,410
    Resources:
    17
    Maps:
    1
    Spells:
    10
    Tutorials:
    6
    Resources:
    17
    Structure makes my eyes bleed.

    Indent one step when inside any block, this includes the global block AND function blocks.

    Some blank rows between functions would increase readability too
     
  3. MyPad

    MyPad

    Spell Reviewer

    Joined:
    May 9, 2014
    Messages:
    1,107
    Resources:
    2
    Models:
    1
    Icons:
    1
    Resources:
    2
    EDIT:

    This is a previous review, and has been archived in the next review.
     
    Last edited: Aug 22, 2018
  4. MyPad

    MyPad

    Spell Reviewer

    Joined:
    May 9, 2014
    Messages:
    1,107
    Resources:
    2
    Models:
    1
    Icons:
    1
    Resources:
    2
    This bundle has been updated. Thus, it is only proper to leave behind another review. This review is more rigorous than the last.

    Previous Review

    Nitpicks:

    • Constants which are configurable could be moved to functions instead.
    • Adding some more effects to the railgun when firing would be appreciated.
    Notes:

    • There is a lot to be desired from readability.
    • Try to observe the JPAG standard when submitting resources such as these.
      • Constants should be in UPPER_CASE
      • Non-constants, locals, variables should be camelCase
      • Functions should be LikeThis.
    • Null your variables that are handles or extensions thereof.
    Status:
    • [color]Awaiting Update.[/color]


    Notes:


    • As the code is written in such a way that it would not normally compile on regular World Editor (before patch 1.30), I would suggest enclosing this in a scope.
    • Caps-Locked variables are constants in normal JPAG convention, but in the spell, these are not constants.
    • Functions can be preceded with a private prefix to save you some time in typing.
    • In function Railgun_DamageVictims, a float literal was spotted on the declaration of a local real variable damage.
      local real damage           = 0.16*GetUnitState(target,UNIT_STATE_MAX_LIFE)
    • In function Railgun_DestructableBlackList, the statement can be shortened from the following:
      Code (vJASS):

      local unit caster           = GetTriggerUnit()
      local integer destructType  = GetDestructableTypeId(GetFilterDestructable())
         
      //if the destructable is alive and is not a pathblocker of any kind.
      if GetDestructableLife(GetFilterDestructable()) <= 0 then
          return false
      endif
      if destructType == 'YTlb' then
          call SaveBoolean(RAILGUN_TABLE,GetHandleId(caster),StringHash("Railgun_Rangelimit"),true)
          return false
      endif
      if destructType == 'YTab' then
          call SaveBoolean(RAILGUN_TABLE,GetHandleId(caster),StringHash("Railgun_Rangelimit"),true)
          return false
      endif
      if destructType == 'YTpb' then
          call SaveBoolean(RAILGUN_TABLE,GetHandleId(caster),StringHash("Railgun_Rangelimit"),true)
          return false
      endif
      if destructType == 'YTfb' then
          call SaveBoolean(RAILGUN_TABLE,GetHandleId(caster),StringHash("Railgun_Rangelimit"),true)
          return false
      endif
      call SaveBoolean(RAILGUN_TABLE,GetHandleId(caster),StringHash("Railgun_Rangelimit"),true)
      return true
       


      to

      Code (vJASS):

      local unit caster           = GetTriggerUnit()
      local integer destructType  = GetDestructableTypeId(GetFilterDestructable())
         
      //if the destructable is alive and is not a pathblocker of any kind.
      if GetDestructableLife(GetFilterDestructable()) <= 0 then
          return false
      endif

      call SaveBoolean(RAILGUN_TABLE,GetHandleId(caster),StringHash("Railgun_Rangelimit"),true)
      return (destructType == 'YTlb') or (destructType == 'YTab') or (destructType == 'YTpb') or (destructType == 'YTfb')
       
    • In function Railgun_UnitBlackList, you can delete the local variable declaration. (It is not particularly useful).
    • In functions Railgun_Cast, Railgun_Stop, and Railgun_Fire, you can merge the checking of the correct ability being cast into said functions.
      You can also use
      TriggerAddCondition
      instead of
      TriggerAddAction
      if only for an incremental performance gain.
    • In function Railgun_Cast, within the first loop, the variable visualizer is assigned to a newly created unit owned by the Passive player. You can store the passive player into a variable, since its' value is constant.

      The local group variable visualization is never nulled.

    • In function Railgun_Stop, the locally declared group g is not nulled.

      As much as FirstOfGroup is preferred over ForGroup here, the possibility of ghost units being included may prematurely end the loop.

    • In function Railgun_FireCleanup, using a FirstOfGroup over a ForGroup is preferable, since the dummy units aren't likely to be removed by anything else.
    • In function Railgun_Fire, on the second loop, using a FirstOfGroup over a ForGroup is preferable, as the type of iteration is destructive.

    Nitpicks:


    • Using string hashes may be fine, but beware of using them too often, as they are likely to produce key collisions.
    • You might want to add a warning to all who will use this, that this uses up one hashtable out of 255-256.
    • The spell could use some constant functions, which store information about a particular aspect of the spell.

    Status:


    • Awaiting Update
     
  5. Bribe

    Bribe

    Joined:
    Sep 26, 2009
    Messages:
    7,723
    Resources:
    25
    Maps:
    3
    Spells:
    10
    Tutorials:
    3
    JASS:
    9
    Resources:
    25
    Instead of prefixing everything with Railgun_, use the "private" keyword.

    This doesn't need a hashtable. A unit indexer, which many maps already have, would serve better (and faster).

    Work towards implementing more of these kinds of changes (such as the ones from MyPad), and I'll return here to give it another look once you've done so. I want to see how many things you spot on your own, first, before I point them out.

    This resource looks cool. Your code will get more refined. Lots of room for growth.