Expose node network IDs to GDScript #105217
Conversation
| @@ -184,6 +185,9 @@ class SceneMultiplayer : public MultiplayerAPI { | |||
|
|
|||
| const HashSet<int> get_connected_peers() const { return connected_peers; } | |||
|
|
|||
| int get_network_id(Object* p_object, int p_peer_id) const; | |||
There was a problem hiding this comment.
I'm happy to implement has_network_id() and has_instance() if needed.
| // return -1; | ||
| // } | ||
|
|
||
| return nc->cache_id; |
There was a problem hiding this comment.
As far as I can tell, this is the ID that's ultimately used in the RPC packet, but correct me if I'm wrong.
elementbound
force-pushed
the
expose-network-node-id
branch
from
b8dbe89 to
7c047bb
Compare
| ObjectID oid = p_object->get_instance_id(); | ||
| const NodeCache *nc = nodes_cache.getptr(oid); | ||
|
|
||
| // if (!nc->confirmed_peers.has(p_peer_id) || !nc->confirmed_peers.get(p_peer_id) || !nc->recv_ids.has(p_peer_id)) { |
There was a problem hiding this comment.
Does it make sense to check here if p_peer_id has confirmed the remote ID?
There was a problem hiding this comment.
If the peer has not confirmed the id? Is there anything the user can do? Or can the RPC be queued until the id has been confirmed?
There was a problem hiding this comment.
Reworked this a bit. Now getting the ID will always return the locally assigned ID, and an extra method was added to check if that ID was confirmed by a given peer.
|
If I understand the code correctly, the way the ids work is that each peer assigns an id to their own version of the node and broadcasts it to all other peers right? So each peer knows how to refer to a node to any other peer, but the ids themselves are not the same for all peers. If this is to be used alongside RPCs, it means users cannot call rpc() once, but must instead iterate over all peers, get their own id, and then call rpc_id() for each peer individually. This can be alleviated by having the engine do the conversion internally when a node is detected in the rpc parameters. I guess it might still incur some performance penalty, as the payload must be re-encoded for each peer separately. Maybe the system can be reworked so that the peer that has authority over the node assigns the network id for all other peers? That would be way outside the scope of this PR in any case. |
This seems like a significant drawback. |
|
After thinking about this problem a bit more, I opened a new proposal here: This PR does not conflict with it, because even if the mechanism of the ids change, this is a lower-level API that would stay the same. |
|
@JoNax97 I think the issue of having to re-encode the packet for every target can be avoided, by using the locally assigned ID when talking to peers that know about it, and using the node path for peers that don't. This way, you get at most two buffers to encode, instead of one per peer. If memory serves right, Godot does this under the hood too for RPCs. |
|
Would you say godotengine/godot-proposals#12217 supersedes this pull request? |
|
No, I don't think so. I think it's fairer to say that this PR partially implements it. |
Implements godotengine/godot-proposals#11044