godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
86.16k stars 19.16k forks source link

UPNP does not find valid gateway #87953

Open dtgreene opened 4 months ago

dtgreene commented 4 months ago

Tested versions

Godot v4.2.stable

System information

Godot v4.2.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3060 Ti (NVIDIA; 31.0.15.3742) - AMD Ryzen 5 5600X 6-Core Processor (12 Threads)

Issue description

Calling discover() on a new UPNP instance works as expected and my router shows up as one of the devices found. However, get_gateway() always returns null without any further messages. UPNP is enabled on the router.

Router:

image

Script:

extends Node3D

func _ready():
    var upnp = UPNP.new()
    var upnp_result = upnp.discover(2000, 2, "")

    if upnp_result == OK:
        for i in upnp.get_device_count():
            var device = upnp.get_device(i)
            print("Description URL: " + str(device.description_url))
            print("IGD Status: " + str(device.igd_status))
            print("Is Valid Gateway: " + str(device.is_valid_gateway()))

        print(upnp.get_gateway())
    else:
        print("UPNP discover not OK: " + str(upnp_result))

Godot reports the IGD status as 5 or "Not Connected". I downloaded the miniupnpc client and added the mapping directly through the CLI. It also reported the status as not connected but continued anyway and worked successfully.

image

I'm not sure if this would still be considered a bug but I wonder if it's possible for Godot to behave this way as well since the mapping can still be added even with the not connected status.

Steps to reproduce

  1. Create a new UPNP instance
  2. Call upnp.discover()
  3. Check upnp.get_gateway()

Minimal reproduction project (MRP)

demo.zip

DanielKinsman commented 4 months ago

I think it's not up to Godot to behave the same way, it's up to your gdscript code? Can you not just call the add port mapping function on the individual device?

https://docs.godotengine.org/en/stable/classes/class_upnpdevice.html#class-upnpdevice-method-add-port-mapping

dtgreene commented 4 months ago

I did try adding the port mapping but the result was the same, failed due to no gateway. From what I can tell, this check happens in the thin wrapper Godot has around the underlying miniupnpc library:

https://github.com/godotengine/godot/blob/master/modules/upnp/upnp.cpp#L315-L323

brunoCreator commented 1 month ago

Is it your primary router or secondary, I have this problem when using my secondary router connected to another router that connects directly to the provider. When I use the primary router, it works fine. My secondary router is a tp-link, and my primary router is ZTE.

brunoCreator commented 1 month ago

It is unfortunate that the only free alternative for networking is UPnP, that is disabled on most routers. There should be a better option, without having to expend money in a tunneling service.

Calinou commented 1 month ago

@dtgreene Do you have two routers in your network setup (i.e. behind double NAT)? If so, you won't be reachable from the Internet unless one of the routers is on the DMZ (i.e. has all its ports forwarded) to avoid a double NAT situation.

dtgreene commented 1 month ago

@Calinou @brunoCreator I only have the one router setup in my house. I was thinking maybe Godot's implementation with miniupnpc just needed tweaking since you can add the mapping through their CLI but not with Godot.

Either way I learned that UPnP really isn't a great solution and I'd be better off with a NAT punchthrough or a relay server. I left this issue open because I thought this was still a genuine bug. I can close it though if it doesn't seem like a problem.