godotengine / godot

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

`multiplayer_peer.close()` doesnt make `multiplayer_peer` into an `OfflineMultiplayerPeer` after being an `ENetMultiplayerPeer` #81540

Open ana-rchy opened 1 year ago

ana-rchy commented 1 year ago

Godot version

v4.1.1.stable.mono.official [bd6af8e0e]

System information

Godot v4.1.1.stable.mono - Arch Linux #1 SMP PREEMPT_DYNAMIC - Vulkan (Compatibility) - NVIDIA GeForce RTX 3060 Ti (nvidia; 535.104.05) - AMD Ryzen 7 2700X Eight-Core Processor (16 Threads)

Issue description

Screenshot_ ig this was wrong

this also happens if the server closes while the client is connected unsure if this is an issue with other multiplayer peer types, as im not familiar with those

Steps to reproduce

  1. join a server as a client, check the output of print(multiplayer.multiplayer_peer)
  2. leave the server by running multiplayer.multiplayer_peer.close() (or close the server)
  3. check output of print(multiplayer.multiplayer_peer), itll be ENetMultiplayerPeer

Minimal reproduction project

enet-offline-bug.zip

ana-rchy commented 1 year ago

every day i fall deeper into the pit of despair that is networking

akien-mga commented 1 year ago

@ana-rchy Please keep your comments constructive, as per our Code of Conduct: https://godotengine.org/code-of-conduct/ You're not encouraging contributors to help debug and possibly fix this issue by disparaging their work.

ana-rchy commented 1 year ago

@ana-rchy Please keep your comments constructive, as per our Code of Conduct: https://godotengine.org/code-of-conduct/ You're not encouraging contributors to help debug and possibly fix this issue by disparaging their work.

networking in general is painful, regardless of implementation

AThousandShips commented 1 year ago

Where is this functionality promised? This isn't claimed in the documentation, as far as I can find at least

I see how this could be an interesting thing to have, but that's not how it's said to be working, and there seems to be confusion over what "offline" means here, it doesn't mean "the peer is offline" but "this is a peer that is always offline"

It mimics the behavior of a server with no peers connected.

Zireael07 commented 1 year ago

Multiplayer documentation is confusing to say the least :(

ana-rchy commented 1 year ago

Where is this functionality promised? This isn't claimed in the documentation, as far as I can find at least

I see how this could be an interesting thing to have, but that's not how it's said to be working, and there seems to be confusion over what "offline" means here, it doesn't mean "the peer is offline" but "this is a peer that is always offline"

It mimics the behavior of a server with no peers connected.

we atleast need some more 'official' way/documentation of determining whether a client is connected to other peers or not, because so far getting the array size of get_peers() is the only reliable way but its not as obvious when has_multiplayer_peer() and the different MultiplayerPeer types exist. maybe like, add a note to get_peers() that thats the recommended way of checking for connectedness, so that other people dont fall into using either has_multiplayer_peer() (assuming close() is used) or checking the type of MultiplayerPeer