-
-
Notifications
You must be signed in to change notification settings - Fork 605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Proposal] ANSI color support #293
Comments
Hey, we would certainly we interested in this. As the rendering, yes, I agree. We were interested in adding pygame by rendering which should be significantly faster than the current implementation with matplotlib |
Cool, I will create a PR accordingly! And I agree, it will be better if it will be a separate print function Regarding Pygame: unfortunately it isn't very portable, but if it provides speed benefits and can be added as a non-mandatory component, it might be good for Windows/Mac/Ubuntu users at least. |
Proposal
Currently when print(env) is used, it already prints an ASCII representation whereby each cell is printed as two letters AB, A being the cell type, and B being the cell color. Most terminal nowadays or at least for a few decades) support ANSI colors, so it would be good to have ANSI color support (using the second letter as color code instead of printing it), ideally as an optional flag.
Motivation
Personally I don't like Matplotlib visualization except for paper output, as it's overkill to render to an image to then open a window and display a grid compared to just printing it to the shell, hence this proposal. But there is another aspect: in the long-run it might also be good to make optional the dependencies which have nothing to do with the environment simulation itself (especially matplotlib, pillow, which are behemoths with native components which are known to cause portability issues on regular basis). Having a good ASCII renderer with ANSI colors could be a first step in this direction.
Pitch
I have implemented a POC, please let me know if I should create a PR of the following:
https://gist.github.com/patham9/2d2c0c125bc683ba600778123bed4b0a
This is how it looks like in Termux on Android:
Alternatives
Alternatively rendering could also be done with a different library, but separating rendering from Minigrid will make using the library less convenient and might cause potential version conflicts in the future, so I don't recommend it. In any case the grid logic should be clearly separated from the rendering code and libraries needed for rendering should be optional.
The text was updated successfully, but these errors were encountered: