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

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

Open
1 task done
jonbarrow opened this issue Feb 15, 2025 · 5 comments
Open
1 task done
Labels
awaiting-approval Topic has not been approved or denied enhancement An update to an existing part of the codebase

Comments

@jonbarrow
Copy link
Member

jonbarrow commented Feb 15, 2025

Checked Existing

  • 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 PretendoNetwork/pokemon-rumble-world-secure#3 and #77

@jonbarrow jonbarrow added awaiting-approval Topic has not been approved or denied enhancement An update to an existing part of the codebase labels Feb 15, 2025
@DaniElectra
Copy link
Member

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?

@jonbarrow
Copy link
Member Author

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

@DaniElectra
Copy link
Member

DaniElectra commented Feb 15, 2025

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;
Loading

(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

@jonbarrow
Copy link
Member Author

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

@DaniElectra
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-approval Topic has not been approved or denied enhancement An update to an existing part of the codebase
Projects
None yet
Development

No branches or pull requests

2 participants