Object list window: allow massive focus setting

[vincele]

The attached patch make a context menu entry to allow focus to be set on multiple selected planets at the same time.
If the chosen focus cannot be applied to some of the planets then those planets foci stay unchanged...

Please review and comment. Testing also welcome, especially WinOS and MacOS, as usual...

This is the start of my work on objlistwnd, next to come is planet suitability...

[The extension diff has been deactivated and can no longer be displayed.]

[vincele]

Just a quick note:
For the previous patch to be useful (and testable) for me on linux,
the multiselection (CTRL or SHIFT based) was not working properly.

So I needed to change OISInput.cfg likewise :

Code: Select all

$ svn diff
Index: OISInput.cfg
===================================================================
--- OISInput.cfg	(revision 7874)
+++ OISInput.cfg	(working copy)
@@ -21,7 +21,7 @@
 # Define X11 settings
 x11_mouse_grab=false
 x11_mouse_hide=true
-x11_keyboard_grab=false
+x11_keyboard_grab=true
 XAutoRepeatOn=true

Made it functioning again

[Geoff the Medio]

Instead of a #define, please use a const int CAPITALIZED_NAME;

Might be nice if the commands to set focus specified how many selected planets can use each available focus.

Do you need separate storage for available foci and all foci? You can increment std::set<>::interator using std::advance(...) to get the right string, without need to put them into a vector as well...

Put a space in "TemporaryPtr<Planet>one_planet".

And double-check that that pointer is valid after getting it, before using it to set focus.

Also, note that you can't just set focus on the client's copy of a planet... you need to issue an order to set the focus for it to take effect on the server. I'm not certain, but I would guess there is no signal connection that will do this from calling Planet::SetFocus as you have done. Check how it works in the SidePanel and PlanetPanel code, including SidePanel::PlanetPanel::SetFocus.

[vincele]

Thanks Geoff, for all the good feedback. I'll address that and re-spin.

Might be nice if the commands to set focus specified how many selected planets can use each available focus.

Not that it shouldn't be easily done, but I don't think I'll personally need that info myself. You'd need to know to which planets it won't apply for that to be useful, no ?

But anyways, I can add that. In the same patch or as a subsequent one ?

And how would you suggest to display it ?

Terse-mode (compact but not intuitive)

Code: Select all

=> Set Focus
====> Growth (1)
====> Industry (8)
====> Research (8)

or self-explanatory-mode (bigger text size)

Code: Select all

=> Set Focus
====> Growth (applies to 1 planet)
====> Industry (applies to 7 planets)
====> Research (applies to all selected planets)

[Geoff the Medio]

Terse is fine, I think. Separate or combined patch; don't care.

[Dilvish]

Just a quick note:
For the previous patch to be useful (and testable) for me on linux,
the multiselection (CTRL or SHIFT based) was not working properly.
So I needed to change OISInput.cfg likewise :

Code: Select all

-x11_keyboard_grab=false
+x11_keyboard_grab=true

Made it functioning again

That seems a bit surprising to me-- all my OISInput.cfg settings are as yours were, including

Code: Select all

x11_keyboard_grab=false

and I have no trouble (on Ubuntu) with multiselection in FO.

[vincele]

Terse is fine, I think. Separate or combined patch; don't care.

OK, here is a new patch, hopefully addressing all your feedback.

[The extension diff has been deactivated and can no longer be displayed.]

[vincele]

But during my testing, I saw a bug, when you change focus in sidepanel, it is not refreshed in objlistwnd's focus column. That bug is already present in svn, so my patch does not create a regression.
How would one fix it ?
Making sidepanel's setfocus refresh objlistwnd if it has focus shown in a column, this does not sound good to tie them.
Or hook objlistwnd on a signal emitted when some planet focus change ?

[Geoff the Medio]

Or hook objlistwnd on a signal emitted when some planet focus change ?

UniverseObject-derived classes emit StateChangedSignal when their state changes. Some may also emit other signals for more specific changes. You might want to connect objects' StateChangedSignal to a function that refreshes the object list row for that object. Note, however, that if refreshing rows takes too long, it could introduce more delay in the UI when issuing orders.

Re: the patch, I should be able to right click on a single planet when none are selected and get the set focus option. Also, when setting focus, the selected rows shouldn't reset.

[vincele]

Re: the patch, I should be able to right click on a single planet when none are selected and get the set focus option.

This new patch does it...

[The extension diff has been deactivated and can no longer be displayed.]

Also, when setting focus, the selected rows shouldn't reset.

Is this possible with Gigi ? I don't know what is killing the selection, and it annoys me too..

[Geoff the Medio]

Is this possible with Gigi ?

Have a look at the producible items list in the production screen, or the fleets / ships lists. I can't check immediately, but suspect some efforts have been made to retain selections when list contents are updated.

Edit: Also, under the assumption that I'll commit a version of this soon, please PM me if you have a real name you'd prefer to be listed as in the credits.

Edit2: committed

[vincele]

Thanks, I'll try to have a look at the update from signal thing.

[vincele]

This is fixing the set focus loose the selected rows, crude attempt...

[The extension diff has been deactivated and can no longer be displayed.]

[vincele]

WRT the sidepanel focus change not being reflected in objlistwnd, the following patch fixes it for me, is that the right way to do it ?

[The extension diff has been deactivated and can no longer be displayed.]

[Geoff the Medio]

Not really... Presently setting a planet's focus doesn't cause UniverseObject::StateChangedSignal to be emitted, but instead ResourceCenter::ResourceCenterChangedSignal is emitted. Based on comments (see MapWnd.cpp, around the line

Code: Select all

GG::Connect(SidePanel::ResourceCenterChangedSignal,     &MapWnd::UpdateSidePanelSystemObjectMetersAndResourcePools, this);

), this is done because it could cause a bunch of updates to unrelated objects meters, and only the stuff shown in the Sidepanel actually needs updating. This is a potential approximation / bug, in that some stuff could depend on the focus setting of other stuff, but this is probably a rare situation.

Instead, you could set up a connection from ResourceCenterChangedSignal of any ResourceCenter with a row in the ObjectListWnd, and update that row whenever the signal fires. (Similar to the StateChangedSignal connections stored in ObjectListBox::m_object_change_connections)