diff --git a/engine/src/main/java/org/terasology/engine/network/PingComponent.java b/engine/src/main/java/org/terasology/engine/network/PingComponent.java new file mode 100644 index 00000000000..d649935a44a --- /dev/null +++ b/engine/src/main/java/org/terasology/engine/network/PingComponent.java @@ -0,0 +1,34 @@ +// Copyright 2023 The Terasology Foundation +// SPDX-License-Identifier: Apache-2.0 +package org.terasology.engine.network; + +import org.terasology.engine.entitySystem.entity.EntityRef; +import org.terasology.gestalt.entitysystem.component.Component; + +import java.util.HashMap; +import java.util.Map; + +/** + * PingStockComponent stock the ping information of one user. + *

+ * Might be used to stock ping information and display it in future. + */ +public final class PingComponent implements Component { + + @Replicate + private Map pings = new HashMap<>(); + + public void setValues(Map values) { + pings.clear(); + pings.putAll(values); + } + + public Map getValues() { + return new HashMap<>(pings); + } + + @Override + public void copyFrom(PingComponent other) { + this.setValues(other.getValues()); + } +} diff --git a/engine/src/main/java/org/terasology/engine/network/PingStockComponent.java b/engine/src/main/java/org/terasology/engine/network/PingStockComponent.java deleted file mode 100644 index 46608e283cf..00000000000 --- a/engine/src/main/java/org/terasology/engine/network/PingStockComponent.java +++ /dev/null @@ -1,50 +0,0 @@ -// Copyright 2021 The Terasology Foundation -// SPDX-License-Identifier: Apache-2.0 -package org.terasology.engine.network; - -import org.terasology.engine.entitySystem.entity.EntityRef; -import org.terasology.gestalt.entitysystem.component.Component; - -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -/** - * PingStockComponent stock the ping information of one user. - *

- * Might be used to stock ping information and display it in future. - */ -public final class PingStockComponent implements Component { - - // TODO Map is not supported for replication (no type handler), - // therefore keys and values are replicated via lists. - // Not the best solution for performance but for <100 players and low update rates it should do the job - - @Replicate - private List pingKeys = new ArrayList<>(); - @Replicate - private List pingValues = new ArrayList<>(); - - public void setValues(Map values) { - pingKeys.clear(); - pingValues.clear(); - for (Map.Entry entry : values.entrySet()) { - pingKeys.add(entry.getKey()); - pingValues.add(entry.getValue()); - } - } - - public Map getValues() { - Map returnValues = new HashMap<>(); - for (int i = 0; i < pingKeys.size(); i++) { - returnValues.put(pingKeys.get(i), pingValues.get(i)); - } - return returnValues; - } - - @Override - public void copyFrom(PingStockComponent other) { - this.setValues(other.getValues()); - } -} diff --git a/engine/src/main/java/org/terasology/engine/network/PingSubscriberComponent.java b/engine/src/main/java/org/terasology/engine/network/PingSubscriberComponent.java deleted file mode 100644 index 34e11715a8c..00000000000 --- a/engine/src/main/java/org/terasology/engine/network/PingSubscriberComponent.java +++ /dev/null @@ -1,14 +0,0 @@ -// Copyright 2021 The Terasology Foundation -// SPDX-License-Identifier: Apache-2.0 -package org.terasology.engine.network; - -import org.terasology.gestalt.entitysystem.component.EmptyComponent; - -/** - * PingSubscriberComponent, only on the server system, will be added to a client entity when this client subscribe. - * Server will only send ping information to the clients subscribed. - *

- * It can be used to stock the ping information of users in future. - */ -public class PingSubscriberComponent extends EmptyComponent { -} diff --git a/engine/src/main/java/org/terasology/engine/network/ServerPingSystem.java b/engine/src/main/java/org/terasology/engine/network/ServerPingSystem.java index 31b6ff2fcd2..ccf0b876ee6 100644 --- a/engine/src/main/java/org/terasology/engine/network/ServerPingSystem.java +++ b/engine/src/main/java/org/terasology/engine/network/ServerPingSystem.java @@ -1,4 +1,4 @@ -// Copyright 2021 The Terasology Foundation +// Copyright 2023 The Terasology Foundation // SPDX-License-Identifier: Apache-2.0 package org.terasology.engine.network; @@ -8,7 +8,6 @@ import org.terasology.engine.entitySystem.systems.RegisterMode; import org.terasology.engine.entitySystem.systems.RegisterSystem; import org.terasology.engine.entitySystem.systems.UpdateSubscriberSystem; -import org.terasology.engine.logic.players.LocalPlayer; import org.terasology.engine.network.events.DisconnectedEvent; import org.terasology.engine.network.events.PingFromClientEvent; import org.terasology.engine.network.events.PingFromServerEvent; @@ -25,18 +24,21 @@ /** * This system implement the server ping to clients on need base. * It runs on the server, pings to all clients who subscribe this function. + * + * @see PingFromServerEvent + * @see PingFromClientEvent + * @see SubscribePingEvent + * @see UnSubscribePingEvent */ @RegisterSystem(RegisterMode.AUTHORITY) public class ServerPingSystem extends BaseComponentSystem implements UpdateSubscriberSystem { + /** The interval in which pings are sent, in milliseconds. */ private static final long PING_PERIOD = 200; @In private EntityManager entityManager; - @In - private LocalPlayer localPlayer; - private Map startMap = new HashMap<>(); private Map endMap = new HashMap<>(); @@ -52,54 +54,77 @@ public void initialise() { @Override public void update(float delta) { - long time = Duration.between(lastPingTime, Instant.now()).toMillis(); + Instant now = Instant.now(); + long time = Duration.between(lastPingTime, now).toMillis(); if (time > PING_PERIOD) { - - // Server ping to all clients only if there are clients who subscribe - if (entityManager.getCountOfEntitiesWith(PingSubscriberComponent.class) != 0) { - Iterable clients = entityManager.getEntitiesWith(ClientComponent.class); - for (EntityRef client : clients) { - if (client.equals(localPlayer.getClientEntity())) { - continue; - } - - // send ping only if client replied the last ping - Instant lastPingFromClient = endMap.get(client); - Instant lastPingToClient = startMap.get(client); - // Only happens when server doesn't receive ping back yet - if (lastPingFromClient != null && lastPingToClient != null && lastPingFromClient.isBefore(lastPingToClient)) { - continue; - } - - Instant start = Instant.now(); - startMap.put(client, start); - client.send(new PingFromServerEvent()); - } + // only collect ping information if anybody is interested + if (entityManager.getCountOfEntitiesWith(PingComponent.class) > 0) { + startPings(); + updateSubscribers(); + } else { + clear(); } + lastPingTime = now; + } + } - //update ping data for all clients - for (EntityRef client : entityManager.getEntitiesWith(PingSubscriberComponent.class)) { - PingStockComponent pingStockComponent; - if (!client.hasComponent(PingStockComponent.class)) { - pingStockComponent = new PingStockComponent(); - } else { - pingStockComponent = client.getComponent(PingStockComponent.class); - } - if (localPlayer != null && localPlayer.getClientEntity() != null) { - pingMap.put(localPlayer.getClientEntity(), new Long(5)); - } - pingStockComponent.setValues(pingMap); - client.addOrSaveComponent(pingStockComponent); - } + /** + * Clear internal maps, for instance, when there are no more subscribers. + */ + private void clear() { + startMap.clear(); + endMap.clear(); + pingMap.clear(); + } - lastPingTime = Instant.now(); + /** + * Send a ping signal ({@link PingFromServerEvent}) from the server to all + * clients. + * + * Any entity with a {@link PingComponent} is considered as subscriber. + * + * Clients are supposed to answer with {@link PingFromClientEvent} to confirm + * the ping. + */ + private void startPings() { + for (EntityRef client : entityManager.getEntitiesWith(ClientComponent.class)) { + sendPingToClient(client); + } + } + + /** + * Send a ping signal to the client. + */ + private void sendPingToClient(EntityRef client) { + Instant lastPingFromClient = endMap.get(client); + Instant lastPingToClient = startMap.get(client); + // Send ping only if the client has replied to the last ping. This happens when + // there is still a ping in-flight, that is, the server hasn't received an answer + // from this client yet. + if (lastPingFromClient != null && lastPingToClient != null + && lastPingFromClient.isBefore(lastPingToClient)) { + return; + } + + startMap.put(client, Instant.now()); + client.send(new PingFromServerEvent()); + } + + /** + * Update the ping stock ({@link PingComponent}) on all subscribers + */ + private void updateSubscribers() { + for (EntityRef client : entityManager.getEntitiesWith(PingComponent.class)) { + client.updateComponent(PingComponent.class, pingComponent -> { + pingComponent.setValues(pingMap); + return pingComponent; + }); } } @ReceiveEvent(components = ClientComponent.class) public void onPingFromClient(PingFromClientEvent event, EntityRef entity) { - Instant end = Instant.now(); - endMap.put(entity, end); + endMap.put(entity, Instant.now()); updatePing(entity); } @@ -119,19 +144,11 @@ public void onDisconnected(DisconnectedEvent event, EntityRef entity) { @ReceiveEvent(components = ClientComponent.class) public void onSubscribePing(SubscribePingEvent event, EntityRef entity) { - entity.addOrSaveComponent(new PingSubscriberComponent()); + entity.addOrSaveComponent(new PingComponent()); } @ReceiveEvent(components = ClientComponent.class) public void onUnSubscribePing(UnSubscribePingEvent event, EntityRef entity) { - entity.removeComponent(PingSubscriberComponent.class); - entity.removeComponent(PingStockComponent.class); - - //if there is no pingSubscriber, then clean the map - if (entityManager.getCountOfEntitiesWith(PingSubscriberComponent.class) == 0) { - startMap.clear(); - endMap.clear(); - pingMap.clear(); - } + entity.removeComponent(PingComponent.class); } } diff --git a/engine/src/main/java/org/terasology/engine/rendering/nui/layers/ingame/OnlinePlayersOverlay.java b/engine/src/main/java/org/terasology/engine/rendering/nui/layers/ingame/OnlinePlayersOverlay.java index fef906ab13a..ac5097062f4 100644 --- a/engine/src/main/java/org/terasology/engine/rendering/nui/layers/ingame/OnlinePlayersOverlay.java +++ b/engine/src/main/java/org/terasology/engine/rendering/nui/layers/ingame/OnlinePlayersOverlay.java @@ -1,34 +1,33 @@ -// Copyright 2021 The Terasology Foundation +// Copyright 2023 The Terasology Foundation // SPDX-License-Identifier: Apache-2.0 package org.terasology.engine.rendering.nui.layers.ingame; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.terasology.engine.entitySystem.entity.EntityManager; import org.terasology.engine.entitySystem.entity.EntityRef; import org.terasology.engine.logic.afk.AfkComponent; import org.terasology.engine.logic.players.LocalPlayer; import org.terasology.engine.logic.players.PlayerUtil; import org.terasology.engine.network.ClientComponent; -import org.terasology.engine.network.PingStockComponent; +import org.terasology.engine.network.PingComponent; import org.terasology.engine.network.events.SubscribePingEvent; import org.terasology.engine.network.events.UnSubscribePingEvent; +import org.terasology.engine.registry.In; +import org.terasology.engine.rendering.nui.CoreScreenLayer; import org.terasology.nui.Color; import org.terasology.nui.FontColor; import org.terasology.nui.databinding.ReadOnlyBinding; import org.terasology.nui.widgets.UIText; -import org.terasology.engine.registry.In; -import org.terasology.engine.rendering.nui.CoreScreenLayer; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; import java.util.Map; /** - * Overlay that lists all players that are currently online. + * Overlay that lists all players that are currently online and their pings. */ public class OnlinePlayersOverlay extends CoreScreenLayer { - private static final Logger logger = LoggerFactory.getLogger(OnlinePlayersOverlay.class); - private UIText text; @In @@ -43,69 +42,65 @@ public void initialise() { text.bindText(new ReadOnlyBinding() { @Override public String get() { - logger.info("localPlayer is: {}", localPlayer); - PingStockComponent pingStockComp = localPlayer.getClientEntity().getComponent(PingStockComponent.class); - if (pingStockComp == null) { - String playerListText = determinePlayerListText(); - return playerListText; - } else { - String playerAndPing = determinePlayerAndPing(pingStockComp); - return playerAndPing; - } + PingComponent pingComponent = localPlayer.getClientEntity().getComponent(PingComponent.class); + return determinePlayerList(getPingMap(pingComponent)); } }); } - private String determinePlayerListText() { - Iterable allClients = entityManager.getEntitiesWith(ClientComponent.class); - StringBuilder sb = new StringBuilder(); - boolean first = true; - for (EntityRef clientEntity : allClients) { - if (!first) { - sb.append("\n"); + /** + * Assemble a map from connected players (or clients) to their ping. + * + * If the ping component is null, the connected clients are determined by looking at entities with the {@link ClientComponent}. + * In this case, the ping values are {@code null}. + * + * @param pingComponent component with information on connected players, or {@code null} if not present + * @return a mapping from connected clients to their respective ping (or {@code null} if the ping cannot be determined) + */ + private Map getPingMap(PingComponent pingComponent) { + //TODO: There's a noticeable delay when opening the overlay before the first ping comes in and all players are shown. + // We could either try to match the entity refs here, or pre-fill the component sooner in the ServerPingSystem. + if (pingComponent != null) { + return pingComponent.getValues(); + } else { + Map pings = new HashMap<>(); + for (EntityRef client : entityManager.getEntitiesWith(ClientComponent.class)) { + pings.put(client, null); } - ClientComponent clientComp = clientEntity.getComponent(ClientComponent.class); - sb.append(PlayerUtil.getColoredPlayerName(clientComp.clientInfo)); - first = false; + return pings; } - return sb.toString(); } - private String determinePlayerAndPing(PingStockComponent pingStockComponent) { - Map pingMap = pingStockComponent.getValues(); - StringBuilder sb = new StringBuilder(); - boolean first = true; - for (Map.Entry entry : pingMap.entrySet()) { - EntityRef clientEntity = entry.getKey(); - if (clientEntity == null || clientEntity.getComponent(ClientComponent.class) == null) { - logger.warn("OnlinePlayersOverlay skipping a null client entity or component"); - continue; - } + /** + * Create multi-line string, containing one line per connected player. + */ + private String determinePlayerList(Map pings) { + List lines = new ArrayList<>(); + for (Map.Entry entry : pings.entrySet()) { + lines.add(determinePlayerLine(entry.getKey(), entry.getValue())); + } + return String.join("\n", lines); + } - if (!first) { - sb.append("\n"); - } + /** + * Create a single-line string with the player name and their ping. + * + * 

+     *      [AFK]    Player4612                           42ms
+     *      -------- -------------------------------- --------
+     *         8                  32                      8
+     *
+ */ + private String determinePlayerLine(EntityRef client, Long ping) { + ClientComponent clientComp = client.getComponent(ClientComponent.class); + AfkComponent afkComponent = client.getComponent(AfkComponent.class); - ClientComponent clientComp = clientEntity.getComponent(ClientComponent.class); - AfkComponent afkComponent = clientEntity.getComponent(AfkComponent.class); - if (afkComponent != null) { - if (afkComponent.afk) { - sb.append(FontColor.getColored("[AFK]", Color.red)); - sb.append(" "); - } - } - sb.append(PlayerUtil.getColoredPlayerName(clientComp.clientInfo)); - sb.append(" "); - Long pingValue = pingMap.get(clientEntity); - if (pingValue == null) { - sb.append("-"); - } else { - sb.append(pingValue.toString()); - sb.append("ms"); - } - first = false; - } - return sb.toString(); + String prefix = (afkComponent != null && afkComponent.afk) ? FontColor.getColored("[AFK]", Color.red) : ""; + String displayName = PlayerUtil.getColoredPlayerName(clientComp.clientInfo); + String displayPing = (ping != null) ? ping + "ms" : FontColor.getColored("---", Color.grey); + //TODO: the formatting does not work well since we're not using a mono-spaced font. we should investigate whether we can use + // a different UI element or at least a monospaced font to align prefix, player name, and ping better. + return String.format("%-8s%-32s%8s", prefix, displayName, displayPing); } @Override diff --git a/engine/src/main/resources/org/terasology/engine/assets/ui/onlinePlayersOverlay.ui b/engine/src/main/resources/org/terasology/engine/assets/ui/onlinePlayersOverlay.ui index 22b6268545f..75a5b4342d9 100644 --- a/engine/src/main/resources/org/terasology/engine/assets/ui/onlinePlayersOverlay.ui +++ b/engine/src/main/resources/org/terasology/engine/assets/ui/onlinePlayersOverlay.ui @@ -7,7 +7,7 @@ { "type": "UIBox", "layoutInfo": { - "width": 200, + "width": 400, "use-content-height": true, "position-horizontal-center": {}, "position-vertical-center": {} diff --git a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandler.java b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandler.java index 0c75399f6de..ebec6d4dd64 100644 --- a/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandler.java +++ b/subsystems/TypeHandlerLibrary/src/main/java/org/terasology/persistence/typeHandling/coreTypes/GenericMapTypeHandler.java @@ -62,13 +62,18 @@ private PersistedData serializeEntry(Map.Entry entry, PersistedDataSeriali @Override public Optional> deserialize(PersistedData data) { + + Map result = Maps.newLinkedHashMap(); + + if (data.isNull()) { + return Optional.of(result); + } + if (!data.isArray() || data.isValueMap()) { logger.warn("Incorrect map format detected: object instead of array.\n" + getUsageInfo(data)); return Optional.empty(); } - Map result = Maps.newLinkedHashMap(); - for (PersistedData entry : data.getAsArray()) { PersistedDataMap kvEntry = entry.getAsValueMap(); PersistedData rawKey = kvEntry.get(KEY); @@ -101,7 +106,7 @@ private String getUsageInfo(PersistedData data) { " \"mapName\": [\n" + " { \"key\": \"...\", \"value\": \"...\" }\n" + " ]\n" + - "but found \n'{}'" + data + "'"; + "but found \n'" + data + "'"; } @Override