Add option to save ttyrec files every "M" episodes (#260)

[heiner]

Taking over from #260. Fixes #256.

[heiner]

Downside of PR as-is: Just setting savedir no longer saves data, one needs to also set save_ttyrec_every to a nonzero value now.

Alternative: Default save_ttyrec_every to -1 which could mean "1" if savedir is not-None, "0" otherwise?

[cdmatters]

Downside of PR as-is: Just setting savedir no longer saves data, one needs to also set save_ttyrec_every to a nonzero value now.

Alternative: Default save_ttyrec_every to -1 which could mean "1" if savedir is not-None, "0" otherwise?

Could we not just have a setting where if savedir is set - then we save every x. So the off switch is controlled by savedir, and there is no second off switch.

[heiner]

Downside of PR as-is: Just setting savedir no longer saves data, one needs to also set save_ttyrec_every to a nonzero value now.
Alternative: Default save_ttyrec_every to -1 which could mean "1" if savedir is not-None, "0" otherwise?

Could we not just have a setting where if savedir is set - then we save every x. So the off switch is controlled by savedir, and there is no second off switch.

Yes.

My alternative suggestion was along those lines but not exactly that. The reason I went for something else first is that savedir has this additional twist where None means don't save, empty string means create a new random directory. I thought we could make the off switch the other flag instead...

[cdmatters]

Downside of PR as-is: Just setting savedir no longer saves data, one needs to also set save_ttyrec_every to a nonzero value now.
Alternative: Default save_ttyrec_every to -1 which could mean "1" if savedir is not-None, "0" otherwise?

Could we not just have a setting where if savedir is set - then we save every x. So the off switch is controlled by savedir, and there is no second off switch.

Yes.

My alternative suggestion was along those lines but not exactly that. The reason I went for something else first is that savedir has this additional twist where None means don't save, empty string means create a new random directory. I thought we could make the off switch the other flag instead...

From a linguistic point of view, i think this PR is right to make save_ttyrec_every=0 be OFF, rather than savedir=None, and its cleaner to remove te "" None case difference. I'm happy with this (breaking!) change