PR, Palworld Support (Tested on Minecraft and PalWorld) #57
Conversation
…com/gorcon/rcon) (Fixes auth issues, untested on other servers besides pal world). Added strictCommandPacketIdMatching parameter to RCON constructor. PalWorld does not respect packet id's (Always returns 0) so this was added to ignore the packet ids and match to the most recently sent command instead. Default is true.
|
Just tested on a MC server just and it works.
|
There was a problem hiding this comment.
Hi thanks for the contribution.
There was a lot of magic constants in the new implementation of RCON Packet.
I can see that there might be a bug where the buffer size is off by one resulting in a missing null pad.
For the strictCommandPacketIdMatching, does the server guarantee request response order? Or should the concurrent request be limited to 1. Even though the implementation is flawed I agree we could add this as an option.
Thanks @ExusAltimus Partial-Fix: #56 Discovered-in: #57
Co-authored-by: Alexander WB <[email protected]>
…ad lock (TaskCompletionSource.SetResult) when server drops connection. Validated multithreaded operation. Updated shell. Added keep alive packets.
There was a problem hiding this comment.
Updated the PR with requested changes, also added autoConnect property because PalWorld RCON implementation sucks so it drops the connection about every 20 seconds when a command isnt sent (I verified it wasn't related to RequestTimeout). PalWorld routinely kills the connection which lead to other issues I encountered such as a deadlock with the authentation task when attempting to use the same RCON instance and reconnect (The reader never flushed). Exposed 2 properties Authenticated and Connected, and a method SetPassword. The new version feels very stable on PalWorld even though its dropping the connection. Tested on minecraft, was able to kill the server, restart it and use the same RCON instance without needing to call ConnectAsync again.
|
Also forgive me but I dont contribute often, what do you mean by "Can merge if your rebase in master" |
|
To make the branch mergeable you need to merge or rebase it with master you can run this command: |
|
Thank you, I applied the changes you requested, I also merged changes from main |
|
I've been successfully using the RCON for palworld for about a week now, palworld auto disconnects the RCON every 20 seconds but autoconnect lets me reuse the same instance without issues
|
|
Can we get this merged? Thanks for everyone's work! |
|
Hey sorry for the delay the new version should be up shortly. |
Fixes #56
Updated the RCONPacket to use gorcon's implementation (https://github.com/gorcon/rcon/blob/master/packet.go).
This fixed the issues I was having with authentication, I suppose it should work with other servers since gorcon claims support with several.
Also added a parameter to RCON called strictCommandPacketIdMatching (default: true) which when enabled matches the command to the response packet (This was the existing behavior).
When set to false the command instead behaves as a queue and any response will complete the command taskSource.
This is because PalWorld always returns 0 for the response packet id (bad).