Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#1187 closed enhancement (fixed)

[PATCH] Hotkey to display all health bars

Reported by: historic_bruno Owned by: Deiz
Priority: Should Have Milestone: Alpha 11
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description

A hotkey pressed to show health bars for all units on screen at once. This would be handy during battles so you could quickly see the health of all units without needing to select them. The hotkey could be either a toggle or active only when held down.

Related and perhaps unnecessary feature: show health of units with recently changed hitpoints, i.e. those being healed or damaged. If implemented, it should be a config option and separate from the "show all" feature.

Attachments (11)

Ticket#1187.patch (46.8 KB ) - added by James 12 years ago.
A Patch to allow Tab key to show (while depressed) status bars for all friendly units on screen. (hopefully)
Ticket#1187_V2.patch (2.5 KB ) - added by James 12 years ago.
Attempt #2, hopefully minus some obvious flaws
Ticket#1187_V3.patch (3.1 KB ) - added by James 12 years ago.
Fixed so that correct units have status bars removed - still to allow for new units entering screen
Ticket#1187_V4.patch (3.1 KB ) - added by James 12 years ago.
This one works quite well, allowing for new units entering screen, but is a little bit hackish (not pretty)
Ticket#1187_V5.patch (3.8 KB ) - added by James 12 years ago.
Works well except function (in order to be called every frame) is called in update cursor function so stops working when cursor is over a resource site.
Ticket#1187_V6.patch (3.8 KB ) - added by James 12 years ago.
I really can't foresee any problems but feel free to point out flaws when you find them
Ticket#1187_V6.2.patch (3.7 KB ) - added by James 12 years ago.
or maybe this one…
health_bar_hotkey_v01.patch (3.4 KB ) - added by Kieran P 12 years ago.
health_bar_hotkey_v02.patch (3.4 KB ) - added by Kieran P 12 years ago.
healthbars.patch (3.7 KB ) - added by Deiz 12 years ago.
healthbars.2.patch (3.8 KB ) - added by Kieran P 12 years ago.

Download all attachments as: .zip

Change History (54)

comment:1 by James, 12 years ago

OK, I was thinking of creating my own RTS when a friend suggested that I try this open source project. I thought I would start out with something easy and not too important but I can't seem to get started. I have managed to find the javascript that handles the health of a unit (but not where the health bars are actually drawn) and the cfg file that has hotkeys and the hotkeys.cpp file that handles them, but I can't find where they are implemented after that. I would really appreciate some help getting started - as you may be able to tell this is my first attempt at an open source project...

comment:2 by Jonathan Waller, 12 years ago

To get started I think you will need to look at public/gui/session/input.js to handle the hotkey press. Then the status bars are handled in public/simulation/components/StatusBars.js. Then in order to communicate between the two components public/simulation/components/GUIInterface.js is used. In GUIInterface there is already a GuiInterface.prototype.SetStatusBars() function, this should help show you how to create a new one.

The game has a limited interface between simulation components and the rest of the engine in order to be able to save games and synchronize multiplayer games across a network, which is why GUIInterface is needed to communicate. For this ticket you shouldn't have problems though since the StatusBars component doesn't get network synchronized.

The code is quite intimidating at first, but it is fairly well structured generally, so after a while you will get used to it. input.js is one of the less friendly pieces of code unfortunately.

IRC normally has someone helpful on it if you want to ask questions, the 0ad-dev channel is the place to go for technical questions.

comment:3 by James, 12 years ago

Thanks, that has helped a lot. I'll get straight on it.

comment:4 by James, 12 years ago

Well, that's a good first step: I have the hotkey working (I know it's the easy part but it's something). Now I just have to figure out how to find out which friendly units are on screen and we're in business. As always help is welcome, there is a lot of code to sift through.

comment:5 by James, 12 years ago

Owner: set to James
Status: newassigned

comment:6 by vts, 12 years ago

Hi there! We currently have some functions that almost do what you want, but not quite. Therefore, in [11230] I've added a function called PickFriendlyEntitiesOnScreen that does this and exposed it to the JS side for you to use. After you update from SVN, you should be able to use it from any of the GUI scripts (i.e. anything under binaries/data/mods/public/gui) like so:

var ents = Engine.PickFriendlyEntitiesOnScreen(Engine.GetPlayerID());

This returns a list of entity IDs that are friendly to the current player and that are visible on the current screen.

Hit us up on #0ad-dev on quakenet by the way, most developer discussion happens there and we'll be glad to help you out :)

Last edited 12 years ago by vts (previous) (diff)

comment:7 by vts, 12 years ago

Quantumstate raised a valid point that I completely overlooked in my eagerness to add this; wouldn't you want the health bars of all entities on screen to be visible, not just your own? :)

comment:8 by michael, 12 years ago

A player may want that, but should a player be allowed that much info about the health of the enemy? Relatedly, I don't like how you can see how many resources an enemy worker is carrying.

Last edited 12 years ago by michael (previous) (diff)

in reply to:  8 comment:9 by historic_bruno, 12 years ago

Replying to michael:

Relatedly, I don't like how you can see how many resources an enemy worker is carrying.

I can see both sides of that argument. In the real world, if an enemy were in LOS, wouldn't you notice what they were carrying and how much? :) Unless they were trying to conceal it, but is there any real advantage to that?

by James, 12 years ago

Attachment: Ticket#1187.patch added

A Patch to allow Tab key to show (while depressed) status bars for all friendly units on screen. (hopefully)

comment:10 by James, 12 years ago

Keywords: review added; simple removed
Milestone: BacklogAlpha 9
Resolution: worksforme
Status: assignedclosed

comment:11 by James, 12 years ago

Resolution: worksforme
Status: closedreopened
Summary: Hotkey to display all health bars[Patch]Hotkey to display all health bars

comment:12 by James, 12 years ago

Well, I think that has sorted it - thanks for all the help.

comment:13 by James, 12 years ago

Status: reopenednew

comment:14 by Jonathan Waller, 12 years ago

Something seems to have gone wrong when you were generating your patch. The GuiInterface file shows removing the whole files contents and replacing it making it imporssible to spot the changes from the diff. Could you try regenerating it? It might be the line endings messing up.

Also you have a typo in input.js line 589.

I think there is a bug with your code, you use PickFriendlyEntitiesOnScreen() when the hotkey is pressed down and then you also call it again when the hotkey is released. Some units might move off screen in the meantime so these would be left with visible status bars. Also you might want to allow for units moving onto and off the screen while the key is being held down, this would be a more complex change.

comment:15 by James, 12 years ago

Thanks for calling me up on that - that typo was just embarrassing. I see what you mean about the bug and have declared units on screen as a variable elsewhere so that it is available for both functions. The reason I didn't notice the flaw and indeed the lack of ability to add units entering screen is that PickFriendlyEntitiesOnScreen?() seems to select all friendly entities - not just those on the screen. Namely that as far as I can tell, it already does both those things...

by James, 12 years ago

Attachment: Ticket#1187_V2.patch added

Attempt #2, hopefully minus some obvious flaws

in reply to:  15 comment:16 by vts, 12 years ago

Replying to JimmyJ:

The reason I didn't notice the flaw and indeed the lack of ability to add units entering screen is that PickFriendlyEntitiesOnScreen?() seems to select all friendly entities - not just those on the screen. Namely that as far as I can tell, it already does both those things...

Can you clarify a bit on this? How did you observe this behaviour? I can't seem to reproduce it here. I tried moving the camera so only a few units are on screen, holding down tab, and while holding it down moving the camera back so some other units are back on screen; only the health bars for those that were initially on screen remained visible, which seems to suggest that it did indeed only select the ones on screen.

comment:17 by James, 12 years ago

Keywords: review removed

I observed this bahaviour using the same technique as you, but now it only works intermittently. It appears that certain events, such as mouse movement and sometimes clicks cause the function to reselect units on screen - also meaning that units aren't deselected sometimes in my second Patch. I am changing the keyword from review since clearly this is going to require more thought than I anticipated. I'm sorry I was a bit over zealous in submitting my patch.

comment:18 by Jonathan Waller, 12 years ago

I am happy with people submitting patches early, it is much better than when people start working and never upload anything and then vanish after a while so we lose all of their work. Also since the point of the review process is to pick up issues like this, you would expect to have to make some changes.

comment:19 by James, 12 years ago

Keywords: review added

by James, 12 years ago

Attachment: Ticket#1187_V3.patch added

Fixed so that correct units have status bars removed - still to allow for new units entering screen

by James, 12 years ago

Attachment: Ticket#1187_V4.patch added

This one works quite well, allowing for new units entering screen, but is a little bit hackish (not pretty)

comment:20 by James, 12 years ago

I have tried to test this as best I could - I think it works and can see no reason why it would not...

comment:21 by vts, 12 years ago

Seems to work ok for me. Glad you made V4; in V3, hovering over units while holding down tab would cause their health bar to disappear for a moment, but it looks like you fixed that in V4 :)

Haven't had a chance to look at the code in detail yet; will do that as soon as I (or somebody else) can, but looking good so far.

comment:22 by Kieran P, 12 years ago

Summary: [Patch]Hotkey to display all health bars[PATCH] Hotkey to display all health bars

comment:23 by Jonathan Waller, 12 years ago

Your method of coping with updates doesn't seem very nice. You end up with a data structure containing a very large number of duplicate entries. I think it would be cleaner to maintain a list by doing some work to not add duplicate entries. This would also spread the cost over a time period rather than doing a whole lot of work at once, though I don't know how good performance is so this might not be noticeable.

comment:24 by James, 12 years ago

Thanks - I am aware that my solution was a bit hackish, but I wanted to get it working, thinking that I could clean it up a bit later. I'm working on that now.

comment:25 by James, 12 years ago

Right, I think that is better - I still have an array, but it only contains the units on screen. Also, only the health bars of units off screen are removed so even at extremely slow frame rates the health bars shouldn't flicker.

by James, 12 years ago

Attachment: Ticket#1187_V5.patch added

Works well except function (in order to be called every frame) is called in update cursor function so stops working when cursor is over a resource site.

comment:26 by James, 12 years ago

Really now all I need is a way of checking for new entities on screen every frame. At the moment it is inside UpdateCursor(), but that only gets called when the cursor is not over a resource site.

comment:27 by Kieran P, 12 years ago

Milestone: Alpha 9Alpha 10

comment:28 by James, 12 years ago

Keywords: review removed

comment:29 by James, 12 years ago

Keywords: review added

by James, 12 years ago

Attachment: Ticket#1187_V6.patch added

I really can't foresee any problems but feel free to point out flaws when you find them

by James, 12 years ago

Attachment: Ticket#1187_V6.2.patch added

or maybe this one...

comment:30 by Kieran P, 12 years ago

Keywords: patch added
Priority: If Time PermitsNice to Have

comment:31 by Kieran P, 12 years ago

Priority: Nice to HaveShould Have

comment:32 by Jonathan Waller, 12 years ago

Looking at your latest patch there are problems with your allStatusBars() function.

You should cache the result of temp.indexOf(entsOnScreen[ent]) for efficiency rather than having to call it twice.

Checking if(temp.length == 1 && temp[0] == entsOnScreen[ent]) is unnecessary since splice will work fine without it, your method actually leads to you handing bad parameters to splice since you wipe the array and then call splice on it (there is no error message for this though).

You have duplicated for (var ent in g_Selection.toList()) {...}

by Kieran P, 12 years ago

Attachment: health_bar_hotkey_v01.patch added

comment:33 by Kieran P, 12 years ago

Owner: changed from James to Kieran P

Attached a modified version of JimmyJ's work. It's a bit cleaner, better variable and function names, extracted the splice code into a reusable function, and more consistent indentation.

comment:34 by Jonathan Waller, 12 years ago

With the Splice function, since any Javascript object is passed by reference, you modify the parameter as well as returning the modification. Conventionally functions only do one of these things. And the function should be renamed as mentioned on irc.

by Kieran P, 12 years ago

Attachment: health_bar_hotkey_v02.patch added

comment:35 by Kieran P, 12 years ago

Attached another patch. Made us of the fact that the variable passed in is modified directly, and renamed the splice method to array_splice. If it still isn't clear, feel free to suggest a new name.

comment:36 by Kieran P, 12 years ago

From Philip on IRC.

"better to add a new g_Selection.setHealthBarDisplayList() which acts kind of like setHighlightList and interacts properly with the selected/highlighted lists"

Will work on the changes soon.

comment:37 by Kieran P, 12 years ago

Keywords: review removed
Owner: changed from Kieran P to James

The changes required for this are beyond my available time :-(

@JimmyJ Your changes work and thank you for your work. I spoke with a more experience developer on the team and hooking into the updateCursorAndTooltip function isn't the way to go.

His comment was: "better to add a new g_Selection.setHealthBarDisplayList() which acts kind of like setHighlightList and interacts properly with the selected/highlighted lists".

If you want to have another crack at the patch, please feel free to do so.

comment:38 by Kieran P, 12 years ago

Milestone: Alpha 10Alpha 11

comment:39 by James, 12 years ago

Owner: James removed

comment:40 by Deiz, 12 years ago

Keywords: review added
Owner: set to Deiz
Status: newassigned

I don't believe this can be done truly elegantly, because there's no way to check whether a formerly-on-screen unit is still on screen, without intersecting the arrays, which is potentially much slower than just doing an O(n) iteration over the currently-on-screen entities.

cmpStatusBars.SetEnabled() returns early if a bar's state isn't being changed, anyhow. Thus, my method is to try to enable health bars for all of a player's on-screen units every simulation update. I never actually check for units being outside the screen, as that's bound to be slower than the simpler approach. Instead, the only "complex" check occurs when the hotkey is released, at which point health bars are disabled for all selected and highlighted units.

by Deiz, 12 years ago

Attachment: healthbars.patch added

comment:41 by historic_bruno, 12 years ago

What about the rendering cost? I would like to think it won't render off-screen overlays but I don't know if it does culling... that could offset the gain from not sorting offscreen units. So it's worth checking into.

I would change the terms in your patch from "health" to "status" for consistency, since it's really just toggling status bars of unknown function.

Unfortunately it's all conflicted now, but luckily a short patch so it's not too painful.

Last edited 12 years ago by historic_bruno (previous) (diff)

by Kieran P, 12 years ago

Attachment: healthbars.2.patch added

comment:42 by ben, 12 years ago

Resolution: fixed
Status: assignedclosed

In 12187:

Adds hotkey (Tab) for showing all player's entities' status bars at once, based on patch by Deiz/F00. Fixes #1187

comment:43 by historic_bruno, 12 years ago

Keywords: review removed
Note: See TracTickets for help on using tickets.