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

New nex-go #26

Merged
merged 53 commits into from
Apr 7, 2024
Merged

New nex-go #26

merged 53 commits into from
Apr 7, 2024

Conversation

jonbarrow
Copy link
Member

@jonbarrow jonbarrow commented Nov 12, 2023

Draft since lots of things are changing still, but wanted to get this into more peoples hands

The goal of this PR is to bring this library up to date with the new nex-go rewrite in the works PretendoNetwork/nex-go#42. Eventually I would also like to refactor all the methods implemented here which assume a PID is a uint32. A PID is not always a uint32, it can be a uint64 on modern clients (the Switch)

Currently this library does still assume a client is a legacy client when reading PIDs, but we now have the ability to support modern clients here

@jonbarrow
Copy link
Member Author

jonbarrow commented Dec 10, 2023

Most common implementations now use the new protocol interfaces. The exceptions to this are Secure Connection and Matchmaking

Secure Connection does technically use the interface, but there are places where it just always assumes a PRUDPServer and casts, which makes it not compatible with the HPPServer

Matchmaking does not use the interface at all, which I would like to fix at some point as it's now the only common implementation to not do it this way

It might get verbose, but we can make use of Go's type assertions to work around these casts. For example in Matchmaking

server.OnClientRemoved(func(client *nex.PRUDPClient) {
	fmt.Println("Leaving")
	common_globals.RemoveClientFromAllSessions(client)
})

Could become

if server, ok := server.(*nex.PRUDPServer); ok {
	server.OnClientRemoved(func(client *nex.PRUDPClient) {
		fmt.Println("Leaving")
		common_globals.RemoveClientFromAllSessions(client)
	})
}

So that the OnClientRemoved call only happens if server asserts to a PRUDPServer. We would need many of these assertions though, for both the server and the client interfaces

@DaniElectra
Copy link
Member

DaniElectra commented Dec 10, 2023

I think we should be fine right now by assigning the interfaces since HPP isn't expected to work with this. Once we implement websockets, we can remove the casts and use a common interface for PRUDP and websockets for these cases

@DaniElectra
Copy link
Member

btw I have noticed that there is some inconsistent naming for the matchmaking protocols. For example, on nex-protocols-go we use match-making and match-making-ext but on the common protocols they are named matchmaking and matchmaking-ext. I think we should rename one of them for better consistency

@jonbarrow
Copy link
Member Author

I think we should be fine right now by assigning the interfaces since HPP isn't expected to work with this

I would like at some point to make HPP compatible, since it's my understanding that HPP is supposed to be a drop-in replacement for NEX, just over HTTP. Allowing for access to all protocols/methods normally accessible in the NEX server. But I agree that it's fine for right now. Our use case does not require this, it's mostly a "something nice we should do in the future" thing

btw I have noticed that there is some inconsistent naming for the matchmaking protocols. For example, on nex-protocols-go we use match-making and match-making-ext but on the common protocols they are named matchmaking and matchmaking-ext. I think we should rename one of them for better consistency

I agree. I had not noticed this. I'll update common since the protocols lib is correct

jonbarrow and others added 7 commits December 14, 2023 04:16
This field doesn't have a station URL set usually.
Now that the websockets server is integrated into PRUDPServer, we have
to question if we even want to remove these casts, since it doesn't make
much sense for HPP.

Also updaete SecureConnection::Register to support TCP addresses on the
client for websocket implementations.
@jonbarrow
Copy link
Member Author

@DaniElectra I updated everything to support the new type system. It's basically untested though, just did enough to get Go to stop showing errors/warnings

HPP support also needs to be re-added. Right now it assumes a PRUDPConnection. In most cases though this is just to remove a type assertion for the server variable, because there's TONS of places where we just do blind assertions without checking (which WILL lead to panics if the wrong client interacts with some protocols)

A solution to the PRUDPConnection assumption would be to just check the type of packet.Sender(). If it's a PRUDPConnection then use the connections endpoint to get the server like it is now, otherwise if it's a HPPClient use the server on the client

@DaniElectra DaniElectra merged commit 112d279 into main Apr 7, 2024
@binaryoverload binaryoverload deleted the nex-go-rewrite branch January 9, 2025 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants