Terminal Redirection over Rednet

[SquidDev]

Pinging @lyqyd and @apemanzilla

Background: I'm proposing to add a method of broadcasting terminal state over a TCP socket, and was planning to use TRoR for this. I've made some modifications to the original standard to help make this easier.

This is a very rough draft ATM: I'm going away over the weekend and wanted to run this up a flagpole to get some initial feedback. One of the key aims of this is to not require a Lua parser to achieve minimal functionality, therefore I've replaced most instances of textutils.serialize/.unserialize with a different format.

I'm happy to rename this to something which isn't an existing standard as it isn't backwards compatible with it.

Changes from the original standard/nsh implementation

• Packets cannot contain the \n character: this is used to separate multiple packets.
• The arbitary data cannot contain ;: this seems to lead to parsing ambiguities in TW if the terminal text contains ;.
• Add TY and TV packets to allow blitting a single line/the entire terminal.
• Removed TG packet: I'm not 100% sure of the purpose of this.
• Add TR packet to allow resizing the client's terminal: this allows the server to broadcast to multiple clients without caring what size they all are.
• TI sends width, height and color support. We could potentially merge the TD and TA packets.
• EV uses a comma separated list instead of textutils.serialize.

  Changes still needed

• [x] SQ, SR, SP and SC - @lyqyd I'm not 100% sure what is going on here. Added SP and SC.
• [x] Add TT packet.
• File transmitting. I don't know if we want this as a separate protocol.

Thoughts?

[dmarcuse]

I agree with most of this but I have a couple concerns:

• The TY/TT packet data could contain commas, which would break the structure. I propose we require the text data for these packets to be surrounded with double quotes similar to EV.
• I feel that file IO falls outside the scope of this standard, but it could be implemented as a set of F_ packets - FI for data about a path, FL for directory listings, FD for file data, etc.

[SquidDev]

In TT and TI the text has to be the same length as the foreground and background colors and so I didn't feel such delimiter was needed: we just need to know how long one of them is. However I'm fine with quoting it if people think it would be better.

nsh adds a file sending/receiving system which is what that comment is based on. However I agree that it would be better in a separate proposal.

[dmarcuse]

Ahh, good point. It might be a good idea to make that clear in the spec.

[lyqyd]

SQ, SR, SP, and SC are connection-negotiation packets, and are not strictly necessary for the actual transmission of screen data information. SP, however, is used to query client capabilities, so it may still be useful. Client capabilities are returned via another SP packet (or they're supposed to be, nsh is non-compliant!) in a string with hyphens surrounding each keyword. nsh currently uses -fileTransfer-extensions-, though I'm not sure where I was going with the extensions keyword. Servers would have the option to query client capabilities and clients have the option to answer.

TD and TA are safe to merge. I'd propose TQ (terminal query) to request the width/height/color TI information from the client. This packet would then nominally be sent any time term.getSize() or term.getColor() was called, and the server would be responsible for distinguishing which of the results needed to be passed back to the program, if applicable. This is a change I'll likely make in nsh at some point.

The format change to TT is sensible for your application, but I would prefer to use packet code TV for the reformatted packet. I'm happy with the TY format. The TT packet could then be added in its current form. That would allow servers to choose to use TT or TV for their mass screen updates, and it would be extremely easy to update nsh to look at client capabilities to know if TT is usable, or if a fallback to TV will be necessary. If we add TT and move your suggested format to TV, it would be important to note that TV is the default to use for full screen updates and that servers should use TT only if they are able to negotiate that with the client.

File transferring should be in a separate protocol. nsh builds it in to the client because of the capability-negotiation system. Clients incapable of file transfer won't include the fileTransfer keyword in their response, if they respond to capability queries at all. If you're interested in a list of F_ packets, you could start here.

TG is an artifact of the original implementation, which literally only transported term calls over rednet. Since the screen was on another computer, and no buffers were in use, cursor position needed to be requested from the computer with the screen. With buffers, it's less necessary, but nsh will retain it for its fallback bufferless mode.

The changes to EV are probably fine, though care must be taken that events with nil arguments in the middle don't end up having the later arguments dropped.

[SquidDev]

I think I've made all the changes in the above comments. Thanks for the especially detailed feedback @lyqyd. I think I'll leave SQ and SR and the F_ packets for separate extensions.

I'm not sure if you wanted TG in the spec or not. I don't really like using TI for multiple things as it then makes it dependent on which packet was sent to the client so it might be worth using TG as the response packet too?

[lyqyd]

Thanks for making those changes, this looks excellent!

For TG, we could add it and a TP response packet pair, as that would allow nsh to be fully compliant with some effort on my part. I'm a little reluctant to drop it from the spec entirely, as 50% of the use cases we're currently aware of want to include it. I'm open to more discussion on that point if someone feels strongly either way, though. I also don't like the idea of using TI for different data. The size/color merge was sensible, but I don't know that tacking on cursor position would be as well.

It may be worth taking a look at the S_ packet series and creating a new C_ packet series based on them specifically for connection negotiation and data transmission. I think I might spin up a PR for that sometime in the near future.

I think after we've decided what to do about TG, this should be ready to be looked at for general approval. I'm fairly happy with the way it's shaped up. Thanks for putting in the effort to get it written up, @SquidDev!

Could also tack on an extra .0 to match the semantic versioning scheme we use elsewhere, if you want. :wink:

[SquidDev]

.0 added! I've ended up using TG as the request and response packet - I'm happy to change this if need be. Thank you so much for the feedback @lyqyd!

[lyqyd]

I think I prefer TP in response to a TG, as that means that each T_ packet has exactly one purpose and expected contents. Was there a specific reason to use TG for both query and response?

I'm looking forward to updating nsh to be fully compliant with the standard!

[SquidDev]

Ahh, sorry: I'd missed that for whatever reason 🤦. I was thinking that SP uses the same code for query and response and so TG should too: I prefer not doing so though so TP is better.

[oeed]

Quick question, as it's a little ambiguous. As metadata is optional, if a packet doesn't have any metadata can the : and ; be omitted?

For example: <Packet code><Packet contents>

Also, apart from what Lyqyd was saying about T_, is there any reason the packet code needs to be two characters? I guess at this scale it doesn't really matter, but I'd imagine that it's possible to put it in to one character to make it a bit smaller.

[SquidDev]

Personally I feel that requiring : and ; keeps the packet structure uniform and easier to parse: you can simply scan for ; rather than checking whether : is there and then scanning for ; or not. I can't speak for @lyqyd though.

Two characters allows additional extensions to be added without having to revert to cryptic names: for instance nsh adds F_ packets to allow file transfer.

[lyqyd]

I think requiring :; is useful, as it cleanly delimits the packet code, the optional metadata area, and the data portion of the packet. Most of my operations on those packets are string.match using the :; combination to cleanly split out packet type code and data. This allows the possibility of eventual three-character packet codes if they become necessary, etc. So to clarify, the :; is required under this spec. It may be useful to specifically notate that the :; cannot be dropped in the no-metadata case, since the question came up!

@SquidDev's example with the F_ packets is spot-on.

Thanks for getting TP in there! I'm pretty pleased with this standard as it stands right now. I'd also like to invite @viluon to take a look at this point, if he hasn't seen it already, as I think we're moving close to acceptance.

[SquidDev]

Whilst I hesitate to add things at this stage, I would like people's thoughts on an additional packet.

Emulators tend to use a "polling" method of terminal handling: check to see if the terminal has changed and display the output. This means you are basically reduced to TV and TY. However there may be scenarios where the terminal changes at a point different to the cursor's position. To transmit this change you either have to send the entire terminal or set the cursor, blit the changed region, and restore the cursor.

To solve this I propose a TZ packet (not sure about the name) which contains the x and y position to start writing at and the data to blit (in the same format as TY). Like term.blit this would not change the cursor position, just the the buffer data.

Thoughts?

[dmarcuse]

While I don't see myself using it, I think it would be a useful addition. Would it also leave the cursor at the end of the text, or at its original position?

[SquidDev]

@apemanzilla Its primary purpose was in emulators - it came from deficiencies I'd found in the spec when mocking up a renderer for it. If you don't feel you'll need it for CCEmuX then I don't feel it needs to be in the spec - there is probably a better way and this just adds bloat.

[lyqyd]

I'm having a little trouble envisioning a scenario in which a ComputerCraft terminal manages to write to an area of the screen somewhere other than at the current cursor position. Is this intended for a case in which the server doesn't ever update the client's cursor position? I don't have any particular opposition to it, I'd just like to better understand the role it fills.

[SquidDev]

I'll try to explain this a little better. From what I can tell there are two ways of handling transmitting a terminal:

• Transmit the method call (such as term.write, term.blit) to the client. This is effectively what nsh does (and OpenComputers IIRC).
• Every update tick check for differences and send what has changed (or the entire terminal). This is what ComputerCraft does.

The issue with the latter is that the screen may have been updated and the cursor moved on. A simple example might be print("hello\nworld"). Under nsh this will get transmitted as (I think):

TW:;hello
TC:;1,2
TW:;world
TC:;1,3

However when checking each tick you don't have any of this information and so you'd probably have to do:

TY:;00000,ffffff,hello
TC:;1,2
TY:;00000,ffffff,world
TC:;1,3

Or if we have TZ:

TZ:;1,1,00000,ffffff,hello
TZ:;1,2,00000,ffffff,world
TC:;1,3

This means you don't have to jump the cursor about: It can be set to the new position directly and each region drawn. This becomes more beneficial when using the window API where multiple areas might be updating at once but the cursor position not actually "changing" (obviously it does but it is restored after each refresh) hence the TC packet doesn't even have to be sent.

I hope that makes sense. I might be missing a much more obvious solution though 😄.

[dmarcuse]

The way I'm planning to do it with CCEmuX is just relaying the calls from the emulation side (rather than rendering) which means we won't need TZ.

[lyqyd]

Ah, okay. I can see the value in TZ. For clarity, your first example has TV where it should have TY and the second example should have TZ instead, correct?

If TZ fills the single-line update use case, I think it would be better to have it pass only the Y-coordinate and the entire up-to-date line. Would that work for your use case? I don't feel like there's enough to distinguish it from TY presently, and that this change would make the intended use clearer. Bear in mind that ComputerCraft clients can't write anything to the screen without changing the cursor position.

The example with the print call is probably missing a dozen-plus packets, to be honest! Thankfully, they get summed up into one TT usually.

[SquidDev]

Yeah, sorry: I've fixed some the packets in the example above.

I'd rather it was just a partial line. The aim of this packet is to send a minimal update region, hence the x and y coordinates, and so sending the whole line when a couple of characters have changed doesn't make sense.

However, if we're not going to need this for CCEmuX I think we are OK to ignore this: it'll just add things to the spec which aren't needed.

[lyqyd]

Okay, if we're leaving out TZ, I'm satisfied with this spec as it stands.

[SquidDev]

I've updated with some spelling, grammar and stylistic fixes as found by @viluon. It would be helpful if everyone could have one last read through to see if we've missed anything.

Would people like me to squash commits?

[dmarcuse]

The client MAY send events such as key presses to the client. However the client MAY ignore these.

I believe you mean the server may ignore event packets?

Also, I think it would be a good idea to include an event extension so that the client knows whether events are being ignored or not.

[lyqyd]

I don't think there's any particular need to squash the commits. It doesn't look like we've required that in the past for PR submissions.

[SquidDev]

@apemanzilla Sorry for taking an age to respond.

The client MAY send events such as key presses to the client. However the client MAY ignore these.

I believe you mean the server may ignore event packets?

Yeah, I'll fix that now.

Also, I think it would be a good idea to include an event extension so that the client knows whether events are being ignored or not.

Currently the server doesn't send which extensions it supports. Is this something we want to add? I personally don't have a problem with a client sending event packets which will be ignored: what are other people's thoughts?

[dmarcuse]

I like the idea of sending extensions in both directions.

[lyqyd]

I don't have any problem with clients sending events that simply get ignored.

What capabilities would servers have? I don't see any particular problem with capability queries being bi-directional, as long as the spec allows them to be ignored by either/both ends.

[SquidDev]

I guess the server's SP packet could carry the supported capabilities?

There may be extensions where it would be helpful for the client to know whether it is supported. For instance the client could display a GUI if a particular extension is supported.

[lyqyd]

Alternatively, the SP packet could be sent with a payload of "clientCapabilities" to the client to request the list, or "serverCapabilities" to the server. This also allows other extensions to use SP packets for different things, if desired.

[SquidDev]

I'm generally against packets being context dependent: they mean different things depending on what packet the other side sent. I feel if different data needs to be sent then a different packet should be used.

I don't know what other people's thought are on this though?

[viluon]

@SquidDev absolutely. What if you miss a packet? Communication should be 100% context independent, unless you use an ACK packet system that ensures no data is lost.

[lyqyd]

We are still primarily developing these standards for use in ComputerCraft, where packet loss is essentially a non-issue.

I don't have any major objections to the server sending its capabilities and the client responding with its own list, I suppose.

[oeed]

We are still primarily developing these standards for use in ComputerCraft, where packet loss is essentially a non-issue.

I would have to agree, I can't really see it ever being a issue in the future. CraftOS 2 doesn't appear to support packet sending over the internet does it?

[SquidDev]

There are situations where the event buffer may become stuffed (>256 events). However these situations are far and few between and so aren't really worth worrying about. I'll add this information to the SP packet tonight.

[viluon]

We are still primarily developing these standards for use in ComputerCraft, where packet loss is essentially a non-issue.

Imagine communication between two PDAs, or a situation where the weather changes while the two peers are far apart @lyqyd.

[lyqyd]

I feel like those sorts of issues are beyond the scope of this standard. Lossless transport is a good subject for a separate standard, though.

[viluon]

Context dependency hardwired into a standard which uses this low a level of network communication? Sounds defective by design to me. If you can make a fairly straightforward step towards lossless transport, do it. Or is there any issue that would prevent you from doing so that I'm unaware of?

[SquidDev]

@viluon The whole TRoR protocol goes to pot if even one packet is dropped: consider the loss of one "cursor set" packet, all subsequent packets will be written to the wrong line (until another cursor set packet).

There is no reason someone cannot devise a TCP style system which ensures packets arrive correctly in order however that is not within the scope of this PR.

[viluon]

That's a good point @SquidDev. I guess it doesn't matter then. I was thinking maybe the standard could work well even without a whole new layer for reliable communication...

[SquidDev]

Right, sorry for taking so long to get to this. I've added a commit on server capabilities. Any other comments?

[dmarcuse]

There's a couple inconsistencies using color and colour (e.g. under Terminal broadcasting packets table) but other than that it looks good to me.

[lyqyd]

Looks good to me. I'm going to mark this as accepted, with a merge coming in a few days if we've had no other input.

[SquidDev]

Great, thank you so much to everyone (especially @lyqyd) for their feedback 😄 🎉.