ProgressBars v2.0.1

[Moderator]

18th Apr 2016

Your resource has been reviewed by BPower.
In case of any questions or for reconfirming the moderator's rating,
please make use of the Quick Reply function of this thread.

Review:

A wonderful system and definitly worth a 5/5 rating.
Recycling bar dummies is a very welcome bonus.

Why is BoundSentinel in the demo map?

Troubleshooting:

• The hardcoded timer timeout in method setPercentage could be replaced by a descriptive constant.
  ---
• Each time you release a timer you should null that timer handle, because of code blocks like:
  

  if (this.timer == null) then
      set this.timer = NewTimerEx(this)
  endif

  It implies that the released timer is still reserved for your system, which is not true.
  TimerUtils probably gave the timer out for another code running.
  It's very likely because the timer stack always returns the last recycled timer.
  ---
• if (this.target != null) then could preferably be if GetUnitTypeId(target) != 0 then

Review changelog:

1. -

 

[Rheiko]

cool, but what is progress bar for?
something like exp/hp/mana bar?

 

[Chaosy]

cast bar for spells I guess

 

[Rheiko]

that's the first thing that comes to my mind..

 

[TriggerHappy]

It can be used for anything you want.

 

[BPower]

The progress bar model is just really nice .

The dummy needs locust.

version 0.01 ?? rather 1.0.0

Edit:
timer timer Why ....

private static method getDummy takes real x, real y returns unit You don't use x and y at all.

I thought method onDestroy should be avoided. Can't argue about this, its just something I read, but I think you just should write method destroy on your own.

Also in onDestroy you do call SetUnitAnimationByIndex(this.bar, 0) which looks ugly in your demomap, in case you cancel the spell.
I would recommend using call SetUnitPosition(this.bar, 2147483647, 2147483647) before. ( or any really high x/y value)

You could use 0.031250000 instead of 0.03, it would be more precise.

I would create new dummies at the edge of the map, just use SetUnitPosition and a hillarious high x and y value, because if it is on 0,0 it will be displayed there or at its current position for a millisecond before it gets moved above the targetunit. (You can experience it in your demo map)
When recycling the dummy place him back to the edge of the map.

You could mention your demo spell leaks and should not be copied. ^^

If it gets approved, I'll use it as optional library for my TownPortalSystem.

 

[Chaosy]

I think you could do this pretty simple by just having a model with an 1 second duration animation. Then set the animation to speed to lets say 50% as a cast bar. I think there is a system that works that way,

 

[BPower]

I think you could do this pretty simple by just having a model with an 1 second duration animation. Then set the animation to speed to lets say 50% as a cast bar. I think there is a system that works that way,

No you can't.

 

[deathismyfriend]

version 0.01 ?? rather 1.0.0

Edit:
timer timer Why ....

I thought method onDestroy should be avoided. Can't argue about this, its just something I read, but I think you just should write method destroy on your own.

Also in onDestroy you do call SetUnitAnimationByIndex(this.bar, 0) which looks ugly in your demomap, in case you cancel the spell.
I would recommend using call SetUnitPosition(this.bar, 2147483647, 2147483647) before. ( or any really high x/y value)

You could use 0.031250000 instead of 0.03, it would be more precise.

I would create new dummies at the edge of the map, just use SetUnitPosition and a hillarious high x and y value, because if it is on 0,0 it will be displayed there or at its current position for a millisecond before it gets moved above the targetunit. (You can experience it in your demo map)
When recycling the dummy place him back to the edge of the map.

Going off of the above post.

This should not have been submitted unless it is verion 1.0.0 or higher.

timer timer should be avoided.

onDestroy should never be used create your own destroy method.
Look at Magtheridon's Struct for Dummies tutorial i believe the exact reason is in there. Also don't use SetUnitPosition. It is slower and less efficient than the SetUnitX/Y counterparts.

0.031250000 should be used. It is used by almost all jassers / vJassers

SetUnitX/Y with a hide unit and move then show unit will fix this problem.

 

[BPower]

SetUnitX/Y is no option, because you don't know how large the map is. You could easily move the unit outside mapbounds and crash the game. That's why you have to use SetUnitPosition.

Nestharus did it in his Dummy library and I think its a good solution.

 

[TriggerHappy]

timer timerWhy ....

Becausethis.timerlooks nice (in TESH), leave me alone.

private static method getDummy takes real x, real y returns unit You don't use x and y at all.

Thanks for pointing that out, I left that there on accident.

I thought method onDestroy should be avoided. Can't argue about this, its just something I read, but I think you just should write method destroy on your own.

onDestroy is fine.

You could use 0.031250000 instead of 0.03, it would be more precise.

I personally use 0.01, I only keep it at 0.03 to get approved here.

Also in onDestroy you do call SetUnitAnimationByIndex(this.bar, 0) which looks ugly in your demomap, in case you cancel the spell.

What's wrong with the death animation? How does that look ugly. The only reason it looks ugly (has nothing to do with the line you pointed out) when you cancel the spell is because it doesn't follow the unit, but I'll work on that.

This should not have been submitted unless it is verion 1.0.0 or higher.

Jesus christ, the version number is really just cosmetic. The system works fine you're really saying I shouldn't submit this because I titled it 0.01? Get it together lol.

Anyway changed version number to 1.00 (will update doc and map in next release) and uploaded a video.

SetUnitX/Y is no option, because you don't know how large the map is. You could easily move the unit outside mapbounds and crash the game. That's why you have to use SetUnitPosition.

I refuse to use SetUnitPosition, or even path check for that matter (in this lightweight system). I will consider bound sentinel as an optional dependency.

 

[deathismyfriend]

Jesus christ, the version number is really just cosmetic. The system works fine you're really saying I shouldn't submit this because I titled it 0.01? Get it together lol.

Anyway changed version number to 1.00 (will update doc and map in next release) and uploaded a video.

I refuse to use SetUnitPosition, or even path check for that matter (in this lightweight system). I will consider bound sentinel as an optional dependency.

It is in the rules for the spell submission section. Do not submit anything under version 1.0

Use the world bounds library to detect when unit is leaving map area.

 

[Anachron]

First off: I like the idea of progress bars like this in game and like that there is finally a library for it.

My questions/ideas:
Why did you add + 0.1 to the time? Isn't 0.01 enough?
You should change the name of the timer to clock or something more appropriate.
You can add optional ARGB support. (Vexorians Version)
Why is t_enabled on default visibility? Shouldn't it be private?
Last but not least, the demo really sucks. Make it a spell with start delay, one over time (rain of fire / blizzard) and maybe some hp or mana bar for another unit.

Edit: The show method is funny

You can change this

        method show takes boolean flag returns nothing
            if (flag == false) then
                set this.hidden = true
                call ShowUnit(this.bar, false)
            elseif (hidden) then
                set this.hidden = false
                call ShowUnit(this.bar, true)
            endif
        endmethod

to this

        method show takes boolean flag returns nothing
            set this.hidden = not flag
            call ShowUnit(this.bar, flag)
        endmethod

It's okay if it's already shown, it won't flicker or anything.

 

[TriggerHappy]

Use the world bounds library to detect when unit is leaving map area.

Why would I constantly check for safe coordinates (every 0.03 seconds) when I could just use bound sentinel?

Why is t_enabled on default visibility? Shouldn't it be private?

Nice catch, thanks.

The show method is funny

That's what coding at 2am gets you

Thanks.

I'll be updating in a little, right now the progression formula is a bit wrong.

Updated, however there is still an issue with the AnimationSpeed formula in that over long a duration it becomes slightly more inaccurate.

Also please nobody complain about the bar not being destroyed when the demo spell is interrupted, I'll have the example working in the next release.

 

[Malhorne]

if (this.t_enabled == false) then
->
if not this.t_enabled then

Anyway that looks good I'll review better tomorrow if I got some time
+rep anyway !

EDIT : Need to spread :/

 

[Anachron]

I would make these private:

        boolean t_enabled = false
       
        real finishTime = -1
        real ticks = 0

And make a method operator (Getter) as "elapsed / remaining".

 

[TriggerHappy]

if (this.t_enabled == false) then
->
if not this.t_enabled then

Anyway that looks good I'll review better tomorrow if I got some time
+rep anyway !

EDIT : Need to spread :/

The not operator is slower than ==.

I would make these private:

        boolean t_enabled = false
       
        real finishTime = -1
        real ticks = 0

And make a method operator (Getter) as "elapsed / remaining".

True

 

[Malhorne]

The not is slower ?
Seriously ?
Thanks for the info

 

[TriggerHappy]

The not is slower ?
Seriously ?
Thanks for the info

Actually, I guess that was a myth.

I'll update the benchmarking thread and this accordingly.

 

[JesusHipster]

Hey, it's awesome.

 

[Maker]

The show method can potentionally critically harm the system an make it malfunction. Hiding and then showing a unit removes some of the effects of Locust, for example it will be picked with GroupEnumUnitsInRange and it will make the unit drag selectable. The unit will show the selection circle when the drag box is over the unit.

A working solution is to remove/add Locust.

 

[BPower]

Edit: The required fix is mentioned in Maker's post above, not this post .

            if (this.timer != null) then
                if (this.t_enabled) then
                    call PauseTimer(this.timer)
                endif
                call ReleaseTimer(this.timer)
            endif

ReleaseTimer will either pause or destroy the timer. You don't have to do it on your own.
Furthermore it will check if you pass in a null timer and throw a debug warning.

Edit: The only reason I could see, is that first pausing before destroying a timer reduces the chance of creating a bug. (If such a bug does exist at all)

 

[Maker]

The show method can potentionally critically harm the system an make it malfunction. Hiding and then showing a unit removes some of the effects of Locust, for example it will be picked with GroupEnumUnitsInRange and it will make the unit drag selectable. The unit will show the selection circle when the drag box is over the unit.

A working solution is to remove/add Locust.

Could you apply a fix to this? Let's get the resource approved.

 

[Kazeon]

What is those library blablabla requires blahblah and static method blahblah for? Learning jass is so driving me crazy x.X

 

[Malhorne]

This is not a place to ask for help about vJASS.
Please use WEHZ or T&S to do so....

 

[Justicebringer]

Neat system!

 

[Xarwin]

Great idea and system, thanks for sharing dude.

 

[Foopad]

This could have so many uses. Great upload

 

[TriggerHappy]

I'm going to be updating soon, sorry for the delay.

 

[Banti]

I like your system but it doesn't seems working for summoning spells (casting time before something is summoned). When I use the spell progress bar is showing after the spell is finished but for spell like rain of fire is good

 

[JesusHipster]

Nice!

 

[TriggerHappy]

I'm going to be updating this to version 2.0 soon.

 

[TriggerHappy]

I've updated to 2.0.0.

It's in beta because the code can probably be simplified and I was having some issues with my HP/MP bars, however I don't think it was an issue directly with the script.

Basically you can set the progress bars % now via.setPercentage

v2.0.0
- beginProgression and everything associated with it has been removed. Use .setPercentage instead.
- zOffset getter included.
- xOffset and yOffset added.
- show method now adds and removes locust.

 

[TheGoldenGoblin]

Yea the hp bar regenerates itself But I think I found another bug if the unit dies..

 

[BPower]

Yea the hp bar regenerates itself But I think I found another bug if the unit dies..

No no, the demo doesn't check for the units health condition. Everything works very well.

@TriggerHappy: Once you release a timer null the timer reference, otherwise you might re-start a timer which is currently running for a different library.
The timer will be pushed back into the TimerUtils timer stack and may be used everywhere.


That timer could also be destroyed (does this really happen? Ever?), as TimerUtils destroys timers once the max array size is reached.

 

[JesusHipster]

I like it how you took after that small hp\mp testmap's way of using bars. Good job.

 

[TriggerHappy]

I like it how you took after that small hp\mp testmap's way of using bars. Good job.

Yes, I didn't consider pausing/reversing the animation.

You damn Russians!

 

[TriggerHappy]

Version 2.0.0 released!

Demo now includes a functional HP/MP bar example.

v2.0.0
- Default health bar hidden in demo.
- ProgressBars will now no longer fill up out of nowhere.
- Fixed a possible, rare leak associated with not nulling a timer.

 

[BPower]

Yesterday I've read the introduction in Missle Recycler from noones favorite Avenger Captain Bri.. America and apperently "paused units consume very few CPU resources".
You should consider adding PauseUnit(...) into getDummy. I think the best way is to set the created unit to bj_lastCreatedUnit and return bj_lastCreatedUnit to prevent a possible unit handle leak.
Users still can mess around with any visuals althought the unit is paused.


This makes no sense it should be the other way around. I just read this is the description and didn't download the map. Is the code in there outdated?

            set this.timer  = null
            set this.timer2 = null
           
            if (this.timer != null) then
                call ReleaseTimer(this.timer)
            endif
           
            if (this.timer2 != null) then
                call ReleaseTimer(this.timer2)
            endif

Edit: The code inside the map file doesn't match with the desciption, both need an update.

Captain Bribe would be a big win for the Avengers

 

[TriggerHappy]

Updated.

v2.0.1
- Added support for different types of progress bars (without recycling) via .createEx(integer unitId).
- Properly nulled the timer handles.
- Paused progress bar dummies to reduce CPU usage.

 

[SinnerPo]

Very cool system,but why not make the bar is automatically created for units,which will appear on the map only with time?I don't know vJass,and now faces that can't implement it myself.
And..bar does not want to disappear after the death of unit

 

[Malhorne]

It is a bad idea to make them pop automatically because it could lag and it will create too much effects.
You surely want those only for boss...

 

[TriggerHappy]

It is a bad idea to make them pop automatically because it could lag and it will create too much effects.
You surely want those only for boss...

Not true.

This system should be able to handle hundreds of bars (if not more).

 

[Malhorne]

That's pretty nice but anyway I don't think the idea of putting a bar for every unit is a good one.

 

[TriggerHappy]

That's pretty nice but anyway I don't think the idea of putting a bar for every unit is a good one.

I don't really agree.

Anyway just putting that out there that this system should not lag that easily..

 

[TriggerHappy]

How about RPG maps or times where there are over 30 units . It would cause lag for sure. Please make it optional for example only heros.

I just said two posts ago it should support hundreds of bars.

The next version will likely just be an optimization improvement which should allow even more bars than the current version.

 

[BPower]

To all you killjoys out there.

Unless your hardware is pre-millenium this system will not cause any fps drop you could even notice.

Don't believe me? Go play pudge wars. Every hook consists out of ~50 - 150 links in a 12 player game.

 

[meowie]

Nevertheless, Great system. But its need more samples documentation and option
For example : height of the dummy model as some models may be different in scale.
, specifically units option documentation or maybe in GUI custom script

 

[TriggerHappy]

Nevertheless, Great system. But its need more samples documentation and option
For example : height of the dummy model as some models may be different in scale.
, specifically units option documentation or maybe in GUI custom script

Yeah the documentation clearly includes all of that, except for GUI support which would be ridiculous to include in the doc.

*   Options:
*       x            - set X coordinate
*       y            - set Y coordinate
*       xOffset      - offset of the target unit, if any.
*       yOffset      - offset of the target unit, if any.
*       zOffset      - how high the bar is from the ground.
*       color        - allows you to tint the bar or add transparency
*       targetUnit   - pick which unit the bar should hover over
*       size         - set model scale

or more specifically, you were complaining about not being able to set the height of the model

local ProgressBar bar = ProgressBar.create()
set bar.zOffset = 100

I do agree that more samples could be included, but this is very straight forward to use and the API is simplistic.

More than enough information is embedded into the current examples.

Please stop putting negativity out there about my system when it's not even true.