Skip to content

might fix steam socket issue#342

Closed
Detanup01 wants to merge 3 commits intodevfrom
sns_fix
Closed

might fix steam socket issue#342
Detanup01 wants to merge 3 commits intodevfrom
sns_fix

Conversation

@Detanup01
Copy link
Owner

No description provided.

pInfo->m_hListenSocket = connect_socket->second.listen_socket_id;
if (connect_socket->second.real_port != SNS_DISABLED_PORT) {
pInfo->m_unIPRemote = network->getIP(connect_socket->second.remote_identity.GetSteamID());
pInfo->m_unPortRemote = connect_socket->first;
Copy link
Contributor

@otavepto otavepto Aug 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't that be the key of the dictionary? HSteamNetConnection (some random number)
Should it be connect_socket->second.real_port ?

This seems to be a bug in the original function as well (see above)

Steam_Networking_Sockets::set_steamnetconnectioninfo(...) {
...
pInfo->m_addrRemote.SetIPv4(network->getIP(..., connect_socket->first);
...
}

The 2nd arg should be the port I assume.

Copy link
Contributor

@otavepto otavepto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The arg for the real port might not be correct

@otavepto
Copy link
Contributor

otavepto commented Aug 31, 2025

There seem to be more instances where the random dictionary key is used inappropriately, but none are related to this PR, for example:

bool Steam_Networking_Sockets::send_packet_new_connection(HSteamNetConnection m_hConn)
{
...
    msg.mutable_networking_sockets()->set_connection_id_from(connect_socket->first);
...

Here I would assume set_connection_id_from would be set to the CSteamID of the current player. This whole class implementation might need a complete revisit unfortunately.

@Detanup01
Copy link
Owner Author

fucking cpp cant even understand anything here

@otavepto
Copy link
Contributor

Yeah I get that feeling too every time man 😆

@Detanup01
Copy link
Owner Author

i aint want to merge and back this since the common update so gonna create a new bracnh without that

@Detanup01 Detanup01 closed this Mar 8, 2026
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.

2 participants