Opened 5 years ago

Closed 5 years ago

#3101 closed enhancement (fixed)

[PATCH]Add structure tree to session menu

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

Description

Now that we have a structure tree accessible from the main menu (Learn to Play > Structure Tree), we should add a link to it from the session UI as well. In fact, it wouldn't be a bad idea to have the same "Learn to Play" menu brought into the session, for reference.

Attachments (5)

structTreeSession.patch (1.9 KB) - added by Loïc Lopes 5 years ago.
civIconOverlay.patch (3.8 KB) - added by Loïc Lopes 5 years ago.
Patch v2
t3101_hide_civ_icon_for_observer.patch (831 bytes) - added by elexis 5 years ago.
Quick fix. Should be updated again with #3168.
t3101_dynamically_hide_elements_for_observers.patch (4.6 KB) - added by elexis 5 years ago.
Dynamically hides / shows the elements in the upper panel when changing the perspective.
t3101_dynamically_hide_elements_for_observers_v2.patch (5.5 KB) - added by elexis 5 years ago.
Pause button now available to observers in singleplayer mode, while still preventing defeated players and other observers in multiplayer to pause the game. Refactors the updating the top panel GUI to a single method, which is called when resigning too (so that you cant pause the game after resigning). The observerText should already be hidden for gaia.

Download all attachments as: .zip

Change History (28)

Changed 5 years ago by Loïc Lopes

Attachment: structTreeSession.patch added

comment:1 Changed 5 years ago by Loïc Lopes

Milestone: BacklogAlpha 19
Summary: Add structure tree to session menu[PATCH]Add structure tree to session menu

comment:2 Changed 5 years ago by Loïc Lopes

Keywords: review patch added

comment:3 Changed 5 years ago by Loïc Lopes

I added the link by using the civ_icon (after a discussion on the chat). It seemed nice as it wasn't used for anything and players might try by default to click on it.

comment:4 Changed 5 years ago by FeXoR

Does it give a visual feedback if the mouse is moved above it? (IMO it should have since it's not obviously a button) EDIT: Or is the tooltip suficient?

Last edited 5 years ago by FeXoR (previous) (diff)

comment:5 Changed 5 years ago by Loïc Lopes

No feedback for now. We may add some info to the tooltip as "CivilizationName? - Structure tree".

comment:6 Changed 5 years ago by FeXoR

A glowing effect would ofc be nice ;) Not sure if that's possible.

comment:7 Changed 5 years ago by Loïc Lopes

A solution would be to add a border as for the construction icons which glows. Otherwise we would need to duplicate the civ icons with a glowing effect (with Gimp or any other).

comment:8 Changed 5 years ago by sanderd17

You'll have to go to session/sprites.xml and session/styles.xml. There you probably need to make one style for the civ button, with three sprites for each button state (see gui/common/common_styles.xml and gui/common/common_sprites.xml where the button styles are defined, you can probably take some stuff from it).

I'm not sure if the image you set is used as overlay (in which case you can just define the style on the civ icon itself), or of the image is replaced when hovering.

In the latter case, you need to duplicate the civ icon XML object, so that you have the civ icon itself as background, while you also have a transparent button with the same size on top of it. The button can than use more or less transparent textures to provide a glow or a border.

For the overlay textures itself, I asked the help of Pureon, so you probably should only make dummy images, that shows the possibilities (f.e. a white half-transparent square image of the same pixel size as the civ icons should be easy to make).

Modifying the tooltip would of course be welcome.

comment:9 Changed 5 years ago by Pureon

Please use the placeholder emblem button states I committed here http://trac.wildfiregames.com/changeset/16464/

Changed 5 years ago by Loïc Lopes

Attachment: civIconOverlay.patch added

Patch v2

comment:10 Changed 5 years ago by leper

Any updates? (The translate call is broken as was already noted on IRC a few days ago; the !== "" is useless; how does it work for observers?)

comment:11 Changed 5 years ago by Loïc Lopes

Yes I corrected that. But I have problems to manage the pause/unpause. I'll pass on the chat when I come back from work to get some help on this subject..

comment:12 Changed 5 years ago by leper

Any updates?

comment:13 Changed 5 years ago by sanderd17

Any progress?

It's a nice functionality, so I'd like to get it in game.

If you can't work on it further, I'll take over the work.

comment:14 Changed 5 years ago by sanderd17

Owner: set to sanderd17
Resolution: fixed
Status: newclosed

In 16610:

Add structure tree to the game session. Based on patch by Louhike. Fixes #3101

comment:15 Changed 5 years ago by sanderd17

Keywords: simple review removed

comment:16 Changed 5 years ago by elexis

Resolution: fixed
Status: closedreopened

comment:17 Changed 5 years ago by elexis

Doesn't work for observer:

WARNING: JavaScript warning: gui/session/menu.js line 626
reference to undefined property g_Players[Engine.GetPlayerID(...)]
ERROR: JavaScript error: gui/session/menu.js line 626
TypeError: g_Players[Engine.GetPlayerID(...)] is undefined
  openStrucTree@gui/session/menu.js:626:3
  __eventhandler146 (press)@civIconOverlay press:0:1

Changed 5 years ago by elexis

Quick fix. Should be updated again with #3168.

comment:18 Changed 5 years ago by sanderd17

AFAICS, observers already don't have a civ icon.

comment:19 Changed 5 years ago by mimo

But there is still a problem even with the last proposed fix when switching perspective to gaia: I guess it is because the button is set hidden only at init, while it should be updated when switching player.

Last edited 5 years ago by mimo (previous) (diff)

Changed 5 years ago by elexis

Dynamically hides / shows the elements in the upper panel when changing the perspective.

comment:20 Changed 5 years ago by mimo

Resolution: fixed
Status: reopenedclosed

In 16633:

prevent loading of templates in AIManager when no AIs, fixes #3101

comment:21 Changed 5 years ago by mimo

Resolution: fixed
Status: closedreopened

Reopening as closed by mistake

comment:22 Changed 5 years ago by mimo

Thanks for the patch. While testing, I noticed 3 things:

  • gaia is not an observer, so the observerText should be hidden when playerID = 0
  • pauseButton is set inactive after having resigned: is it a behaviour we want ? in MP games certainly, but I'm not convinced for SP game. Same thing for gaia in SP games, we may want to pause after having changed perspective to gaia. I think the behaviour should depend on SP/MP, and no disabling on SP.
  • futhermore, whatever the chosen behaviour for the pause after resign, it should be made consistent with what is done in the resignGame function. With the patch, when you resign, the button is still active and disabled only when you change player.

Changed 5 years ago by elexis

Pause button now available to observers in singleplayer mode, while still preventing defeated players and other observers in multiplayer to pause the game. Refactors the updating the top panel GUI to a single method, which is called when resigning too (so that you cant pause the game after resigning). The observerText should already be hidden for gaia.

comment:23 Changed 5 years ago by mimo

Resolution: fixed
Status: reopenedclosed

In 16637:

fixes #3101, patch by elexis

Note: See TracTickets for help on using tickets.