Reduce duration of delays when the game is running slowly

[nstoddard]

This makes autoexplore and similar actions feel much faster when the game is lagging due to e.g. a staff of conjuration, and the game feels a bit faster even when not lagging. It also reduces delays slightly in all cases, so e.g. the main menu animation runs ~5-10% faster, to match the 50ms delay it's apparently supposed to have.

This only affects the SDL version.

[nstoddard]

In some cases, this speeds up animations quite a bit, enough that I'm not sure if they were meant to run that fast. E.g. the animation of an ally using lightning/negation is quite a bit faster than before. We may need to increase some animation delays to account for that (or maybe not, if they're supposed to be faster than they currently are).

[flend]

Presumably increasing delays will affect other platforms too though?

[nstoddard]

Presumably increasing delays will affect other platforms too though?

Good point.

The required changes for the terminal version are nearly identical to this PR. I'm not very familiar with the WebBrogue code; it looks like its version of nextKeyOrMouseEvent uses blocking rather than polling, so the required changes are somewhat different. If we decide to increase delays, someone familiar with how to run WebBrogue will probably be needed to help (I think I can implement the changes, but someone will need to test them).

I actually prefer the slightly faster animations; I think this might be the intended speed, and we may not need to increase delays at all. Would anyone be interested in testing this PR, to see what they think of the faster animations?

[nstoddard]

I added support for the terminal version. I don't think this needs to support the web version unless we decide to increase delays (which I don't think is needed).

[tmewett]

Hard to say these faster speeds are intended if the game never ran at that speed during dev. If it's noticeably different I think we should try to preserve the old speeds using delays. It's going to be hard to get testers without releasing it

[nstoddard]

I did some more testing, and it turns out that rendering time changes depending on how much of the level you've explored. On an unexplored level, rendering takes 1-2 ms on my laptop; on a fully-explored level, it's 4-5 ms. The animation of using a staff, for instance, has a delay of 16 ms; with the delays from rendering, that's 17-21 ms. Also, rendering time probably depends a lot on hardware.

Increasing animation delays by more than 1 ms would make them slower in some cases than they were before, but increasing them by just 1 ms would have relatively little effect and I don't think it would be worth it.

@pender Do you have any thoughts on this? I'm guessing you were the one to decide on the animation delays.

[pender]

@pender Do you have any thoughts on this? I'm guessing you were the one to decide on the animation delays.

No opinions on this at all. When I was writing Brogue, I was developing and testing using a homebrew OS X platform implementation that was very slow to update compared with the current implementations. So development was biased toward low delays, and I didn't have a good way to feel out the ideal timings.

I'm generally inclined toward the fastest animations we can do that convey the information and maintain atmosphere. Roguelikes should be snappy! Maybe it's worthwhile for someone to review the animation times, feel out different values in Wizard Mode, and propose an overhaul.

[nstoddard]

I'm generally inclined toward the fastest animations we can do that convey the information and maintain atmosphere. Roguelikes should be snappy! Maybe it's worthwhile for someone to review the animation times, feel out different values in Wizard Mode, and propose an overhaul.

I filed an issue for this. This PR will be necessary before that can be worked on, though, since otherwise the effective animation delays depend on hardware, how much of the level has been explored, etc.

My own opinion is still that we should go ahead and make this change without increasing delays, and see what players think of the slightly faster animations.

[zenzombie]

Is is possible to allow the user to control the delay like how replay speed works?

[zenzombie]

Is is possible to allow the user to control the delay like how replay speed works?

The reason I ask is because I'm wondering if there a relationship between the delay settings and the user's hardware? I.e. With fixed delay settings, will they always be optimal for some hardware platforms but not others?

[nstoddard]

Is is possible to allow the user to control the delay like how replay speed works?

The reason I ask is because I'm wondering if there a relationship between the delay settings and the user's hardware? I.e. With fixed delay settings, will they always be optimal for some hardware platforms but not others?

Nope, this PR fixes that (it's the main purpose; the faster animations are a side effect of that).

Examples (with some of the numbers made up):

• Current Brogue release, fast hardware: autoexplore (for instance) has a 20 ms delay; if each frame takes 1 ms to process (simulate/render/etc), the total delay is 21 ms.
• Current Brogue release, slow hardware (or other reasons for slowness, e.g. spectral blades): if each frame takes 10 ms, autoexplore will take 20 ms + 10 ms = 30 ms (instead of the intended 20 ms).
• This PR, fast hardware: it subtracts the time it takes to simulate+render each frame from the animation delay; if rendering takes 1 ms, autoexplore delays for 19 ms, so the total is the 20 ms that autoexplore is supposed to take.
• This PR, slow hardware: if processing each frame takes 10 ms, it subtracts that from the autoexplore delay, so it still takes 20 ms total.
• This PR, really slow hardware (or with tons of spectral blades): if rendering takes 30 ms, autoexplore doesn't add any extra delay (whereas the current version would add 20 ms for a total of 50). In this case, the only way to speed up animations to their intended speed is to optimize the game.

Brogue actually already has logic for this, but it's currently only applied to a few animations (such as the water animations); this PR generalizes this code to work for all animations.

[zenzombie]

Ok thanks for the explanation. I should have time in the next couple days to do a bit of testing.

[zenzombie]

Ok so did some testing but I have fast hardware and don't notice a difference. It seems fine. Neither too fast nor too slow.

[zenzombie]

LGTM

[nstoddard]

Thanks for testing this! I reworded the changelog entry to try to clarify what this PR does.

[tmewett]

My only concern is that now pauseForMilliseconds doesn't actually mean pause for N milliseconds

[nstoddard]

My only concern is that now pauseForMilliseconds doesn't actually mean pause for N milliseconds

What about pauseUpToMilliseconds, or pauseUntilNextFrame? They'd also document that frames are the given number of milliseconds apart.

[flend]

My only concern is that now pauseForMilliseconds doesn't actually mean pause for N milliseconds

What about pauseUpToMilliseconds, or pauseUntilNextFrame? They'd also document that frames are the given number of milliseconds apart.

I think either is fine as long as pauseUntilNextFrame has a clear param name like (maxMillisecondsDelay).
Please make either change and let's merge this, it's a good change.

[tmewett]

hmm, actually rename means breaking platform API and this change is technically non-breaking. I'm just going to merge and abandon any more bikeshedding urges

[nstoddard]

hmm, actually rename means breaking platform API and this change is technically non-breaking. I'm just going to merge and abandon any more bikeshedding urges

Whoops, I only saw this after updating the name. Should I undo that?

[tmewett]

Oh noo, sorry! Looks like there's a conflict anyway so might as well undo, apologies