[PATCH] After pressing tab as observer, the status bars don't disappear

[elexis]

If you press the tab button, then your units health and aura icons are displayed.

However if you do this as a spectator, then the status bars don't disappear when releasing the button. You can only remove them by hovering each entity individually.

Also status bars over gaia entities like trees are displayed.

It would be nice too if your enemies status bars are displayed, so that you can select the weakest enemy unit to attack. Currently you have to hover each unit to find its health.

[leper]

• => in the new code and in the filter you moved.
• use if (observer) ... else if [...] so you don't have to move all that code.
• The GuiInterface change is ugly, also seems a bit strange. Could also use x || y to make it nicer.

Though I'm not really sure about that part of the change at all (nor about the addition of observer specific checks in the gui.

[elexis]

Replying to leper:

• => in the new code and in the filter you moved.

What does that mean? indention?

• use if (observer) ... else if [...] so you don't have to move all that code.

Yes.

• The GuiInterface change is ugly, also seems a bit strange. Could also use x || y to make it nicer.

I know its ugly, I can't look at it either. But the problem is that the player argument is automatically filled in by the GuiInterface. player || specificPlayer doesn't work therefore, since the player variable always contains the current player ID. I could make a second function GetEntitiesByOtherPlayer with an argument that has a different name than 'player'.

Though I'm not really sure about that part of the change at all (nor about the addition of observer specific checks in the gui.

Pressing the tab button should display the status bars. If you are observer and press it, then the status bars appear but don't disappear without the patch. With the patch they disappear again. Without the patch status bars appear over gaia stuff like resources too. With the patch they appear only over player entities. So the patch fixes two actual errors. There must be an observer specific check, otherwise those 2 errors will appear. One could check if the client is an observer and disable the feature completely, but why not make it a useful feature with that patch above, so that the observer can better see which player is winning the battle.

I put the priority on should have, since that hotkey is heavily broken for observers, not an enhancement. You have hover each resource after pressing that key in order to fix the bug.

[elexis]

-Renames PickSimilarFriendlyEntities to PickSimilarPlayerEntities and PickSimilarFriendlyEntities to PickSimilarPlayerEntities (since it doesnt pick allied entities).

-Solved ugly javascript code and optimized performance by pushing the relevant code to c++ (rangemanager) and using the the fat arrow function in recalculateStatusBarDisplay()

-Should have because currently (as described above) the tab hotkey is heavily broken if you use it as an observer.

[elexis]

TODO: some cleanup suggested by sanderd17 in irc on june 3rd 2015.

[elexis]

(1) Renames functions from ...AllPlayerEntities to ...NonGaiaEntities (2) Removes the loop when computing the ownership mask (3) Uses for (const entity_id_t& ent : ents) for a cleaner loop (4) Removes an unused player argument in the GuiInterface

[Itms]

Code review:

• remove the trailing space and add back that line in session.js
• why do you have both this function in ScriptInterface.cpp and the EntitySelection helper (that you don't call AFAICS)? Is this an oversight?
• emplace_back in the range manager function
• why don't you directly check the gaia bit in the owner mask?

Apart from that it looks good!

[elexis]

• Added comments
• Fixed whitespace
• CStr instead of std::string
• emplace_back instead of push_back
• Range-based loops in existing code
• Removed unused function PickNonGaiaEntitiesInRect
• Directly checks the owner instead of computing a mask in GetEntitiesByPlayer
• Introduces NON_GAIA_PLAYERS so that PickPlayerEntitiesInRect and GetEntitiesByPlayer can be used instead of duplicating existing functions.

Instead of introducing that constant, we could add another boolean parameter nonGaiaEntities to those two functions and CheckEntityVisibleAndInRect and if it's true, ignore the player / owner argument. But that seems more ugly to me, not sure.

[Itms]

• I'm not very enthusiastic about this NON_GAIA_PLAYERS. You should take advantage of that mask code you removed (it's more clever than you think): just add a GetEntitiesByMask function and use it in GetEntitiesByPlayer and GetNonGaiaEntities.
• Also, this range-based for is not really nice, I think keeping iterators is ok until we make EntityMap support range-based fors seamlessly.
• Why did you add that check to line 101 of Selection.cpp?
• Your change at the end of Selection.cpp is bad because now you go through your lists twice so you increase the complexity. I don't see why you touched that code in the first place, but using range based fors and emplace_back should be enough.

[elexis]

In 17622:

Rename PickFriendlyEntitiesOnScreen to PickPlayerEntitiesOnScreen.
Rename PickFriendlyEntitiesInRect to PickPlayerEntitiesInRect.
Rename PickSimilarFriendlyEntities to PickSimilarPlayerEntities.
Add comment.
Refs #3215.

[elexis]

In 17623:

Show status-bars of all players to observers, fixes #3215.

[elexis]

Thanks sanderd17 and Itms for all the feedback!

[elexis]

In 17663:

Remove the virtual keyword since this is a helper-function not used by the interface. Refs #3215.

[elexis]

In 17758:

Improve and fix some wrong observer-checks that didn't take defeated players into account. Refs #3168, #3215.
Introduce isPlayerObserver to easily cover that case.
Move some checks and updates into selectViewPlayer and eliminate setObserverMode.
Initialize the music before changing the perspective.

[elexis]

Actually I'm not sure anymore whether the recommendation from 2015-06-03 to use NonGaia was the best choice, since some entities don't have an ownership component or have owner -1 returned.

This becomes relevant if we want to get all entities owned by gaia or players, but not unowned ones, so GetGaiaAndNonGaia entities sounds like all entities when it's not (the function was proposed for introduction in ​https://code.wildfiregames.com/D229)