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

Add DNS resolution in NetworkedMultiplayerEnet::create_client() #18154

Merged
merged 1 commit into from
Apr 13, 2018

Conversation

mhilbrunner
Copy link
Member

@mhilbrunner mhilbrunner commented Apr 12, 2018

Closes #12512.

Tested & works locally. Now you can actually use domains for your server with enet! (Or "localhost" instead of "127.0.0.1", I guess)

Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

Very good overall, thanks 👍

if (p_address.is_valid_ip_address()) {
ip = p_address;
} else {
ip = IP::get_singleton()->resolve_hostname(p_address);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe:

#ifdef GODOT_ENET
		ip = IP::get_singleton()->resolve_hostname(p_address);
#else
		ip = IP::get_singleton()->resolve_hostname(p_address, IP::TYPE_IPV4);
#endif

In case someone is using externally linked ENet we should try to resolve using IPv4 as IPv6 is not supported.
(not sure who uses that, but since we support it, we can try to do our best)

@@ -119,7 +119,7 @@ class NetworkedMultiplayerENet : public NetworkedMultiplayerPeer {
virtual int get_peer_port(int p_peer_id) const;

Error create_server(int p_port, int p_max_clients = 32, int p_in_bandwidth = 0, int p_out_bandwidth = 0);
Error create_client(const IP_Address &p_ip, int p_port, int p_in_bandwidth = 0, int p_out_bandwidth = 0);
Error create_client(const String &p_ip, int p_port, int p_in_bandwidth = 0, int p_out_bandwidth = 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm just being pedantic here, but can you change p_ip to p_address? (like you rightfully did in other parts 👍 )

@mhilbrunner
Copy link
Member Author

@Faless Thanks, fixed. 🎉

Copy link
Collaborator

@Faless Faless left a comment

Choose a reason for hiding this comment

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

Great job! 👍

@Faless Faless merged commit eac2863 into godotengine:master Apr 13, 2018
@akien-mga akien-mga added this to the 3.1 milestone Apr 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants