nex-go icon indicating copy to clipboard operation
nex-go copied to clipboard

[Enhancement]: Remove HPP from this library and put it into its own

Open jonbarrow opened this issue 9 months ago • 5 comments

Checked Existing

  • [x] I have checked the repository for duplicate issues.

What enhancement would you like to see?

HPP, while related to NEX, is fundamentally very different in terms of operation. Despite this libraries name being nex-go, it's real job is to provide the PRUDP protocol and some basic/common NEX/RDV related things like types. But HPP does not rely on ANY of the PRUDP related functionality here, which is the bulk of the scope of this library. It might be worth considering removing HPP support from this library and putting it into its own

https://github.com/PretendoNetwork/hpp-go does exist, but it was never used. If I remember correctly one of the reasons why we chose to bundle it here was to access some non-exported data and to share configurations for things like library versions, but I'm not sure those are still valid/worthwhile reasons anymore?. HPP is a totally separate idea from PRUDP, most games will never use it, and it is explicitly documented as an alternative to it, I'm not sure it makes sense to still try and put HPP support in the same place as PRUDP?

Any other details to share? (OPTIONAL)

Somewhat related to https://github.com/PretendoNetwork/pokemon-rumble-world-secure/pull/3 and https://github.com/PretendoNetwork/nex-go/issues/77

jonbarrow avatar Feb 15 '25 22:02 jonbarrow

HPP works the same way at the RMC level, which is very much NEX. In fact, RMC would be what games care about in the end when interacting with the server. We have even seen before that the HPP and PRUDP servers of games are interconnected: https://discord.com/channels/408718485913468928/881694640099700827/1199477693381611611

Because of this, I don't think HPP should be split into its own library. Even if that were to happen, I think both abstractions should be handled in the next layers nex-protocols-go and nex-protocols-common-go, since otherwise we would be duplicating a LOT of code and making a maintenance burden to keep both versions in the same level. Though if we were to do this, we would have to duplicate the handling, since the current code on those abstractions is very much reliant on nex.PacketInterface.

Other projects such as NintendoClients can handle HPP and PRUDP just fine since the backend is abstracted from the client implementations. I don't see any reason why we should do any different here?

DaniElectra avatar Feb 15 '25 22:02 DaniElectra

I don't see why we would need to duplicate any code? The HPP implementation here already works off of the interfaces and exported types/functions, and doesn't use any private fields that would be unavailable to another package. I just copy-pasted all of the hpp_*.go files into a fresh Go module, added nex-go/v2 as a dependency, updated the references to use the nex package rather than the current package, and it seems to be working just fine? Go isn't having any issues with this from what I can see, and the test server in test/hpp.go runs without issue in the fresh Go module after updating all the imports to hpp (of the parts that are HPP-specific). The HPP library can still import and use anything we export from here, so we shouldn't need to duplicate the effort? Moving HPP to it's own library would only move the HPP-specific parts

Also with regards to NintendoClients, while I think it's a great tool and I love and appreciate everything Kinnay does, I do have some gripes with how parts of it are structured. I don't think it's always the best example to use for structuring. Such as how he combines multiple NEX protocols into a single, massive, file at times (see friends.py, which are only related by the name "friends", and matchmaking.py which while related to each other having them in one file is rather untenable)

My reasoning for suggesting the split is that the transport is so different, and is very Nintendo-specific (HPP does not seem to exist in RDV titles), that it might be worth having separately. PRUDPLite is also Nintendo-specific, but it shares enough parts of the transport that I think it makes sense to still keep it here (It's still PRUDP just the "next stage" of it. The protocol and connection functions basically identically, just over WebSockets and with a slightly different, but still very similar, packet structure. Whereas HPP is an alternative to PRUDP, it's a completely different transport protocol, using HTTP instead, and uses very different mechanisms to achieve this such as it's reliance on the HTTP headers and really only using RMC). The name of this repo is, honestly, kinda misleading and a remnant of when I originally planned to have all of NEX in a single repo before deciding to layer it. Really what this library provides is PRUDP, with a bit of helper types for NEX stuff (like LibraryVersions and some common NEX/RDV types), it's mostly about the transport protocol

jonbarrow avatar Feb 15 '25 23:02 jonbarrow

I don't see why we would need to duplicate any code? The HPP implementation here already works off of the interfaces and exported types/functions, and doesn't use any private fields that would be unavailable to another package. I just copy-pasted all of the hpp_*.go files into a fresh Go module, added nex-go/v2 as a dependency, updated the references to use the nex package rather than the current package, and it seems to be working just fine? Go isn't having any issues with this from what I can see, and the test server in test/hpp.go runs without issue in the fresh Go module after updating all the imports to hpp (of the parts that are HPP-specific). The HPP library can still import and use anything we export from here, so we shouldn't need to duplicate the effort? Moving HPP to it's own library would only move the HPP-specific parts

Oh so you are suggesting something like this?

graph TD;
    nex-go-->hpp-go;
    nex-go-->nex-protocols-go;
    hpp-go-->nex-protocols-go;
    nex-protocols-go-->nex-protocols-common-go;

(Note: nex-protocols-go wouldn't import hpp-go, it imports the public PacketInterface from nex-go)

If that is the case then I'm open to this. I hadn't considered that the needed abstractions are exported

DaniElectra avatar Feb 15 '25 23:02 DaniElectra

Yes that's essentially what I was getting at. Just moving the HPP specific parts to their own library, importing the "common" stuff from here, just to separate the transports since they're so different

jonbarrow avatar Feb 16 '25 03:02 jonbarrow

That's fair, sounds good to me

hpp-go would actually be imported from nex-protocols-go here: https://github.com/PretendoNetwork/nex-protocols-go/blob/523bc12b34f1dad8b71e7446507a276b747533de/globals/respond.go#L42

But I'm good with that

DaniElectra avatar Feb 16 '25 09:02 DaniElectra