Page MenuHomeWildfire Games

Nonvisual replay hashtesting option
ClosedPublic

Authored by elexis on May 28 2018, 1:11 AM.

Details

Summary

The non-visual replaymode is a bit awkward when one debugs OOS issues. It performs a hash-test only every 100 turns.
But sometimes we need to replay a commands.txt file in order to determine if the hash matches at a given turn or not.
So it should be possible to also test the quick hash and every full hash when requested.
This patch adds two commandline options and enables full hash computation by default.

Test Plan

Pass -hashtest-full=false -hashtest-quick=false or similar and possibly use it in conjunction with D1519.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

elexis created this revision.May 28 2018, 1:11 AM
Vulcan added a subscriber: Vulcan.May 28 2018, 1:17 AM

Successful build - Chance fights ever on the side of the prudent.

Link to build: https://jenkins.wildfiregames.com/job/differential/590/display/redirect

Visual Replay should use this parameter too, so that one can fastforward.
Perhaps the option could be reused for autostarted games too.

Notice the nonquick hash is not quick, so by default this behavior change will make it somewhat slower.
But probably not sufficiently slower and the non-visual replay user should be informed if there is an OOS, it's an investment.

source/ps/Replay.cpp
311 ↗(On Diff #6667)

This is quite noisy, perhaps the line can be ommitted. But then the reader has to infer successful hashtesting which is not too desirable when debugging errors.

temple accepted this revision.May 29 2018, 3:05 AM
temple added a subscriber: temple.

Works as expected.

This revision is now accepted and ready to land.May 29 2018, 3:05 AM

(As mentioned, testing the full hash every 20 turns instead of every 100 turns makes it a bit slower by default. But it seems it's better to split these two use cases so that one can maximize performance or debugging information. Wasn't 100% sure if other documentation needs to be updated, checking again. Thanks review)

In D1538#62508, @elexis wrote:

(As mentioned, testing the full hash every 20 turns instead of every 100 turns makes it a bit slower by default. But it seems it's better to split these two use cases so that one can maximize performance or debugging information. Wasn't 100% sure if other documentation needs to be updated, checking again. Thanks review)

(And apart from turn 0, every 100 turns were quick-hashes.)

In D1538#62512, @temple wrote:

(And apart from turn 0, every 100 turns were quick-hashes.)

I didn't notice, but it's true, tested on a22:

Turn 100 (500)...
hash ok (9ab7b6d29e3ef9cd6571ac6f39e666b1)

replay:

hash-quick 9ab7b6d29e3ef9cd6571ac6f39e666b1
turn 101 500

(How could we not need this before.)

This revision was automatically updated to reflect the committed changes.

OOS were easier to debug when they were really "bad" :p
Cheers for this!