| 07-03-2006, 06:15 AM | #1 |
Blackrose Ritual Description: When cast, it begins an unstoppable ritual that will inevitably kill at least one living sacrifice. The sacrifices will continuously lose life to damage enemies nearby. The damage dealt will only be a fraction of the amount drain from the units, but since it damages the entire area, is potentially far more powerful. Naakaloh Spell Session 05 - Blackrose Ritual f.w3x Edited by Naakaloh. Reason: Fixing issues noted by PipeDream, thanks. Edited by Naakaloh again. Reason: Fixing more of the issues noted by PipeDream. Edited by Naakaloh again 2. Reason: Removed all potential issues with nulling timers. Notes: - The number of sacrifices increase the damage dealt. - The spell can dynamically be made to last longer or finish sooner by healing or damaging the sacrifices. - The targetting cursor is for choosing sacrifices; the damage is dealt in a much larger area, which is shown by the effect. - The filename is a little long, so try not to put it too many folders deep; apparently warcraft has issues detecting it if the filepath is too long. - Winner of the Spell Session 5. |
| 07-04-2006, 12:32 AM | #2 |
Very nice job. Real clean work. Stuff that needs to be fixed for approval: - spelling: "Persistent" - Need local hostage fixes: BlackroseRitual_GroupSortWeakestToStrongest BlackroseRitual_SacrificeUnitCondition BlackroseRitual_DamageCondition BlackroseRitual_SacrificeDrain BlackroseRitual_ReturnXSizeSubgroup (can remove set g= null. use return bug to fix the leak though) - Timer is set to null in BlackroseRitual_CastActions, use ItoT and store your timers in integers instead. - PauseTimer() before DestroyTimer() - BlackroseRitual_MoveMissilesCircle: You had some troubles removing that G? Give it another go. You have to get rid of the DestroyGroup(uninitialized var) one way or another. Actually I think the reason it works with DestroyGroup is that it stops the thread when you run it on an uninitialized variable. GetPlayableMapRect is a Blizzard.j function which returns bj_mapInitialPlayableMapRect, which you then would destroy and ruin further attempts to use it. So, don't destroy r and you should be fine. You can also use GetWorldBounds() if you're so inclined, which is a native that does allocate a new rect. - First cast lag --- Suggestions / comments: - Why do you destroy casting tempLoc when you shove a copy into the handle vars? Just shove tempLoc in directly. - Select the subgroup randomly. Surprisingly this can be done in linear time in size of group to select from:  JASS://Given a group, filter it down to a random set of size max //If it's smaller than max, the group's contents are unchanged //Returns the size of the group which will be <= max //Runs in O(N) time where N is the number of units in the original group function FilterNRandomUnits takes group g, integer max returns integer local integer n = 0 local integer j local unit u local unit array pool local integer t loop //First pull n units from g set u = FirstOfGroup(g) exitwhen n >= max or u == null set pool[n] = u call GroupRemoveUnit(g,u) set n = n + 1 endloop set j = n loop //Harmonically decaying random replacement set u = FirstOfGroup(g) exitwhen u == null set t = GetRandomInt(0,j) set j = j + 1 if(t < max) then set pool[t] = u endif call GroupRemoveUnit(g,u) endloop call GroupClear(g) set j = 0 loop //Rebuild group exitwhen j >= n call GroupAddUnit(g,pool[j]) set j = j + 1 endloop return n endfunction - IsUnitGroupEmptyBJ iterates through the entire group. You could do a lot better by just checking if FirstOfGroup == null. - You store a lot of redundant data. You might be able to get some noticeable improvements by cutting out some unnecessary game cache accesses in BlackroseRitual_MoveMissilesCircle. You could also cut out a lot of trig calls. - if( GetRandomReal(0.0, 1.0) < GetRandomReal(0, 0.03) ) -> if(GetRandomReal(0.0,1.0) < .015)) |
| 07-04-2006, 04:39 AM | #3 |
| 07-04-2006, 11:19 AM | #4 | |
Quote:
Leaks: There's stuff not set to null. Some of it (like the filters) could just be set to null, some of it is return values so you have to be a little tricky: JASS:function BlackroseRitual_GroupSortWeakestToStrongest takes group unsorted, boolean reversed returns group local group sorted = CreateGroup() local integer iSorted //... set iSorted = HtoI(sorted) set sorted = null return iSorted return null endfunction JASS:function BlackroseRitual_GroupSortWeakestToStrongest takes group sorted, group unsorted, boolean reversed returns nothing endfunction Timer: Nulling timers causes unrepeatable problems. Nobody is sure why. In Mainactions I would avoid this by using GetExpiredTimer() everywhere you need the timer. In cast actions, store HtoI(timer) to a variable and use ItoT where you need something of type timer. This is inconvenient to do with kattana style handle vars, so you might as well use direct game cache. JESP: In the manifest, you forgot to change "spells in this map" to Blackrose Ritual You might also mention that it requires simerror. |
| 07-10-2006, 06:59 PM | #5 |
| 07-13-2006, 06:25 AM | #6 |
| 07-13-2006, 06:48 AM | #7 |
| 07-13-2006, 10:33 AM | #8 |
| 07-16-2006, 06:46 PM | #9 |
| 07-16-2006, 07:33 PM | #10 |
JASS:if( GetRandomReal(0.0, 1.0) < GetRandomReal(0, 0.015) ) call SetHandleHandle(ItoTi(t)) is a little strange too, since internally they're I2S(HtoI(arg)). this is only in cast actions though so there's no point wasting time optimizing. |
| 07-16-2006, 07:50 PM | #11 |