System Zwiebelchen's Casting Bar System

[Zwiebelchen]

This is your chance to learn something, to better yourself. All the comments you have gotten should be viewed as constructive criticism.

I learned something of it. But most of the things mentioned here I already knew, like the BJ thingy.

Also you are never ever going to get a system approved that is not efficient, those bjs are simply not efficient enough.

I didn't write the code for the Resources here. I am using this in my own map I am currently working on. However, after I finished the Castingbar System, I searched the forums here and realized, that there is no resource on a system like that. So I just uploaded it; maybe I can save someone else a little bit of time.
I know the code is far from being perfect. As I said, I'm not a Pro-Jasser; I just started.
For me, the code is suited. I don't need a 110% efficient code, because the function isn't run that often in my map. Of course there are some things I could have done shorter, faster, better ... but it doesn't matter here really for me, since its not a periodic function and recent computers are so fast the syntax efficiency of WC3 doesn't matter anymore.
As I said, I replaced all of the BJs I found senseless.

Also, I my mind, you should do something about the way you display the texttag, the whole force thingie. There are simpler ways of doing it, and as far as I can tell you have a leak there.

Uh? Leak? Where? I checked it after reading it but couldn't find what you mean. Please post the line you are talking of.


*Sigh* I see what I can do about the BJs and create a demo map.

 

[Flare]

It's a pro because not every mapmaker out here is familiar with JASS.

Most systems are relatively friendly to GUI users anyway (apart from, for example, attachment systems since structs aren't available to GUI users). GUI friendliness isn't really a pro, rather a secondary benefit. If someone isn't in any way familiar with JASS, then they have little business using something cioded in JASS.

Anyway, from the way you've posted it, you make it sound like the code is in GUI (for anyone that hasn't actually looked at it). Might be a good idea to change it to something more correct (e.g. GUI-friendly), or remove it.

when will you ever need thousands of castbars on the same time?

You're never going to be able to have thousands of castbars at a time anyway, since there's a limit of 100 text tags in existence at a time, I think

It's gonna be laggy because of the loop and smallscale timers anyway then.

It could be laggier with far fewer instances, and a much more realisible number at that - BJ's can make a hell of a difference when it comes to your code being a source of lag, or lag-free. For example, one particular spell I made, in GUI, worked lag-free up to ~6 instances - as you probably know, GUI actions are, for the most part, consisting of BJ functions. When I remade that spell in vJASS, while taking particular care for trying to get maximum efficiency (primarily through minimal BJ usage), I am able to get 30+ instances running, simultaneously, without seeing any lag (was maintaining about 55-60FPS while spamming).

AoS maps would be able to get 12-15 (if it was 6vs6, and if there was some AI/neutral units around), RPG's could be getting in the region of 20 instances, particularly with boss units/AI-controlled heroes. With that many instances, lag is entirely possible, and those are all plausible situations

    local force TextTagAllPlayers = GetPlayersAll()

    call DestroyForce(TextTagAllPlayers)

1) Why not just use the variable that GetPlayersAll returns?
2) Remove that DestroyForce line - by destroying TextTagAllPlayers, you are destroying the (All players) force. TextTagAllPlayers is pointing to the handle that GetPlayersAll returns, so if you destroy TextTagAllPlayers, you destroy (All players) and vice versa.

    local force TextTagForce = GetForceOfPlayer(GetOwningPlayer(Caster))

If ShowCastingBar == 1, you've just caused a leak, since you are overriding the value contained in TextTagForce, thus losing the reference to the previously stored handle.

 

[Zwiebelchen]

I replaced all of the BJs with natives. I hope it's fine now. :thup:

Also added a Demo Map and removed the leak in the force thingy.

I hope its satisfying enough now

 

[Flare]

You're still destroying bj_FORCE_ALL_PLAYERS

Remove this line:

    call DestroyForce(TextTagAllPlayers)

Replace all instances of TextTagAllPlayers in your Castingbar function with bj_FORCE_ALL_PLAYERS

Remove this line:

    local force TextTagAllPlayers = bj_FORCE_ALL_PLAYERS

Also...

local force TextTagForce = CreateForce()

    if ShowCastingBar == 1 then
        set TextTagForce = GetPlayersAllies(LocalPlayer)
    endif

GetPlayersAllies creates a new force handle, so when ShowCastingBar == 1, you're still leaking

    if ShowCastingBar == 2 then
        set TextTagForce = TextTagAllPlayers
    endif

    call DestroyForce(TextTagForce)
    call DestroyForce(TextTagAllPlayers)

>_< Again, destroying bj_FORCE_ALL_PLAYERS and causing a double-free (i.e. destroying the same handle twice)

 

[Zwiebelchen]

Reworked the force thing again. Should work now.

Also updated the Testmap.

 

[Steel]

A lot of these floating bar systems seem to stem from my initial design and I'm absent from credit, hmm...if you didn't take a look at it you have a lot of things you need to check for.

-There is a limit to the number of text tags the map can create.
-System is not efficient using multiple timers. Each creation of a casting bar requires a new timer. Take the hypothetical situation of 1000 units all cast a spell at once. 1000 timers are created...
-Use a method for your creation, will simplify your code and reduce the number of calls you need to worry about.
-You NEVER use DestroyTextTag. You only nullify them.

Heres what this means

local texttag tt
[tt]-> Null
set tt = CreateTextTag()
[tt]->(Some Created TextTag by the System)

set tt = null
[tt]-> Null //The allocation of tt isn't destroy, it's just pointing at null

 

[Azlier]

He shuts off permanency and he gives it a lifespan, isn't that enough? Steel makes a point about timers, though. There is also destroying of timers present, which I was told is a very bad thing to do. TT would be perfect in this situation, with its ability to run a whole system (even all the spells in a map, if you only need one timer period).

 

[Zwiebelchen]

-There is a limit to the number of text tags the map can create.

Hm, it's not that much of a problem, I think. It should be a rare case that more than 50 units cast simultanously.

-System is not efficient using multiple timers. Each creation of a casting bar requires a new timer. Take the hypothetical situation of 1000 units all cast a spell at once. 1000 timers are created...

There was no other way to generate a "smooth" filling bar without using multiple timers, since the timer intervall depends on the casting time. For example if you use a 4 second spell and a 2,5 second spell on the same timer, you would have an ugly "jumping" castbar that fills up 2 bits or 0 bits from time to time per tick. It looks far better like this, though I accept that it has some flaws.

-You NEVER use DestroyTextTag. You only nullify them.

I used lifespan settings.

There is also destroying of timers present, which I was told is a very bad thing to do.

Errr, why is that? Last time I checked with the handle counter, the system didn't create leaks, so it should be alright.

TT would be perfect in this situation, with its ability to run a whole system (even all the spells in a map, if you only need one timer period).

As I said, it would look ugly using a fixed timer intervall. And Casting bars are eyecandy after all, so it leads to a contradiction ^^

 

[Flare]

As I said, it would look ugly using a fixed timer intervall. And Casting bars are eyecandy after all, so it leads to a contradiction ^^

Not really - with default configuration for TT, you would be able to get an accurate cast bar for most 'normal' durations (e.g. multiples of 0.5) without any situations where you get 2 'bits' appearing at a time. If people want greater accuracy, they can increase the character count or make the character count a parameter of the function (might be nice for spells with particularly long casting times)

 

[Zwiebelchen]

What if you want to change the casting time of spells smoothly based on the heroes int? TT may work fine with casting times that are multiples of X, but what if you need a good accuracy or a casting time of 2,38?

But I see your point. Switching to TT might be an option, but that would require a total system overhaul ... And seriously, my perfectionism has its limits ...

I guess the system is fine now as it is. Sure, it has it's flaws, but it works, it's leak free and it's not THAT slow.

 

[saw792]

If you can tell the difference between 2.38s and 2.40s, you are a greater man than I.

It really wouldn't take long to convert it to a single timer...

What I wonder is why you would submit something adequate, when you could submit something excellent with a small amount of extra effort. Submissions here don't get approved just because somebody might find it useful; you need to back the usefulness up with efficient code.

 

[Zwiebelchen]

If you can tell the difference between 2.38s and 2.40s, you are a greater man than I.

That just shows that you didn't understand what I meant.

It really wouldn't take long to convert it to a single timer...

Fine.

What I wonder is why you would submit something adequate, when you could submit something excellent with a small amount of extra effort. Submissions here don't get approved just because somebody might find it useful; you need to back the usefulness up with efficient code.

I don't care. I put much more work in improving it, than I actually did to write it. Why the hell you asked me for all those improvements when in the end you just say "The system is crap. Rewrite it completely. Nice try anyway."?
I feel jerked.

 

[CaptDeath]

im srry but you will always spend less time on v1 then you will on improvements it just a fact do you think something that had 300+ versions has spent more time on v1 or on v2-300?

+its still a nice system no matter what any one thinks

 

[saw792]

That just shows that you didn't understand what I meant.

I understood completely. You just showed that you didn't understand what I meant, seeing as you NEVER need that level of accuracy (and your code can easily accomodate running 2.38 as 2.40).

I also never said rewrite your system completely. 95% of your code will stay the same.

Don't blame us (me) for telling you what to improve. I'm not gonna lie and say your code is perfect, nor am I going to refrain from telling you if it can be improved.

 

[Zwiebelchen]

Whatever, I'm not going to change to code again now (except someone find's a bug or leak), because this system has exactly the functionality I need for my RPG. And dispite of what you try to tell me: It's not done with easy copy and paste to convert it to a single timer system and I simply don't have the time to do it, cause I want to focus on my project again.

Thanks for all the comments and suggestions. I learned a lot ^^ (Actually if you want to learn jass, theres nothing better than posting an imperfect system, since this ensures the attention of any jass-nerd out there XD)
Maybe someone still might find it useful.

 

[Bloodcount]

Nice system! Really, looks stunning! I might use it. Anyway, +rep and.... doesn't Zwiebelchen mean something like oniony cuz i know that Zwiebel is onion in German.

 

[saw792]

It's fine if you don't want it approved. I wasn't forcing you to do anything.

Anyway, good luck with your map.

 

[RaiJin]

holy shit that is pretty sick lol

+rep im going to definitely use this in my map

 

[Zwiebelchen]

Looks like its getting popular XD

 

[MasterOfRa]

im not sure how to get this to work, do you have to base the ability off of channel or something because it shows the casting bar, but the rest of the trigger fires right away.