Skip to content

Conversation

@dhthwy
Copy link

@dhthwy dhthwy commented Nov 22, 2025

dfhack-run uses this.

I've done only basic testing.

Probably needs some more work.

Kind ofa hassle with the C-style coding and all the macros being used.

@dhthwy dhthwy marked this pull request as draft November 22, 2025 08:38
@dhthwy
Copy link
Author

dhthwy commented Nov 22, 2025

With the changes to Accept() in the PR

connection from: 127.0.0.1,28873 server: 127.0.0.1,5000
connection from: 192.168.1.175,60127 server: 192.168.1.175,5000

Old Accept() behavior:

connection from: 127.0.0.1,23189 server: 0.0.0.0,5000
connection from: 192.168.1.175,48817 server: 0.0.0.0,5000

This is with allow remote enabled.
The actual address the client connected to was lost in the old version. Users may have multiple network interfaces and such.

I don't believe the current behavior is correct.

@dhthwy
Copy link
Author

dhthwy commented Nov 22, 2025

I still need to go check if the changes agree with the API. There may be a case or two where it doesn't.

There also may be instances where the socket needs to be invalidated.

IMO IsValidSocket() should only be looking at the desciptor (or SOCKET) value itself and internally invalidated by CSimpleSocket when necessary, rather than mixing it up with socket error codes as it was before.

Additionally. I can write some tools and tests for this. Haven't looked to see if there are any Lua tests.

ASIO might be nice, but that's presumably a much bigger library, and much bigger effort.

@dhthwy
Copy link
Author

dhthwy commented Nov 23, 2025

I suppose this is good enough for review. I haven't tested yet on Windows, but seems to be good on Linux.

Also the whole TCP Flush() hack is stupid and just makes wasteful system calls. If a socket needs data sent immediately that is what TCP_NODELAY is for, and has to be set before data is written.

Future improvements:

support ipv6.

Initialize socket in the ctor instead of a separate Initialize() function.

Use poll() instead of the ancient select().

@dhthwy dhthwy marked this pull request as ready for review November 23, 2025 17:17
@dhthwy
Copy link
Author

dhthwy commented Nov 23, 2025

Works on Windows. ipv6 support should be easy enough but this will have to be merged first. Some of the existing bugs this patch addresses breaks ipv6.

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.

1 participant