Skip to content
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

Finish decoupling TermWindow functionality from Interpreter, ScreenBuffer, and Telnet #2123

Open
hvacengi opened this issue Sep 21, 2017 · 3 comments
Assignees
Labels
enhancement Something new (not a bug fix) is being requested
Milestone

Comments

@hvacengi
Copy link
Member

Currently the TermWindow class still manages a lot of the logic that should be abstracted inside of Interpreter or ScreenBuffer. As a result, the telnet interface depends on the TermWindow class, which in turn depends on running KSP. Separating this functionality would go a long way towards allowing kOS to run outside of KSP itself, which would dramatically improve the developer's ability to test new functionality and enable compiling scripts outside of KSP. After implementing, I would probably work on taking PR #1552 a new direction.

@hvacengi hvacengi added the enhancement Something new (not a bug fix) is being requested label Sep 21, 2017
@hvacengi hvacengi self-assigned this Sep 21, 2017
@hvacengi hvacengi added this to the v1.1.4.0 milestone Sep 21, 2017
@Dunbaratu
Copy link
Member

Do you expect this to take a while? Such a refactor effectively freezes working on a lot of areas of the mod until it's done. I was thinking of looking in to make colorized terminal stuff, but will put it off if you want to tackle this.

@hvacengi
Copy link
Member Author

It probably depends. I think that if I dedicate a weekend to working on it I should be able to get the PR up, the trick is dedicating a weekend to it. My recommendation would be to not wait for this implementation before you do your work, but to be cognizant that these changes that are to come. The idea of abstraction may very well affect how you implement the coloring. If you were going to simply implement the coloring from within the string literals for each line (maybe as bbcode or some other markup), it probably doesn't change much, but if you were going to implement a separate color tracking system it may effect where you store the data.

The baseline interface should not assume that the display is capable of color rendering. Part of what I'm envisioning after this modification is that some one could add an HTML server terminal, or rig something up with an arduino.

The discussion about actual color implementation can be in a separate issue, but I think it's important that it is noted here so that we address that concern in whichever implementation happens first.

@Dunbaratu
Copy link
Member

Dunbaratu commented Sep 21, 2017

The color would have to be remembered somehow so it's possible to issue a redraw of the screen.
so that means adding color data to ScreenBuffer. Right now ScreenBuffer remembers "at position 53,20, the character is 0x0065" in a list of lists. To make color do-able requires having that list also remember the color at that spot.

The translation from that to how the color gets drawn on telnet as opposed to the terminal is all about how the UnicodeMapper works. I decided we can abstract that by just pretending a fake terminal type that does the super set of everything, and has our own "control codes" for it. This is how the system already works. Then supporting different kinds of output is just a mattter of writing a translation from that UnicodeMapper to the actual terminal (or HTML for what you mean).

Decoupling the terminal from ScreenBuffer may mostly be a matter of making Terminal into the same thing as a Telnet Terminal - have it read the UnicodeMapper commands and interpret them.

@hvacengi hvacengi modified the milestones: v1.1.4.0, v1.1.5.0 Dec 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Something new (not a bug fix) is being requested
Projects
None yet
Development

No branches or pull requests

2 participants