Skip to content

Conversation

@JG-Adams
Copy link
Contributor

Direct response to feature request:

I added two things. A function: is_udp_socket(socket) to ensure it's UDP. and a bool ownsSocket as part of ENetHost to ensure it behave with internal mechanism for socket or to use external without creating or closing socket or changing the configuration of it. (Leaving it all to the implementer of their own network.)

You use your own socket by putting it in the new argument of enet_host_create() ENetSocket* externalUDPSocket.

You set it to NULL to use the internal socket as normally done.

This should theoretically work and will not break anything except you now need to fill in extra parameter. I am unable to test it for myself because I don't need to make my own network so I'm not sure how to do it either.

Things to test:
Use of external socket should work.
is_udp_socket should be valid.
External socket should only be managed by the programmer.

I added two things. a function is_udp_socket(socket) to ensure it's UDP. and a bool ownsSocket for host to ensure it behave with internal mechanism for socket or to use external without creating or closing socket or changing the configuration of it. (Leaving it all to the implementer of their own network.)

You use your own socket by putting it in the new argument of enet_host_create() ENetSocket* externalUDPSocket.

You set it to NULL to use the internal socket as normally done.

This should theoretically work. I am unable to test it for myself because I don't need to make my own network so I'm not sure how to do it either.

Things to test:
Use of external socket should work.
is_udp_socket should be valid.
External socket should only be managed by the programmer.
	modified:   test/cli-server.c
@inlife
Copy link
Member

inlife commented Feb 27, 2025

Hey there,

Thanks for the awesome work on this! One vital element that I noticed is that your suggested change is modifying and breaking the API compatibility.

I agree that functional is interesting, but I think this specific change remains an option to be used in a fork, and not merged here in the repo.

@JG-Adams
Copy link
Contributor Author

@inlife Thanks you for your kind word!
So it break API compatibility? Do you mean that adding an extra parameter to the existing function does that? If so what if I simply create new function with a different name for using socket then? This is for someone who requested a way to work with external network.

return (type == SOCK_DGRAM);
}
#endif
return 0; /* Error: Could not determine socket type */

Check warning

Code scanning / CodeQL

Poorly documented large function Warning

Poorly documented function: fewer than 2% comments for a function of 102 lines.
@JG-Adams
Copy link
Contributor Author

JG-Adams commented Feb 27, 2025

I'm having trouble trying to modify this.
Let us move to #73

JG-Adams added a commit to JG-Adams/enet that referenced this pull request Feb 27, 2025
I needed to re-request zpl-c#71 because codespace wasn't working for me.

The change I made is an attempt to address the API-compatibility concern. I assumed it had to do with how adding an extra parameter to existing function (enet_host_create) would cause inconsistence requirement. So I addressed this by moving it over to enet_host_create_ext() and reuse the code with enet_host_create calling it set to NULL. Making it behave exactly as one would expect.

Hope this is a proper solution! :)
@JG-Adams JG-Adams closed this Feb 27, 2025
@JG-Adams JG-Adams reopened this Mar 1, 2025
@JG-Adams JG-Adams closed this Mar 2, 2025
@JG-Adams
Copy link
Contributor Author

@inlife I looked into ABI. this is sort of hard to understand but I'm curious why it would matter here? I thought you wouldn't be having any issues since you're compiling your own dll and the main program can be compiled for it as well?

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