| 06-30-2009, 06:58 AM | #1 | |
Hi. I'm trying to make a spell in JASS. The spell's description is below:
Unfortunately, I'm running into some problems. The trigger is running very slowly. In 12 seconds, it only summons 2 or 3 units, instead of the 4 that are expected. Also, the summoned units do not explode if the spell's target is killed before the debuff expires or is removed. I've been poring over the code for the last hour or so, but I can't think of what to do to fix it. If you have any ideas on how to fix the bugs, or just general suggestions for efficiency or better programming style, please let me know. I've been programming Java for almost a year now, but I only learned JASS about two weeks ago. I'm good at understanding programming ideas, I just don't have enough experience with this language yet. Thanks for your help.  JASS://Dark Beacon by Garryl //=========================================================================== //Constants - Update for new spell and unit IDs in new maps //=========================================================================== //SpellID - ID for the triggering spell constant function DarkBeacon_SpellID takes nothing returns integer return 'A60E' endfunction //BuffID - ID for the buff that lets this trigger know that the target is still affected constant function DarkBeacon_BuffID takes nothing returns integer return 'B608' endfunction //SummonBuffID - ID for the buff that the summoned units' timers display constant function DarkBeacon_SummonBuffID takes nothing returns integer return 'B609' endfunction //DummyUnitID - ID for the dummy unit that casts the repeat spell constant function DarkBeacon_DummyUnitID takes nothing returns integer return 'n604' endfunction //LocustSpellID - ID for the Locust ability. Uses the default Blizzard version. Don't change this unless you modified the base spell. constant function DarkBeacon_LocustSpellID takes nothing returns integer return 'Aloc' endfunction //=========================================================================== //Values - Change values to modify the spell's damage, area, etc. //=========================================================================== //Int to Str Transfer - How many points of Int are transfered to Strength on each cast function DarkBeacon_IntToStrTransfer takes integer lvl returns integer return 1 endfunction //Duration - How long the beacon lasts and over how long will creatures be summoned - Make sure the base spell has the same duration function DarkBeacon_Duration takes integer lvl returns real return 12.00 endfunction //Total Summons - How many creatures will be summoned over the duration function DarkBeacon_TotalSummons takes integer lvl returns integer return 3 + lvl endfunction //Period - How often the spell summons a new shadow creature - once per period function DarkBeacon_Period takes integer lvl returns real return DarkBeacon_Duration(lvl) / DarkBeacon_TotalSummons(lvl) endfunction //Death Check Frequency - How many times per period the spell should check if the target is dead - must always be >0 //Note: Since minimum wait time is 0.27 seconds, the frequency must be set so that Death Check Period is always > 0.27 function DarkBeacon_DeathCheckFrequency takes integer lvl returns integer return 5 endfunction //Death Check Period - Time between death checks function DarkBeacon_DeathCheckPeriod takes integer lvl returns real return DarkBeacon_Period(lvl) / DarkBeacon_DeathCheckFrequency(lvl) endfunction //Summoned Unit ID - make sure to change along with Summoned Unit ID to Level function DarkBeacon_SummonedUnitID takes integer lvl returns integer if (lvl == 1) then return 'e601' elseif (lvl == 2) then return 'e602' elseif (lvl == 3) then return 'e603' else return 'e601' endif endfunction //Unit ID to Level - transforms a unit ID into the level of Dark Beacon that summoned it function DarkBeacon_UnitID2Lvl takes integer unitID returns integer if (unitID == 'e601') then return 1 elseif (unitID == 'e602') then return 2 elseif (unitID == 'e603') then return 3 else return 0 endif endfunction //Units per period - How many units are summoned every period function DarkBeacon_UnitsPerPeriod takes integer lvl returns integer return 1 endfunction //Summon duration - How long each summoned unit lasts function DarkBeacon_SummonDuration takes integer lvl returns real return 15.00 endfunction //Explosion Damage - How much damage the summoned units deal to nearby units if the target dies function DarkBeacon_ExplosionDamage takes integer lvl returns real return 70.00 + 10.00 * lvl endfunction //Explosion Radius - Radius of the summoned units' explosions if the target dies function DarkBeacon_ExplosionRadius takes integer lvl returns real return 100.00 + 50.00 * lvl endfunction //=========================================================================== //Effects - Change to modify the spell's appearance, effects will only be triggered once each at the start of casting //=========================================================================== //=========================================================================== //Repeated Effects - Effects will be repeated every period, starting at the start of channeling //=========================================================================== //Effects will be added once bugs are sorted out. //=========================================================================== //Main Code - Do not touch //=========================================================================== //Conditions function DarkBeacon_Conditions takes nothing returns boolean return GetSpellAbilityId() == DarkBeacon_SpellID() endfunction //Explode Summons - runs each summoned unit exploding if the target dies function DarkBeacon_ExplodeSummons takes nothing returns nothing local unit thisUnit = GetEnumUnit() local integer lvl = DarkBeacon_UnitID2Lvl(GetUnitTypeId(thisUnit)) local location explosionPoint //Don't explode if summoned unit is already dead if (GetUnitState(GetFilterUnit(), UNIT_STATE_LIFE) >= 0.405) then set explosionPoint = GetUnitLoc(thisUnit) call UnitDamagePointLoc(thisUnit, 0.00, DarkBeacon_ExplosionRadius(lvl), explosionPoint, DarkBeacon_ExplosionDamage(lvl), ATTACK_TYPE_NORMAL, DAMAGE_TYPE_MAGIC) call KillUnit(thisUnit) call RemoveLocation(explosionPoint) endif set thisUnit = null endfunction //Main Actions function DarkBeacon_Actions takes nothing returns nothing local trigger trg_DarkBeacon_TargetDies = CreateTrigger() local unit caster = GetTriggerUnit() local unit target = GetSpellTargetUnit() local player owner = GetOwningPlayer(caster) local integer lvl = GetUnitAbilityLevel(caster, DarkBeacon_SpellID()) local integer summon = DarkBeacon_SummonedUnitID(lvl) local integer numSummonedUnits = 0 // counts the number of units summoned local integer deathChecks = 3 // By starting at 1, each summon will come a little earlier, preventing the clipping of the last one local integer buffID = DarkBeacon_BuffID() local real duration = DarkBeacon_Duration(lvl) local integer totalSummons = DarkBeacon_TotalSummons(lvl) local real period = DarkBeacon_Period(lvl) local real deathCheckPeriod = DarkBeacon_DeathCheckPeriod(lvl) local integer deathCheckFrequency = DarkBeacon_DeathCheckFrequency(lvl) local boolean targetHasDied = false local location targetLoc local group summonedUnits = CreateGroup() //Register trigger to check if target dies call TriggerRegisterUnitEvent(trg_DarkBeacon_TargetDies, target, EVENT_UNIT_DEATH) //Transfer Int to Str - Checks that it doesn't go into negatives if (GetHeroInt(caster, false) >= DarkBeacon_IntToStrTransfer(lvl)) then call SetHeroStr(caster, GetHeroStr(caster, false) + DarkBeacon_IntToStrTransfer(lvl), true) call SetHeroInt(caster, GetHeroInt(caster, false) - DarkBeacon_IntToStrTransfer(lvl), true) else call SetHeroStr(caster, GetHeroStr(caster, false) + GetHeroInt(caster, false), true) call SetHeroInt(caster, 0, true) endif //Loop - check if target died every DeathCheckPeriod, and do stuff every period if the target hasn't died loop //Wait for however long to check if the target has died call PolledWait(deathCheckPeriod) if (GetTriggerExecCount(trg_DarkBeacon_TargetDies) > 0) then call ForGroupBJ(summonedUnits, function DarkBeacon_ExplodeSummons) set targetHasDied = true endif exitwhen (targetHasDied) //If target is still alive, check if it still has the Dark Beacon buff on it, else stop the effects exitwhen (UnitHasBuffBJ(target, DarkBeacon_BuffID()) == false) //If the target is still alive and is still affected, do stuff if (deathChecks < deathCheckFrequency) then set deathChecks = deathChecks + 1 else set deathChecks = 0 set numSummonedUnits = numSummonedUnits + 1 set targetLoc = GetUnitLoc(target) call CreateNUnitsAtLoc( DarkBeacon_UnitsPerPeriod(lvl), summon, owner, targetLoc, bj_UNIT_FACING ) call UnitApplyTimedLife(GetLastCreatedUnit(), DarkBeacon_SummonBuffID(), DarkBeacon_SummonDuration(lvl)) call GroupAddUnit(summonedUnits, GetLastCreatedUnit()) // call UnitAddAbility(GetLastCreatedUnit(), DarkBeacon_LocustSpellID()) //Currently commented out. Locust ability, or similar, will be added once other bugs are worked out call RemoveLocation(targetLoc) endif //Exit if all units have been summoned exitwhen (numSummonedUnits >= totalSummons) endloop //Garbage collecting set caster = null set owner = null set target = null call DestroyGroup(summonedUnits) call DestroyTrigger(trg_DarkBeacon_TargetDies) endfunction //=========================================================================== function InitTrig_Dark_Beacon takes nothing returns nothing set gg_trg_Dark_Beacon = CreateTrigger( ) call TriggerRegisterAnyUnitEventBJ( gg_trg_Dark_Beacon, EVENT_PLAYER_UNIT_SPELL_EFFECT ) call TriggerAddCondition( gg_trg_Dark_Beacon, Condition( function DarkBeacon_Conditions ) ) call TriggerAddAction( gg_trg_Dark_Beacon, function DarkBeacon_Actions ) endfunction |
| 06-30-2009, 04:03 PM | #2 |
Well, I can see that you are using just standard JASS. I highly recommend you switch your code over to vJASS. It will make your life a LOT easier. You can define a scope with an initializer, and then you won't have the long and tedious naming of functions. You won't have to define all of your raw codes in functions, and can instead define them in a global block like this: JASS:scope DarkBeacon initializer Init globals private constant integer SPELL_ID = 'A60E' private constant integer BUFF_ID = 'B608' private constant integer SUMMON_BUFF_ID = 'B609' private constant integer DUMMY_UNIT_ID = 'n604' endglobals Then you simply use them like this: local integer lvl = GetUnitAbilityLevel(caster, SPELL_ID) You have a bunch of locals which you define like this: JASS:local integer buffID = DarkBeacon_BuffID() local real duration = DarkBeacon_Duration(lvl) local integer totalSummons = DarkBeacon_TotalSummons(lvl) local real period = DarkBeacon_Period(lvl) local real deathCheckPeriod = DarkBeacon_DeathCheckPeriod(lvl) local integer deathCheckFrequency = DarkBeacon_DeathCheckFrequency(lvl) You wouldn't need to do this with globals, and also having some better named functions that can be private. I'll explain private functions. With the scope present, you can make all of your functions private, and it will automatically all be converted to unique function names. So if you have a function: private function DeathCheckFrequency takes integer returns real with the scope of DarkBeacon, when compiled, this function will look like: function DarkBeacon_DeathCheckFrequency takes integer returns real So essentially, the scope allows you to cut out some tedious naming, and automatically names the code by: scopeName_functionName This lets you keep your code cleaner, without the tedious naming. I noticed that you like to use LONG variable names for stuff. My personal preference is to keep them as short as possible (1 or 2 letters usually), since even if you forget what it was, you have your comments and your variable block to check. I just make a big deal out of making sure code looks as clean as possible. Instead of using a loop with a polled wait, you can use a Timer with the same wait time as the timer's period. You just make the timer periodic so that it's not a one shot timer, and you call all of your actions in the function that you feed the timer. so something like this: JASS:local timer t = NewTimer() //Need Vex's TimerUtils, if you don't want to use it, use CreateTimer() instead. call TimerStart(t, DeathCheckFrequency(lvl), true, function DeathCheckActions) If you need to have information in the timer's function (like trigger unit, target, etc), then you can create a struct which will hold the data and attach it to the timer (and retrieve it in the timer function). Structs CAN be very complicated, but with their basic usage, they are VERY easy :) If you are interested, I can help you with some basic struct usage at request (I myself am only a basic struct user). You forgot to null "targetLoc", and I suggest that instead of using locations, use x and y instead. They don't leak, and are superior in usage. Also, you can use a loop for creating your units, and avoid the CreateNUnitsAtLoc(). You can then avoid using the GetLastCreatedUnit() and instead put the unit inside of a variable. Instead of using UnitDamagePointLoc, use UnitDamagePoint and put in the x and y coordinates instead. I don't have enough time right now to go through all of your code to figure out why you're having a problem, but I hope this helps in the general coding department. Let me know if you'd like to travel the vjass route, and I can help you get started :) |
| 06-30-2009, 09:01 PM | #3 |
Thank you, Zaraf. Your advice looks very sound. If you could just direct me to a good vJASS resource, I'd love to learn how to use it. It looks like it would be much simpler to use than what I'm currently doing. I'd also like to learn how to attach structs to timers, like you mentioned. It would certainly alleviate some of the difficulties I've been having with this and other spells, if I'm understanding you correctly. I'm also slightly concerned about using x/y coordinates instead of locations. I've heard that most of the functions that have versions for either will crash the game if they reference an x/y coordinate outside the playable map area. Do you know a good way to ensure that x/y coordinates are valid? P.S.: I do remove the locations as soon as I'm done with them in the loop, rather than waiting until the end with the rest of my variables. Do I need to null the location variable anyways, despite the actual object being destroyed?  JASS: |
| 06-30-2009, 10:44 PM | #4 |