godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.13k stars 91 forks source link

Implement automatic host migration in peer-to-peer games #7912

Open PPillau opened 1 year ago

PPillau commented 1 year ago

Describe the project you are working on

A 4-player peer to peer FPS game using Godot‘s integrated multiplayer framework (MultiplayerSpawner, MultiplayerSynchronizer etc.)

Describe the problem or limitation you are having in your project

If the host, who is also a player (and not a dedicated server), leaves mid-game, the game session breaks up and all progress is lost.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Allow for „host migration“ - transfer the multiplayer authority/host over to another player in the session while keeping the game state completely intact (all replicated enemies, spawnables, positions, health, scores etc.)

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Don‘t really know, I am not that knowledgeable in this field. But I guess it should be done automatically by the multiplayer framework.

Probably would be good idea to expose signals for host-migration so one can display appropriate UI messages and pause game processes.

If this enhancement will not be used often, can it be worked around with a few lines of script?

I think it might be possible but definitely not in a few lines of code. On a disconnect you probably have to reset the game on all players left while saving the current state of all enemies and players and game data, then redo the session and manually spawn in everything the way it was before the host disconnect. Seems very tedious and error-prone. Definitely not something I could to myself or any newcomer to multiplayer principles.

Is there a reason why this should be core and not an add-on in the asset library?

The multiplayer framework is already a core part of Godot, and this seems like an essential feature for any peer to peer game. No way around it. Honestly without this feature peer to peer games in Godot are essentially not doable nor feasible.

DanielSnd commented 7 months ago

I'm doing a custom multiplayer peer as a module that uses a node.js socket.io server as a relay server. The host player requests a "room", the server allocates a room for them and sends them a 6 digit code. That player can now share that 6 digit code and other players can join them in the same room.

Since all players are actually just talking to the node.js server it's pretty trivial on that side to "change" who the host of the room is, whenever the host player leaves the server just picks a new player and sends a message to everyone letting them know which ID is the new host.

Now to implement the host migration on Godot's side I couldn't do it without touching engine code. In Godot the server's "unique id" is always 1, while other player's unique ids are random numbers above 1.

First thing I had to do was to not fire the peer_disconnected signal on my custom multiplayer peer if the peer that disconnected is the server (unique id == 1) as that would instantly despawn everything.

The next step of the puzzle was actually transferring the nodes that should belong to the new host so they'll be responsible for these nodes instead, while keeping the nodes that the new host used to own under a different ID to now be controlled by the Server Id (unique_id==1).

And that's the part that is not quite feasible without touching engine code. I need to read and manipulate private members from SceneReplicationInterface SceneMultiplayer and SceneCacheInterface. While I can "set_multiplayer_authority" to change the authority over the nodes, there are plenty of other places along those files that still reference the original unique ID that had authority over them, and to effectively pass these nodes over one needs to iterate through all those nodes and places replacing the IDs. Even to iterate through these nodes one needs to access private members in SceneMultiplayer and SceneReplicationInterface.

The only way to do that without engine code changes I believe would be to recreate and replace these files with "customized" versions.

This commit on my fork shows what exactly the changes I made to make it possible. Honestly it feels pretty hacky and probably not the best way to go about it, but it got the job done for me: https://github.com/DanielSnd/godot/commit/d613fdb2adc2ea7443eaf3ea0a1f432fd103cd92

I mainly created 2 methods:

void SceneMultiplayer::transfer_peer_id_ownership(int p_from_id, int p_to_id,bool remove_from_peer_afterwards) {
    replicator->transfer_peer_id_ownership(p_from_id,p_to_id,remove_from_peer_afterwards);
    cache->transfer_peer_id_ownership(p_from_id,p_to_id,remove_from_peer_afterwards);

    if(remove_from_peer_afterwards && connected_peers.has(p_from_id))
        connected_peers.erase(p_from_id);
}

This transfers the peer id ownership so everything that belonged or used the id p_from_id will now use the id p_to_id, with the option to erase any references to the actual peer with the "before" id, essentially removing them from consideration without having to issue a "disconnect" for the old ID.

HashSet<ObjectID> SceneReplicationInterface::get_replicator_object_ids() {
    return sync_nodes;
}

This let's me access the list of sync_nodes from my module so I can set the ownership on them directly, and delete the ones that aren't relevant to stay in the game, such as the original player node for the original host. I'm using a meta tag on the node to differentiate between something that the new host should keep versus something that should be deleted on the migration.

Here on the scene replicator interface is where it feels the most hacky to me. I need to take both PeerInfos and iterate through all the variables on the old host's peer info and add/set it to the new one.

void SceneReplicationInterface::transfer_peer_id_ownership(int p_from_id, int p_to_id,bool remove_from_peer_afterwards) {
    PeerInfo *pinfo_from = peers_info.getptr(p_from_id);
    PeerInfo *pinfo_to = peers_info.getptr(p_to_id);

    // Can't transfer if one of them doesn't exist
    if (pinfo_from == nullptr || pinfo_to == nullptr)
        return;

    for (ObjectID syncnode: pinfo_from->sync_nodes) {
        pinfo_to->sync_nodes.insert(syncnode);
    }
    for (ObjectID syncnode: pinfo_from->spawn_nodes) {
        pinfo_to->spawn_nodes.insert(syncnode);
    }
    for (KeyValue<ObjectID, uint64_t> &E : pinfo_from->last_watch_usecs) {
        pinfo_to->last_watch_usecs[E.key] = E.value;
    }
    for (KeyValue<uint32_t, ObjectID> &E : pinfo_from->recv_sync_ids) {
        pinfo_to->recv_sync_ids[E.key] = E.value;
    }
    for (KeyValue<uint32_t, ObjectID> &E : pinfo_from->recv_nodes) {
        pinfo_to->recv_nodes[E.key] = E.value;
    }
    pinfo_to->last_sent_sync = pinfo_from->last_sent_sync;

    for (KeyValue<ObjectID, TrackedNode> &E : tracked_nodes) {
        if (E.value.remote_peer == p_from_id) {
            E.value.remote_peer = p_to_id;
        }
        Node* get_node = get_id_as<Node>(E.value.id);
        // Now I need to check if this node's multiplayer authority is the old one and change it to new one if it is.
        if (get_node != nullptr && get_node->get_multiplayer_authority() == p_from_id) {
            // Here I'm not setting it recursively on the root because I could have a synchronizer with different authority as a direct child
            get_node->set_multiplayer_authority(p_to_id, false);
            for (int i = 0; i < get_node->get_child_count(); ++i) {
                Node* child_node = get_node->get_child(i);
                if (child_node != nullptr && child_node->get_multiplayer_authority() == p_from_id) {
                    // Here I am because I would need to create a new recursive method if I wanted to do this checking further, and at least
                    // on my own use it won't matter this deep into the node path.
                    child_node->set_multiplayer_authority(true);
                }
            }
        }
        Node* get_spawner = get_id_as<Node>(E.value.spawner);
        if (get_spawner != nullptr && get_spawner->get_multiplayer_authority() == p_from_id) {
            get_spawner->set_multiplayer_authority(p_to_id, false);
        }
        for (ObjectID synchronizer: E.value.synchronizers) {
            Node* get_synchronizer = get_id_as<Node>(synchronizer);
            if (get_synchronizer != nullptr && get_synchronizer->get_multiplayer_authority() == p_from_id) {
                get_synchronizer->set_multiplayer_authority(p_to_id, false);
            }
        }
    }
    if(remove_from_peer_afterwards) {
        peers_info.erase(p_from_id);
    }
}

I also do something similar on SceneCacheInterface. These methods are called on all peers, and since my module uses is_server_relay_supported all peers already know about all peers, so once all of them update the IDs for the new host accordingly host migration happens perfectly.

Below is my multiplayer test showcasing host migration. MultiplayerSpawners are used to spawn the text for each player, a MultiplayerSynchronizer is used to sync the text's position, and an RPC is used to trigger a tween that scales the text up and down to test that RPCs are working. On the top right corner of the window that's the host it states they're the host. Once I close the top left screen (it takes a moment to actually close) the window to the right of it becomes the host, and everything still works just like before :)

host_migration_is_a_go

Ideally I would love to have a more official and less hacky way of "fully transferring ownership" of things from one ID to another, and also a way to retrieve a list of relevant network objects. I'm not entirely sure if this request should be a new proposal or should be here.

For my own use I'm ok just moving ahead with my hacky solution, I'm already keeping a custom fork with other changes to the engine so it's not a problem to also keep these on top of it. But I'm also making my custom multiplayer peer module available on a public repository (and perhaps a GDExtension version) and not having a solution to allow for host migration would mean I would have to disable host migration for it (probably keep it in a separate branch with instructions on what to change in the engine to allow for it).