Opened 11 years ago

Last modified 19 months ago

#2087 new enhancement

[PATCH] Scroll through loading screen tips

Reported by: michael Owned by:
Priority: Nice to Have Milestone: Backlog
Component: UI – Miscellaneous Keywords: patch
Cc: Patch: Phab:D1730

Description (last modified by Stan)

  1. In single player, allow the player to cycle through the available tips, with arrow buttons (left and right).
  1. When map is done loading, "I'm Ready!" button appears at the bottom of the screen to go into the game.

Attachments (7)

patch.tmp (7.2 KB ) - added by Dennis Ruhe 11 years ago.
Patch with proposed changes
loading2.patch (11.6 KB ) - added by metis 9 years ago.
Further editing
loading2_.diff (12.0 KB ) - added by leper 9 years ago.
Some fixes to loading2.patch, still a slightly unrelated issue left.
2087_Rebased.diff (11.1 KB ) - added by Stan 8 years ago.
This patch fix the comments above (Weird Buttons and position on min resolution) and rebase it to work with current version
2087.2.diff (11.2 KB ) - added by Stan 8 years ago.
Reset the X,Z positions of the camera after loading, and move the default cfg to the right place.
Loading.js_cleanup.diff (4.5 KB ) - added by Stan 8 years ago.
Early Return should remove trailing if any, replace var by let, use g_ instead of caps.
2087.3.diff (10.2 KB ) - added by Stan 8 years ago.
Rebased patch, should fix the style issues. Things remaining are the big button (Which I like the way it is as you can press enter anyways) and the camera movement.

Download all attachments as: .zip

Change History (48)

comment:1 by leper, 11 years ago

Keywords: simple added; GUI tips button removed

in reply to:  description comment:2 by historic_bruno, 11 years ago

Replying to michael:

  1. In single player, allow the player to cycle through the available tips, with arrow buttons (left and right).

Do you mean arrow keys, or there will be arrows on the screen to click?

  1. When map is done loading, "I'm Ready!" button appears at the bottom of the screen to go into the game.

Hmm, an extra button to start the game? I can't say I'm a big fan of that, though it does allow tip cycling. That should have a config option IMO, I could see it getting annoying for experienced players.

comment:3 by wraitii, 11 years ago

I think it's alright. You won't start a game every 2 minute.

comment:4 by Dennis Ruhe, 11 years ago

Owner: set to Dennis Ruhe
Status: newassigned

by Dennis Ruhe, 11 years ago

Attachment: patch.tmp added

Patch with proposed changes

comment:5 by Dennis Ruhe, 11 years ago

Keywords: patch review added
Summary: Loading Screen Enhancements[PATCH] Loading Screen Enhancements

comment:6 by Dennis Ruhe, 11 years ago

At 90% the UI seems to hang.. Prev/next buttons don't work. I don't know what it's doing there but it seems not related to any changes I made.

100% progress seems to be never reported to the GUI, I have no idea where to look for that though.

Last edited 11 years ago by Dennis Ruhe (previous) (diff)

comment:7 by historic_bruno, 11 years ago

Milestone: BacklogAlpha 15

comment:8 by historic_bruno, 10 years ago

Keywords: review removed

Thanks for the patch.

  • The current behavior is misleading, as the game begins in the background even though the loading screen is still visible. While the player is sitting there perusing tips, their enemies could be crushing them :( I would suggest pausing the game on turn 0 in single player mode and possibly disabling the tip cycling altogether in multiplayer mode.
  • You should remove lines instead of commenting them out :) The wait cursor should definitely be removed as the user is able to click on buttons.
  • The cycling logic is strange, current_tip_num starts undefined, which evaluates to false but so does 0, that may not be desired since both will switch to a random tip. It seems like looping around on both ends with a random initial tip was the intended behavior?
  • The layout is messed up at 1024x768 resolution, the lowest we support, so make sure it looks good there. Maybe placing the prev/next buttons above the tip areas would work?
  • Instead of playing with button size, use the boolean "hidden" property to show and hide the start game button.
  • The delay toward the end of map loading is possibly pre-rendering and other stuff that happens at the beginning of a game but isn't split up into incremental loading.
  • We should add some hotkeys for the buttons.
  • A related nice feature would be a "Cancel Loading" or "Return to Main Menu" button, in case the player gets tired of waiting for the map to load on a slow computer, or changes their mind.

comment:9 by wraitii, 10 years ago

Delay at the end is more than likely AI initialization.

comment:10 by wraitii, 10 years ago

Milestone: Alpha 15Alpha 16

comment:11 by Erik Johansson, 10 years ago

I just want to add my support for the "I'm ready" button :) It's really useful, at least if you have a slower computer, to be able to start loading a map, and go grab something to drink etc while it's loading. It's probably a good idea to include a setting for those who don't like it, though imho the button should be there by default.

comment:12 by Stan, 10 years ago

Milestone: Alpha 16Alpha 17

Isn't the ready word a bit long in certain languages ? Maybe a click here to start or just click here. Anyway, I don't think we will have time to do this before feature freeze. Pushing to A17

comment:13 by historic_bruno, 10 years ago

Owner: Dennis Ruhe removed
Status: assignednew

comment:14 by Itms, 10 years ago

Milestone: Alpha 17Backlog

I'll backlog this because no one works on this anymore.

comment:15 by metis, 9 years ago

Owner: set to metis
Status: newassigned

comment:16 by metis, 9 years ago

Picking this up as a getting-feet-wet project. My plan is to start from AmazingDreams' patch, and integrate historic_bruno's review.

Edit: Itms confirmed in chat that "I'm ready" is acceptable wording.

Last edited 9 years ago by metis (previous) (diff)

comment:17 by metis, 9 years ago

Keywords: review added

In loading2.patch:

  1. AmazingDreams' patch is updated to work with current version (such as using Engine.GetGUIObjectByName instead of a global)
  2. The game is now paused once it finishes loading until the use clicks "I'm ready."
  3. The tip cycling logic is now more straight-forward. It initializes current_tip_num to a random tip in init(data), and merely cycles in the next/prevTip() calls.
  4. The Previous/Next tip buttons have been moved to the bottom of the tip text window. This reduces the available space for tips, but no current tips use that much space.
  5. Hotkeys have been added, with defaults:
  6. hotkey.confirm can be pressed once loading is complete to enter the game.

The following things are not addressed by this patch:

  1. There is still a delay towards the end of map loading.
  2. There is no "Cancel Loading" feature. It looks like implementing this will require some way of signalling the onTick() function in binaries/data/mods/public/gui/gamesetup/gamesetup.js that the load has been cancelled. Merely calling Engine.EndGame() does not abort the async. load and eventually segfaults. Simply switching to page_pregame.xml leaves things in an ugly state the next time a new game is started.

in reply to:  17 comment:18 by historic_bruno, 9 years ago

A few minor points:

  • I would change the hotkeys to be nexttip and previoustip, so their function is more explicit.
  • They are tips, not "tooltips" :)

Replying to metis:

  1. The game is now paused once it finishes loading until the use clicks "I'm ready."

I'm sure some people will complain about this, because it's an extra step that wasn't there before, maybe it could be a config option: pauseafterloading? I think it's good for beginning players and should be the default, but more experienced players will have seen the tips and want to skip that.

comment:19 by metis, 9 years ago

This new patch incorporates suggests from leper and historic bruno. loading.js has been substantially cleaned up, and a new option "Tip Controls" has been added to allow the the user to disable the tip control functionality if they don't like it.

Some notes:

  • On the loading screen, the left/right arrow keys (by default) can be used to scroll through tips, and enter can be pressed to start the game once it's ready.
  • The game is only paused in between the loading having finished and the user pressing "I'm ready" to enter the game. The user never sees it in a paused state. This was specifically added to address the problem of the AI playing in the loaded game while the user reads tips / gets a drink / whatever.

comment:20 by leper, 9 years ago

Some more comments:

  • tipImage is set twice. And the first assignment is still broken as it will never hit the || "", might be worth making an if there. tipImageFilePath is useless if you do the first change.
  • You broke the translation.
    • There is no need to translate substrings of tipText if tipText is translated already.
    • The sprintf for loadingMapName.caption does not translate the text, only the map name. (Also use { "foo": bar }.)
  • Remove the spacing around the parameters to updateTip().
  • getTipArray() is useless and the single call to it could just be replaced by the body of it.
  • // Pick a random quote of the day (each line is a separate tip). does not make sense. Replace it with // Pick a random quote of the day (one quote per line).
  • Remove the space before the opening parenthesis on line 23.
  • Calling updateTip() in case !tips || tips.length == 0 does not make sense. I'd move the tip logic in init() to the end of the function and return in case we don't have any tips (possibly changing any defaults that would complicate it).
  • You can just use !data.isNetworked instead of comparing with false.

by metis, 9 years ago

Attachment: loading2.patch added

Further editing

by leper, 9 years ago

Attachment: loading2_.diff added

Some fixes to loading2.patch, still a slightly unrelated issue left.

comment:21 by leper, 9 years ago

Changed your last version of the patch a slightly, but while testing it I noticed that once the game finished loading using the hotkeys for camera movement also moves the camera in-game. Possibly related to GameView handling the hotkeys in Update() while it shouldn't do that in this case. (It should still do so when paused though.)

Also the next and previous buttons look a bit strange on that background. The "I'm ready" button overlaps the quotes on the minimum resolution (1024x768).

in reply to:  21 comment:22 by leper, 9 years ago

Also the next and previous buttons look a bit strange on that background. The "I'm ready" button overlaps the quotes on the minimum resolution (1024x768).

Any progress on this?

comment:23 by leper, 9 years ago

Any updates?

comment:24 by Niek, 9 years ago

Owner: metis removed
Status: assignednew

comment:25 by Niek, 9 years ago

Keywords: review removed

comment:26 by leper, 9 years ago

#689 was a duplicate of this.

by Stan, 8 years ago

Attachment: 2087_Rebased.diff added

This patch fix the comments above (Weird Buttons and position on min resolution) and rebase it to work with current version

comment:27 by Stan, 8 years ago

Keywords: review added
Milestone: BacklogAlpha 20
Owner: set to Stan
Status: newassigned

Is there something else to do ?

Last edited 8 years ago by Stan (previous) (diff)

comment:28 by leper, 8 years ago

Keywords: review removed

The changes to default.cfg are broken. You should both add them to the proper sections and add section headers for them.

A fix for the movement thing would still be nice (maybe just saving the camera position, and then restoring it when actually starting the game).

by Stan, 8 years ago

Attachment: 2087.2.diff added

Reset the X,Z positions of the camera after loading, and move the default cfg to the right place.

comment:29 by Stan, 8 years ago

Keywords: review added; simple removed

comment:30 by elexis, 8 years ago

Keywords: review removed
Summary: [PATCH] Loading Screen Enhancements[PATCH] Scroll through loading screen tips
  • I like the idea.
  • Should be fine that the change only affects singleplayer.

TODO (features):

  • I'm not fond of the "I'm ready" button. It seems to be too big to me. Maybe it should be in the bottom right corner?
  • The camera should start at the correct position instead of moving there quickly.
  • Got this warning when starting multiplayer on "Acropolis Bay (2)":
    WARNING: JavaScript warning: gui/loading/loading.js line 118
    reference to undefined property g_Data.posX
    
    and the camera started somewhere far away from the CC.

TODO (coding quality):

  • Is the gamesetup.js change irrelevant (hence wrong)?
  • What did we say about the naming convention of globals? wiki:Coding_Conventions
  • nextTip() and prevTip() only contain one line, might be nuked
  • the patch contains a tab instead of space after tipText
  • the patch contains some code cleanup, which might be committed separately
  • tipControlsEnabled is not required to be global, as you can just check for button.hidden
  • Can you rename the tips variables to g_LoadingTips etc., so it's more obvious what kind of tip it is?
  • There might be more after this.

by Stan, 8 years ago

Attachment: Loading.js_cleanup.diff added

Early Return should remove trailing if any, replace var by let, use g_ instead of caps.

comment:31 by elexis, 8 years ago

In 17523:

Loading screen cleanup. Patch by Stan, refs #2087.

Use let instead of var.
Use g_ prefix for globals.
Remove trailing whitespace.

by Stan, 8 years ago

Attachment: 2087.3.diff added

Rebased patch, should fix the style issues. Things remaining are the big button (Which I like the way it is as you can press enter anyways) and the camera movement.

comment:32 by Itms, 8 years ago

Milestone: Alpha 20Alpha 21

This patch has to be rebased. I tested it and found it quite ugly :(

To be honest I'm not sure how to improve it though, as I have a very lacking artistic sense.

comment:33 by elexis, 8 years ago

Keywords: simple added

Better change the C++ part to not start the game before the user has clicked the button.

Looking at CGame::ReallyStartGame(), it might be sufficient to just not set m_GameStarted to true until the single player clicked the ready-button.

This hopefully means not having to restore the camera, disabling some hotkeys (including other unintended sideeffects we didn't notice yet) in the first place.

comment:34 by elexis, 8 years ago

Backlogging due to lack of progress.

comment:35 by elexis, 8 years ago

Milestone: Alpha 21Backlog

comment:36 by Stan, 7 years ago

Owner: Stan removed
Status: assignednew

unassigning myself from this ticket.

comment:37 by Stan, 5 years ago

Description: modified (diff)
Milestone: BacklogAlpha 24
Owner: set to Stan
Patch: Phab:1730

comment:38 by elexis, 5 years ago

Keywords: simple removed
Patch: Phab:1730Phab:D1730

comment:39 by Imarok, 5 years ago

Component: UI & SimulationMisc. UI

Move tickets to Misc. UI as "UI & Simulation" got some sub components.

comment:40 by Stan, 4 years ago

Milestone: Alpha 24Work In Progress

I'd like to have this at some point, but needs more work.

comment:41 by Stan, 19 months ago

Milestone: Work In ProgressBacklog
Owner: Stan removed

No progress in a while

Note: See TracTickets for help on using tickets.