Feature Request - Implement dynamic file sizes and protocol versioning in PKSM bridge

[mrhappyasthma]

Using the sword/shield bridge from the Switch Checkpoint transfers the data correctly. But trying to go to 'storage' after receiving the save is crashing.

Based on the change log, it looks like support up to v1.2 was added. Some work might need to be done after the latest updates.

Attached the Luma3DS crash log, in case it's helpful crash_dump.txt crash_dump_00000000.zip

[piepie62]

This is a known problem with the SWSH bridge that needs a redesign of the bridge itself on both Checkpoint and PKSM's sides. The essential issue is that save sizes change between updates, which our current bridge system was never designed for. Thank you for the report, but yeah, already known

[mrhappyasthma]

Is it as simple as adding some kind of header data on the TCP connection specifying the incoming file size? Or is there significant amounts of work that needs to be done in addition to the size change?

(If it's not anything too unreasonable, I could take a stab at it. But I haven't touched these projects before so I'm coming in blind.)

[piepie62]

That would be the general idea, yeah; I'd also want the port to change in order to prevent improper format misreading, and probably a bit of versioning info. If you'd like to take a shot, be my guest; I hate networking code with a burning passion so it would be very appreciated

[mrhappyasthma]

Just updating the save file size 'works'. Although all of the sprites for the SWSH pokemon show as an egg. (Assuming this worked before, perhaps its some issue with data offsets?)

But yeah, I can try to take a stab at rewriting/improving some of the networking logic for this.

[piepie62]

So, updating the save file size "works", yes, but a major problem with that is that there are several valid SWSH save file sizes, based off of the order/amount of updates taken. As for sprites, that's normal; we don't have properly sized G8 sprites so I haven't built a sheet that includes any yet.

[mrhappyasthma]

Ah okay. Well if sprites are an external issue, looks like we just need to update the t3x file.

Adding support for versioning and sending/receiving files (with dynamic size) is definitely something I can look into it.

[piepie62]

Yeah, sprites are just mostly a "I don't do graphic design well, so trying to shrink the sprites and not have them look like ass is a bad thing to assign to me" thing. Same main thing a PKSM Switch version is stuck on, really.

[mrhappyasthma]

I can't help much with the assets, but i can take a look at the current TCP communication and try to whip up a reasonable protocol to make it a bit more robust to change and compliant with dynamic file sizes

[piepie62]

Yep, and thank you for that; was just explaining why it wasn't quite as simple as just updating the spritesheets

[mrhappyasthma]

So I was thinking something like this. Let me know if anyone has suggestions or concerns.

I tried to keep it fairly basic, while still allowing for some flexibility to add more error codes or upgrading protocol versions in the future.

One other thing that I wasn't sure of: the protocol is fairly self-contained. Did we want to consider sharing this code somewhere that makes it easy to import? Then other projects that use this protocol (e.g. Checkpoint or even folks who wants to write custom tooling) could just include this directly without needing to re-defining all the structs and enums. I wasn't sure if there was any preference of file location for this to make it easier to share. (If we really wanted to go extreme, this could be in its own lightweight dependency. But even within the structure of PKSM, I wasn't entirely sure where to put it. I could just keep it in utils but then it felt kind of undiscoverable.)

Let me know what folks think.

```cpp /** * This file defines the PKSM Bridge protocol API. * * Example data flow: * * Client -> Send `pksmBridgeRequest_s` to server and await acknowledgement of supported version * * Server: Checks the protocol version from the request to see if it is supported. * * <- Server send `pksmBridgeResponse_s`. If version is supported, the value matches * one sent from the client. If unsupported, then the value is * PKSM_BRIDGE_UNSUPPORTED_PROTOCOL_VERSION. * * Client: Parses the `pksmBridgeResponse_s` to expect either an error payload or a file payload. * * <- (if error) Server then sends `pksmBridgeError_s` * <- (if !error) Server then sends `pksmBridgeFile_s` * * Client: Conditionally receives the appropriate payload. And handles it appropriately. */ #ifndef PKSMBRIDGE_API_HPP #define PKSMBRIDGE_API_HPP const constexpr int PKSM_BRIDGE_LATEST_PROTOCOL_VERSION = 1; const constexpr int PKSM_BRIDGE_UNSUPPORTED_PROTOCOL_VERSION = -1; typedef struct { /** Used as a sentinel value to identify the request type. Contains 'PKSMBridge'. */ char protocol_name[10]; /** The requested protocol version to use. */ s8 protocol_version; } pksmBridgeRequest_s; typedef struct { /** Used as a sentinel value to identify the response type. Contains 'PKSMBridge'. */ char protocol_name[10]; /** * If the requested protocol_version is supported by the server, this value will * contain the same value to confirm support. If the requested version is not * supported, it will contain `PKSM_BRIDGE_UNSUPPORTED_PROTOCOL_VERSION`. */ s8 protocol_version; } pksmBridgeResponse_s; namespace pksm { /** An enum to define the various types of protocol errors. */ enum class BridgeErrorEnum : s16 { /** * This error code is used when a client requests a protocol version * that the server does not support. */ UNSUPPORTED_VERSION = -1 }; } // namespace pksm typedef struct { /** The error code ID. */ pksm::BridgeErrorEnum error_code; /** The size (in bytes) of the text in `error_message`. */ u32 error_message_size; /** A UTF-8 encoded error message. */ char *error_message; } pksmBridgeError_s; typedef struct { /** The size of the file in bytes. */ u64 file_size; /** * The file contents themselves. The size of these contents is specified * in `file_size`. */ char *file_contents; } pksmBridgeFile_s; #endif // PKSMBRIDGE_HPP ```

[mrhappyasthma]

Discussed offline.

Summary:

• Prefer camelcase
• Use clangformat for styling
• Don't typedef the structs. Global namespace is fine.
• Remove the _s suffix
• Move BridgeErrorEnum into the global namespace
• Fix the const constexpr redundancy
• Drop pksmBridgeError_s for v1 of protocol. Having a string field means we have to worry about translations. It's also not necessary with a single error case, so let's simplify.
• Sharing code for the protocol probably makes sense but may not be highest priority. (for now i'll just plan to implement in both places and take a TODO to refactor it out).