Story mode timers

[Alayan-stk-2]

This PR fixes #2907

Some details on the implementation (I will add an illustrating video later) :

• It uses two separate timers : a normal story mode timer and a speedrun timer. The normal timer is enabled by default, while the speedrun timer is disabled by default.
• It works by computing the time between a start point (when entering story mode) and an end point (either current time or the time at which the story mode was finished), and deducing times where the timer is paused.
• Options manage the display of the timer, but they don't affect the counting itself. Having only one timer having to switch between both modes, or having the timer only work when enabled, would introduce a lot of complications/issues (impossible to change the options after starting the story mode without breaking the timers) for an extremely tiny overhead gain when disabled. Note that a lot of the apparent changes in the options .stkgui are from moving around options to be more logically grouped (like the internet options together), rather than deleting/adding things.
• The normal timer is paused on loading screens and when outside of the story mode. The speedrun timer is only paused on loading screens
• The speedrun timer is always displayed on-screen when enabled (including during loading). It's an important feature for it. It is also displayed to the ms. On the other hand, the normal timer is only displayed on the overworld and in story mode races, and to the second only as it doesn't need more display accuracy (though the internal value is stored more accurately). Their display is mutually exclusive : if the speedrun mode is enabled, the normal timer won't be displayed. They are displayed to the left of the normal race timer position.
• As the story mode will take more than an hour to complete, this PR adds support to display hours with timeToString. It also switches the max value for non-hours strings from 99:59.99 to 59:59.99 (GP total times may be improved by using the hours display, as long custom GP could easily go over an hour, but I didn't touch them in this PR)
• Switching player profiles saves/resets/reloads the timers
• The status (finished or not) and the duration of the timers are saved in players.xml. They are also loaded when opening the game or switching profiles. The speedrun mode is disabled if the story mode is not finished in a unique session, but the normal timer supports multi-session usage.

KNOWN LIMITATIONS :

• The new nodes it tries to read don't exist on old player profiles saved before, so it will have junk data when loading them (and the subsquent saves, while creating the nodes, will contain the invalid data so will keep being worthless). Considering that there is already a profile breaking change in master compared to 0.9.3 ; I don't consider this a huge issue, but it reinforces the need of using a separate config profile folder for the next release (on that topic, I also think that git versions should not use the same profile folder than stable releases). Not an issue anymore as the next release will have a different profile folder.
• After having finished the story mode and stopping the timers, they are not displayed in the achievements or in some similar place ; but keeps either being displayed in the overworld/challenges (normal timer) or on all screens (speedrun timer), assuming the options to display them are enabled. I think this is minor enough to be left to a later enhancement.
• The timers don't stop during cutscenes. Whether it is better to stop it then is a debatable matter and can in any case be left to a later enhancement.
• The probability that there is some bug somewhere is rather high. I've tested it in a lot of situations (launching the story mode, switching the display options, switching profiles, closing/reloading, finishing the game), and I've corrected a lot of issues to get to this PR (the saving/loading was particularly capricious). Despite having corrected the issues I encountered, their number means it's likely that there is still some situation I didn't test where it breaks.

[auriamg]

As discussed on IRC :
I want to discuss this one with @hiker , this PR is huge and lots of code for something that was never on our TODO list. seems something that is only useful to speedrunners though, which are a tiny, tiny fraction of our audience. adding significant amounts of code or new UI options for a tiny fraction of our players is not something we're too interested in

[Alayan-stk-2]

While the speedrun timer is meant only for players which would be interested in completing the story mode quickly ; a lot of work (and indeed, a substantial part of the line/file changes) has gone to also give a more casual story mode timer, which can be saved and resumed over several sessions, as detailed in the presentation message. Enabled by default (so that the player is aware of the options), it is meant to be useful to a larger audience, without being too intrusive (it is only displayed in the overworld and in challenge races, and I could have it displayed only in the overworld or in some other place if preferred).

To support my point about the usefulness of a general timer for the story mode, I don't think it annoy many pokemon players that the game stores and display in profile the game time since start. Granted, it's not the same kind of game, but it's certainly less a game about speed than STK, so if it can make sense there, it certainly can here too.

Then, also take into account that while the speedrun part may not be useful to a lot of players, it will be useful for dedicated players, who do publicity for the game, help to improve the game by finding exploits, balance issues, etc. Surely, adding one checkbox in the UI options is not that big of a deal.

[hiker]

I am happy to support speed-runner with a special command line option or so, but I don't see any reason to add a second timer, so could this be removed.
Also (not sure if it is important or useful): the XMLNode get() functions return true if an attribute exists or false otherwise. So you can always just check this to make sure you don't get incorrect or uninitialised values.

[Alayan-stk-2]

The use of two timers is a conscious design choice :

• the normal story mode timer (geared towards more casual players) and the speedrun timer have different rules on time counting (time outside of the story mode is counted by one but not the other, one can be saved/loaded/resumed and not the other), it's not only a matter of data representation.
• having the timers only active when their display option is enabled means the display options would have to be enabled at all time or the values would be invalid when the player would try to enable them
• having only one dual-purpose timer would mean only one could be kept track off while the display is disabled, and more importantly that switching between one display option and the other would become impossible once the story mode has been launched, as the calculated time would be incompatible.
• the overhead added by the timers running in background without display is negligible, with a few checks per frame and occasional operations (pause/unpause). The memory overhead is also tiny.

If there is some subelement of the timers which could be shared between them without harming the computation, why not ; but really, two standalone counters trumps in simplicity one dual-purpose timer which would have to do a lot of checks to guarantee integrity.

(Of course, I could also just dump the normal story mode timer, but considering that a)it has the potential to be useful for numerous players ; b)implementing it with correct saving and loading was a pain (I had the speedrun timer quite ready much sooner) ; I feel it would be a waste)

EDIT : As for the get() functions, I'll take a look.

[Alayan-stk-2]

I had a look at the get function return value. I can detect quite easily if the node doesn't exist, but it's only worthwhile if this profile had not started the story mode (otherwise, the speedrun values are invalidated, and the normal story timer values will be wrong).

So, considering that the next version will use a different profile folder anyway, I'm not sure if extra code to handle this case more nicely is worth it (would basically only be for current git users).

[hiker]

To summarise auria and my comments: am happy to accept a patch for a speedrun timer, which can be activated via command line, but I really do not want to to overload our gui with even more options (and esp. I don't like the non-speed-run timer). So I am closing this pull request, but encourage you to extract the speed-run timer and a command line option.

Thanks a lot!

[qwertychouskie]

Hmm, I actually rather liked the idea of having a not-so-strict timer for Story Mode. Most users would probably like a timer but would likely find the speedrun timer frustrating, since relaunching resets it completely. Since the UI settings page is mostly empty, is having one spinner or checkbox that big of a deal?

[hiker]

OK, I'll have a look at the pull request again. It seems very invasive, but am happy to re-evaluate.

[Alayan-stk-2]

I'll update the code after the beta. The options need to be adapted to the current options menu, and there are some other things I have to check (there are some issues when profile-switching iirc).

[hiker]

Some suggestions/brain-storming ideas ... feedback welcome:

1. Is there really a need for having two different timers? If STK provides one timer, can we just 'define' the speedrun time to be whatever this timer reports?
2. We already have a class providing real time measurement, is there a reason for using chrono?
3. Instead of a story mode timer, I would just use one timer that measures how long a player has played STK - i.e. just measure the time between start and end of stk ... yes, that leaves the problem of people idling, but might still be useful. And the display could just be on the user screen somewhere.

[Alayan-stk-2]

1. Is there really a need for having two different timers? If STK provides one timer, can we just 'define' the speedrun time to be whatever this timer reports?

The reason for this choice is simple : the story mode timer is meant to count only the time spent inside story mode until beating Nolok for the 1st time ; this includes a pausing whenever leaving the story mode. The speedrun timer keeps running if the player goes back to the main menu or in another mode ("pausing" and "speedrun" don't go well together).

My first attempts used a single timer, but I soon discovered that the conflicting rules created a headdache whenever the options selecting the timer to use was switched (the time of the previous timer was not valid for the new timer). With two timers, the options only control the display and there is no breaking of the underlying value when switching.

2. We already have a class providing real time measurement, is there a reason for using chrono?

The code I used at the beginning was done with chrono, and it worked very well so I never changed it and had not a special look at STK's own functions to handle this. When I will update the PR, I will have a look at it and see if it makes any difference. If it doesn't, I will convert the code to use STK's own functions.

3. Instead of a story mode timer, I would just use one timer that measures how long a player has played STK - i.e. just measure the time between start and end of stk ... yes, that leaves the problem of people idling, but might still be useful. And the display could just be on the user screen somewhere.

Such a timer of total time "played" (really with the game open because of idling as you point out) could indeed be interesting, but it's not quite the same. "You have played 43 hours" and "You finished the story mode in 2h38m" don't replace one another in my opinion. One is just about leaving the executable running, the other is more a kind of achievement.

[hiker]

Even if we use a 'time played' timer, we can add an additional message (and perhaps even store it in the player profile) when the story mode was finished (we can even print it everytime the final challenge is done ... it will only increase the time, who cares :) ). That would give us everything, one timer, and easy to install timer start/end calls.

[Alayan-stk-2]

There are several code incompatibilities here (as the files which this PR change were edited since), in addition to some functional issues.

I've made a rebased branch which also fix the issues, I'll open a new PR in time.