Skip to content
This repository has been archived by the owner on May 6, 2024. It is now read-only.

Update tty tools #283

Merged
merged 14 commits into from
Nov 2, 2021
Merged

Update tty tools #283

merged 14 commits into from
Nov 2, 2021

Conversation

heiner
Copy link
Contributor

@heiner heiner commented Oct 28, 2021

No description provided.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 28, 2021
@heiner heiner force-pushed the heiner/update-ttytools branch from 76b7146 to d008cf9 Compare October 28, 2021 16:50
Heinrich Kuttler added 4 commits October 28, 2021 19:24
For non-NetHack ttyrecs, the "\033[26;0f" doesn't work very well
as it can jump "up", not down. But there's no good way to do this
for all situations.
@heiner heiner force-pushed the heiner/update-ttytools branch from d008cf9 to 8fcddb3 Compare October 28, 2021 17:25
Heinrich Kuttler added 8 commits October 28, 2021 19:48
Also support -1 flag for classic ttyrecs.
As the comment says, could also only start rendering from the last
CLRSCREEN before FLAGS.start, which would be potentially faster.
The logic to do that requires introducing a few extra modes and
does not seem worth it at this point. The logic here is subtle
enough as it is.
@heiner heiner requested review from vzhong and cdmatters October 29, 2021 20:12
data = chr(char).encode("ascii", "backslashreplace")
arrow = "->"

data = str(data)[2:-1] # Strip b' and '
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably better to use a more 'proper' way of doing this that doesnt depend on how Python decides to represent a converted bytes to string?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree! But what is the proper way of doing exactly that? It's not data.encode("ascii", "backslashreplace") ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cdmatters
Copy link
Contributor

Im just testing it out and will add some screen shots to the PR

@cdmatters
Copy link
Contributor

Screenshot 2021-11-02 at 15 09 14

@cdmatters
Copy link
Contributor

Screenshot 2021-11-02 at 15 12 30

@heiner heiner merged commit 6187dc9 into main Nov 2, 2021
@heiner heiner deleted the heiner/update-ttytools branch November 2, 2021 17:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants