#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)
Change History (54)
comment:1 by , 12 years ago
comment:2 by , 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:4 by , 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 , 12 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:6 by , 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 :)
comment:7 by , 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? :)
follow-up: 9 comment:8 by , 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.
comment:9 by , 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 , 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 , 12 years ago
Keywords: | review added; simple removed |
---|---|
Milestone: | Backlog → Alpha 9 |
Resolution: | → worksforme |
Status: | assigned → closed |
comment:11 by , 12 years ago
Resolution: | worksforme |
---|---|
Status: | closed → reopened |
Summary: | Hotkey to display all health bars → [Patch]Hotkey to display all health bars |
comment:13 by , 12 years ago
Status: | reopened → new |
---|
comment:14 by , 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.
follow-up: 16 comment:15 by , 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 , 12 years ago
Attachment: | Ticket#1187_V2.patch added |
---|
Attempt #2, hopefully minus some obvious flaws
comment:16 by , 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 , 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 , 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 , 12 years ago
Keywords: | review added |
---|
by , 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 , 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 , 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 , 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 , 12 years ago
Summary: | [Patch]Hotkey to display all health bars → [PATCH] Hotkey to display all health bars |
---|
comment:23 by , 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 , 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 , 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 , 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 , 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 , 12 years ago
Milestone: | Alpha 9 → Alpha 10 |
---|
comment:28 by , 12 years ago
Keywords: | review removed |
---|
comment:29 by , 12 years ago
Keywords: | review added |
---|
by , 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
comment:30 by , 12 years ago
Keywords: | patch added |
---|---|
Priority: | If Time Permits → Nice to Have |
comment:31 by , 12 years ago
Priority: | Nice to Have → Should Have |
---|
comment:32 by , 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 , 12 years ago
Attachment: | health_bar_hotkey_v01.patch added |
---|
comment:33 by , 12 years ago
Owner: | changed from | to
---|
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 , 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 , 12 years ago
Attachment: | health_bar_hotkey_v02.patch added |
---|
comment:35 by , 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 , 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 , 12 years ago
Keywords: | review removed |
---|---|
Owner: | changed from | to
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 , 12 years ago
Milestone: | Alpha 10 → Alpha 11 |
---|
comment:39 by , 12 years ago
Owner: | removed |
---|
comment:40 by , 12 years ago
Keywords: | review added |
---|---|
Owner: | set to |
Status: | new → assigned |
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 , 12 years ago
Attachment: | healthbars.patch added |
---|
comment:41 by , 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.
by , 12 years ago
Attachment: | healthbars.2.patch added |
---|
comment:43 by , 12 years ago
Keywords: | review removed |
---|
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...