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

[Feature]: Client support #55

Open
1 task done
jonbarrow opened this issue May 20, 2024 · 2 comments
Open
1 task done

[Feature]: Client support #55

jonbarrow opened this issue May 20, 2024 · 2 comments
Labels
approved The topic is approved by a developer feature A feature request

Comments

@jonbarrow
Copy link
Member

Checked Existing

  • I have checked the repository for duplicate issues.

What feature do you want to see added?

Adding client support to the library. We're already very close to being there, once our stability issues are sorted out. Some refactoring will be needed, but I see this as being plausible.

Why do you want to have this feature?

Being able to use a client in the game language as the servers has a few benefits, but the big one is testing. @wolfendale has been poking around some our repos and has had an interest in our lack of tests. @shutterbug2000 and I have talked about using NintendoClients for automated testing for some time, but that comes at the cost of polluting our Go code with Python, which may not be desirable. Having a Go client would mean we can easily write _test.go tests for not just the networking libraries, but the servers as a whole.

The fact that Go can also compile down to PPC and as C shared libraries means there's potential for us to also ship our libraries for homebrew use, giving homebrew developers a NEX client to work with. Though this is only theoretical.

Any other details to share? (OPTIONAL)

We would need to be careful with this. Writing a bad client for tests might be worse than having no tests at all, so if we do decide to try and refactor some stuff to support making clients we need to be absolutely sure the client is compatible with existing servers. We can test this directly on the friends server, since it supports both PRUDPv0 and PRUDPv1, we should be fine. We can also compare it to the results from NintendoClients to make sure it stacks up.

@jonbarrow jonbarrow added awaiting-approval Topic has not been approved or denied feature A feature request labels May 20, 2024
@wolfendale
Copy link
Contributor

I do think that this would be a great thing to do for all the reasons you've raised above. I especially like the idea of having some robust tests especially for nex-go itself and this would give us a great way of doing that.

When I was first looking at the codebase I tried to set up some tests using the abstractions that are already there and several things made that quite difficult:

  • Some of the struct that we use are too tangled with the idea that they're used as part of a server. For example the PRUDPPacket struct has a pointer to a PRUDPServer which would make no sense from the perspective of a client
  • I had issues trying to correctly build a packet without access to certain private module functions (I can't remember the specifics, and looking back at this now, I probably just needed an instance of PRUDPV0Settings that I could create without a server instance, and that would be fine)
  • The way layers of the protocols are stacked is cumbersome. For example, in order to respond to client requests on a PRUDPServer we need to create a PRUDPEndpoint, all PRUDPEndpoints will attempt to read an RMC message from a packet, even if we don't care about RMC (for example testing lower level PRUDP connections)

I think these kinds of issues would probably be a pretty big refactor, and I think it's too hard to ignore these issues and make a client. A few things I'm thinking:

  • I hate proposing any big rewrites, and I'm aware I wasn't around for the work / planning that went into the current rewrite of the library, but it may be worth making some individual issues about codestyle / organisation issues like the above and discussing them separately, it may be that some of this can be done without affecting the public interfaces too drastically
  • It could be worth creating a client as a separate library initially, slowly refactoring both where sensible, until they can either: a) be merged into a single library or b) have common parts extracted into a separate library

Most of this is me just rambling, and I'm no golang expert so there could be aspects of how Go is usually written that I just don't get, but this is an interesting problem and would love to work on it more

@jonbarrow
Copy link
Member Author

My concern with splitting the server and client libraries is that it would essentially double our workload, since a large amount of logic would be duplicated.

With regard to structure and organization, a large part of this library was restructured in order to better reflect how the original Quazal Rendez-Vous library works. That was done to make RE a bit easier to do, to help with more accurate emulation, and to better support non-NEX clients. Though it's not possible to be 1:1, since QRV was written in C/Python which has design features we just can't implement here. So I'm open to modifications in some areas, but I'd like to keep it as close to QRV as possible.

As for the issues with making a client with the codebase as-is, this can probably largely be fixed with some more interface usage. In reality PRUDP/QRV is actually closer to a client-client p2p connection rather than a client-server connection. You're connecting directly to another PRUDPEndPoint, with the same kind of stream settings and all that which the client has. Right now we just make assumptions about being a server and take a few shortcuts. But those shortcuts can be removed. For instance rather than assuming everything is a server, a PRUDPConsumer (or similarly named) interface could be made. Packets and the like would then expect that rather than a server, and the client/server implementations would implement the interface.

We can definitely take some liberties for this. The actual internals of the library I'm happy to change just about as much as we need to. It's the public facing API and general structure of the library which I'd like to keep close to QRV.

@jonbarrow jonbarrow added approved The topic is approved by a developer and removed awaiting-approval Topic has not been approved or denied labels Jul 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved The topic is approved by a developer feature A feature request
Projects
Status: Todo
Development

No branches or pull requests

2 participants