Fix terminal size at 80x24, write bzip2'ed ttyrecs #97
Conversation
facebook-github-bot
added
the
CLA Signed
heiner
force-pushed
the
terminal-ttyrec-stuff
branch
2 times, most recently
from
79d2da8
to
fdb89d4
Compare
This saves a lot of space (ttyrecs compress by 80%+). Only downside is that we can no longer peek into running ttyrecs this way, which was sometimes useful for debugging. To go back to that mode, we can #undefine NLE_BZ2_TTYRECS.
heiner
force-pushed
the
terminal-ttyrec-stuff
branch
from
fdb89d4
to
8e326c8
Compare
| @@ -139,7 +139,7 @@ | |||
| #define TIMED_DELAY | |||
| #endif | |||
|
|
|||
| /* #define AVOID_WIN_IOCTL */ /* ensure USE_WIN_IOCTL remains undefined */ | |||
| #define AVOID_WIN_IOCTL /* ensure USE_WIN_IOCTL remains undefined */ | |||
There was a problem hiding this comment.
This comment could probably be added to, to help future clarity.
| #define AVOID_WIN_IOCTL /* ensure USE_WIN_IOCTL remains undefined */ | |
| /* | |
| * ensure USE_WIN_IOCTL remains undefined | |
| * USE_WIN_IOCTL can set LI and CO global variables for terminal size, which we fix in nle.c | |
| */ | |
| #define AVOID_WIN_IOCTL |
There was a problem hiding this comment.
formatting of the comment can obvs change.
There was a problem hiding this comment.
I see where you're coming from, but I'd like to stick with the policy of not unduly changing the original NetHack source files, and this is one of them.
| #ifdef NLE_BZ2_TTYRECS | ||
| void *ttyrec_bz2; | ||
| #endif | ||
|
|
There was a problem hiding this comment.
| #ifdef NLE_BZ2_TTYRECS | |
| void *ttyrec_bz2; | |
| #endif | |
| #ifdef NLE_BZ2_TTYRECS | |
| void *ttyrec_bz2; | |
| #else | |
| FILE *ttyrec; | |
| #endif | |
I'm curious: why aren't we doing ^^^ ? We never write to the file if the macro is set. Do we not just run the risk of confusion by keeping this pointer around?
There was a problem hiding this comment.
Good question. Who exactly is responsible for this FILE is a bit of a tricky question. For now, I'd like to keep it around for debugging and to allow flushing (although we don't do that currently).
| @@ -201,7 +201,7 @@ def __init__( | |||
|
|
|||
| if self.savedir: | |||
| self._ttyrec_pattern = os.path.join( | |||
| self.savedir, "nle.%i.%%i.ttyrec" % os.getpid() | |||
| self.savedir, "nle.%i.%%i.ttyrec.bz2" % os.getpid() | |||
There was a problem hiding this comment.
pid is always going to be unique within the folder? Are we saving all ttyrecs to there or only the last N?
There was a problem hiding this comment.
Currently we are either saving all ttyrecs or none.
The pid is going to be the process id of the user's process which won't be reassigned until that process stops.
| @@ -31,6 +34,12 @@ init_nle(FILE *ttyrec) | |||
| assert(ttyrec != NULL); | |||
| nle->ttyrec = ttyrec; | |||
|
|
|||
| #ifdef NLE_BZ2_TTYRECS | |||
| int bzerror; | |||
| nle->ttyrec_bz2 = BZ2_bzWriteOpen(&bzerror, ttyrec, 9, 0, 0); | |||
There was a problem hiding this comment.
The block size. 9 is the highest and also what alt.org uses.
http://www.cs.cmu.edu/afs/cs/project/pscico-guyb/realworld/99/code/bzip2-0.9.5c/manual_3.html#SEC30
#undefine NLE_BZ2_TTYRECS.