wc3campaigns
WC3C Homepage - www.wc3c.netUser Control Panel (Requires Log-In)Engage in discussions with other users and join contests in the WC3C forums!Read one of our many tutorials, ranging in difficulty from beginner to advanced!Show off your artistic talents in the WC3C Gallery!Download quality models, textures, spells (vJASS/JASS), systems, and scripts!Download maps that have passed through our rigorous approval process!

Go Back   Wc3C.net > Resources > Code Resources > vJass Spells
User Name
Password
Register Rules Get Hosted! Chat Pastebin FAQ and Rules Members List Calendar



Reply
 
Thread Tools Search this Thread
Old 03-30-2009, 05:09 AM   #31
akolyt0r
In Flames
 
akolyt0r's Avatar
 
Join Date: Jan 2006
Posts: 1,154

Submissions (3)

akolyt0r has a spectacular aura about (120)

Default

Quote:
Originally Posted by Pyrogasm
A double free is exactly the opposite of a leak. It's destroying the same object twice.
sure, but pretty often a double free means you most probably missed to destroy one object, but destroyed another one twice instead ...

So the first object will leak.
Havent looked at the code though ..so i dunno if this applies to your code aswell.
__________________
akolyt0r is offline   Reply With Quote
Sponsored Links - Login to hide this ad!
Old 03-30-2009, 05:22 AM   #32
FriendlyPsycho
User
 
FriendlyPsycho's Avatar
 
Join Date: Feb 2009
Posts: 109

Submissions (1)

FriendlyPsycho is on a distinguished road (12)

Default

I don't get the double frees when I test it though..
FriendlyPsycho is offline   Reply With Quote
Old 03-31-2009, 06:55 PM   #33
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,190

Submissions (19)

Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

Quote:
Originally Posted by Flame_Phoenix
Please fix it.
Update your xe, you nab.


As for the spell, TimedXefx is still an issue. It hasn't yet gone through our review process yet so you can't use it in a spell you submit here. Since it's such a simple library, I'll review it right now: The struct data is useless, you could have just as well attached the xefx to the timer directly. The end result is such a short code that you could just as well inline it into the spell entirely and thus avoid me telling you again that you can't use it.

Also, you should remove the earlier versions of the spell from the post since only the last version will actually be the one to be approved.
__________________

Last edited by Anitarf : 03-31-2009 at 06:55 PM.
Anitarf is offline   Reply With Quote
Old 03-31-2009, 08:16 PM   #34
Flame_Phoenix
retired coder | real ilfe
 
Flame_Phoenix's Avatar
 
Join Date: Mar 2007
Posts: 2,208

Submissions (10)

Flame_Phoenix has a spectacular aura about (90)Flame_Phoenix has a spectacular aura about (90)Flame_Phoenix has a spectacular aura about (90)Flame_Phoenix has a spectacular aura about (90)

Send a message via MSN to Flame_Phoenix
Default

Quote:
Update your xe, you nab.
Actually if you bother enough to see my replay and my screen, you will notice that I got the "double free message" using the test map provided by the creator. I tested the most recent version (at that time) and so I assume that the demo map had the most recent xe (at that time). If anyone has to update something, that someone is not me, but the creator in the demo map, because the error was found in the test map.
On the other hand, if you don't care about that problem, than it is also not my problem as well. The fact is that (at that time) I got an error message I didn't like and I reported in a friendly way. By ignoring my report you risk your credibility as a mod, because if someone tests this spell in the future and if the tester finds the problem again, the tester will think that Anitarf is a nab mod because he approved a spell with a problem.
I see the version was updated, I am happy by that, I didn't test the latest version yet, so I can not say if the problem was fixed or not. I will just remove myself from this thread, I can obviously see my reports are not welcome here.

Btw, Thx to ako for the explanation of the error. I would give you rep++ but it looks like I have to spread some more rep =S
__________________
Check out my tutorials at:
1-Creating a Hero Tavern
2-Complete Icon Tutorial - ALL about Icons
3-Making a spell in vJass - Practice Session 1
Check out all my current spells at here
Finally, check my project:
Castle vs Castle Flame Edition

Last edited by Flame_Phoenix : 03-31-2009 at 08:16 PM.
Flame_Phoenix is offline   Reply With Quote
Old 03-31-2009, 09:02 PM   #35
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,190

Submissions (19)

Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

Quote:
Originally Posted by Flame_Phoenix
Actually if you bother enough to see my replay and my screen, you will notice that I got the "double free message" using the test map provided by the creator. I tested the most recent version (at that time) and so I assume that the demo map had the most recent xe (at that time). If anyone has to update something, that someone is not me, but the creator in the demo map, because the error was found in the test map.
I know what a double free error is so I didn't really need to look at any screenshots or replays to know what was going on.

Quote:
On the other hand, if you don't care about that problem, than it is also not my problem as well. The fact is that (at that time) I got an error message I didn't like and I reported in a friendly way. By ignoring my report you risk your credibility as a mod, because if someone tests this spell in the future and if the tester finds the problem again, the tester will think that Anitarf is a nab mod because he approved a spell with a problem.
You didn't consider the possibility that I tested the map before assuming the error was on your side. Apparently, the xe in the spell map (or maybe the spell itself, I don't know and don't care enough to bother finding out) was updated between our tests and this led to the misunderstanding.
__________________
Anitarf is offline   Reply With Quote
Old 04-01-2009, 04:04 AM   #36
FriendlyPsycho
User
 
FriendlyPsycho's Avatar
 
Join Date: Feb 2009
Posts: 109

Submissions (1)

FriendlyPsycho is on a distinguished road (12)

Default

Quote:
Originally Posted by Anitarf
As for the spell, TimedXefx is still an issue. It hasn't yet gone through our review process yet so you can't use it in a spell you submit here. Since it's such a simple library, I'll review it right now: The struct data is useless, you could have just as well attached the xefx to the timer directly. The end result is such a short code that you could just as well inline it into the spell entirely and thus avoid me telling you again that you can't use it.

Right'o, I'll optimize the library. Should I submit it as another resource instead, might come in handy for those who want to use xefx but are stuck with TimedEffects, or maybe should I just tack in the code with moyack's TimedEffects thread?

Again, I'm sorry if this bothers you, but I kinda disagree with inlining it. What if someone wants to use two variations of it? Even though inlining only takes up very few lines, it is still redundant code, and I might as well just use a library for added modularity.

Last edited by FriendlyPsycho : 04-01-2009 at 04:14 AM.
FriendlyPsycho is offline   Reply With Quote
Old 04-01-2009, 04:25 AM   #37
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,190

Submissions (19)

Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

As I said, you should inline it. It's not much of a library once you write it correctly, and it wouldn't get approved as such. Modularity is all nice and good for coding but when it comes to the end user having a ton of libraries in the map can be a pain.
__________________
Anitarf is offline   Reply With Quote
Old 04-01-2009, 04:36 AM   #38
FriendlyPsycho
User
 
FriendlyPsycho's Avatar
 
Join Date: Feb 2009
Posts: 109

Submissions (1)

FriendlyPsycho is on a distinguished road (12)

Default

Ah, now I see your point.

Updated! Sorry for making your job any harder by trying to prove some of my points. My bad :P
FriendlyPsycho is offline   Reply With Quote
Old 04-03-2009, 02:10 AM   #39
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,190

Submissions (19)

Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

Just a few minor issues remaining:
  • There already is a max collision size constant in xebasic, no need to have another one in your spell.
  • You should use a local trigger variable instead of the GUI global, so users don't have to get the trigger name exactly right.
  • tx and ty could have been local variables instead of struct members, which are a bit less efficient.
  • The same is true for the .a member, like tx and ty you calculate it in loopfunc every time so there's no need to store it.
  • With that in mind, the EventTargetLib is kind of unneeded since the only thing it shortened in the spell code was the angle calculation.
  • You forgot to include a IsUnitInRangeXY check in the enum function.
  • You never use temploc. I assume you meant to use it for the IsUnitInRange check but you could just reuse the GG_Distance's members for that instead.
__________________
Anitarf is offline   Reply With Quote
Old 04-03-2009, 04:20 AM   #40
FriendlyPsycho
User
 
FriendlyPsycho's Avatar
 
Join Date: Feb 2009
Posts: 109

Submissions (1)

FriendlyPsycho is on a distinguished road (12)

Default

1.) Ok done.
2.) What GUI global? The gg_trg thing? Well, it is created anyways so I just used that.
3.) Done.
4.) Done.
5.) Removed usage.
6.) Um I don't need to? Pretty sure about that..
7.) Oops, forgot to clean that up heh. Done.

First post/spell updated to conform to the aforementioned stuff.
__________________
" YOU RAGE, YOU LOSE! "
FriendlyPsycho is offline   Reply With Quote
Old 04-03-2009, 05:21 AM   #41
Vexorian
Free Software Terrorist
 
Vexorian's Avatar


Technical Director
 
Join Date: Apr 2003
Posts: 14,898

Submissions (37)

Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)Vexorian has a reputation beyond repute (1062)

Hero Contest #3 - 2nd Place

Default

Quote:
2.) What GUI global? The gg_trg thing? Well, it is created anyways so I just used that.
smells like bad scoping.

These gg stuff and for example not using a initializer but calling InitTrig are all wrong, they are GUI-dependent, we can get better than that... relying on such a variable or InitTrig just makes your code less portable, it also makes it harder to use the template twice (a la JESP ...)
__________________
Zoom (requires log in)Wc3 map optimizer 5.0
Someone should fix .wav sound in this thing.
Zoom (requires log in)JassHelper 0.A.2.A
Turns your simple code into something that is complicated enough to work.
Faster != more useful
Vexorian is offline   Reply With Quote
Old 04-03-2009, 05:59 AM   #42
FriendlyPsycho
User
 
FriendlyPsycho's Avatar
 
Join Date: Feb 2009
Posts: 109

Submissions (1)

FriendlyPsycho is on a distinguished road (12)

Default

I do use an initializer, scope Overgrowth initializer init
__________________
" YOU RAGE, YOU LOSE! "

Last edited by FriendlyPsycho : 04-03-2009 at 06:00 AM.
FriendlyPsycho is offline   Reply With Quote
Old 04-05-2009, 11:12 PM   #43
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,190

Submissions (19)

Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

Quote:
Originally Posted by FriendlyPsycho
I do use an initializer, scope Overgrowth initializer init
He didn't say you weren't using it, it was just an example:
Quote:
Originally Posted by Vexorian
These gg stuff and for example not using a initializer but calling InitTrig are all wrong,
And he's right. The variable is only created if you have a GUI trigger named exactly the same as the spell's scope name, which is an entirely unnecessary implementation requirement.

Quote:
Originally Posted by FriendlyPsycho
6.) Um I don't need to? Pretty sure about that..
I assume you're trying to get spell like targetting where units are affected as long as their collision size is within the given radius, at least that's the only reason I see for enuming units within (radius+max collision size) range. If that's what you want, then you need a IsUnitInRangeXY check, since the enum may pick units that have their entire collision size outside the given radius, as long as their center is within (radius+max collision size).
__________________
Anitarf is offline   Reply With Quote
Old 04-06-2009, 05:10 AM   #44
FriendlyPsycho
User
 
FriendlyPsycho's Avatar
 
Join Date: Feb 2009
Posts: 109

Submissions (1)

FriendlyPsycho is on a distinguished road (12)

Default

Ok then. Updated the first post.
__________________
" YOU RAGE, YOU LOSE! "
FriendlyPsycho is offline   Reply With Quote
Old 04-06-2009, 08:51 PM   #45
Anitarf
Procrastination Incarnate


Development Director
 
Join Date: Feb 2004
Posts: 8,190

Submissions (19)

Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)Anitarf has a brilliant future (903)

2008 Spell olympics - Fire - SilverApproved Map: Old School Alliance TacticsHero Contest #2 - 3rd PlaceSpell making session 2 winner

Default

The IsUnitInRangeXY check should be in the enum function, not the TargetsAllowed function. There's no need for this condition to be calibrateable, it's an entirely technical part of the code that shouldn't be in the calibration section. Also, like I said, there's no need for special variables to pass the x and y coordinates to the enum function when you could easily use the GG_Distance's x and y members to do that.

The rest looks good.
__________________
Anitarf is offline   Reply With Quote
Reply


Thread Tools Search this Thread
Search this Thread:

Advanced Search

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

vB code is On
Smilies are On
[IMG] code is On
HTML code is Off


All times are GMT. The time now is 08:04 PM.


Affiliates
The Hubb The JASS Vault Clan WEnW Campaign Creations Clan CBS GamesModding Flixreel Videos

Powered by vBulletin (Copyright ©2000 - 2019, Jelsoft Enterprises Ltd).
Hosted by www.OICcam.com
IT Support and Services provided by Executive IT Services