Skip to content

Commit

Permalink
Fix Skin Caching
Browse files Browse the repository at this point in the history 
Changes:
* Instead of caching a skin based upon the player we cached it based upon the textureURL. This means multiple players with the same skin will benefit from the cache and more importantly will mean a player changing their skin will not get a false cache hit.
* This should fix all issues with SkinRestorer and will now correctly show the skin both to the player themselves and to other players

Closes GeyserMC#518
  • Loading branch information
Brendan Grieve committed May 28, 2020
1 parent 14fcd77 commit 956664d
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 60 deletions.
6 changes: 2 additions & 4 deletions ...c/connector/network/translators/bedrock/BedrockSetLocalPlayerAsInitializedTranslator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,8 @@ public void translate(SetLocalPlayerAsInitializedPacket packet, GeyserSession se
session.login();

for (PlayerEntity entity : session.getEntityCache().getEntitiesByType(PlayerEntity.class)) {
if (!entity.isValid()) {
// async skin loading
SkinUtils.requestAndHandleSkinAndCape(entity, session, skinAndCape -> entity.sendPlayer(session));
}
// async skin loading
SkinUtils.requestAndHandleSkinAndCape(entity, session, skinAndCape -> entity.sendPlayer(session));
}
}
}
Expand Down
22 changes: 9 additions & 13 deletions connector/src/main/java/org/geysermc/connector/utils/SkinProvider.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public class SkinProvider {
public static final Skin EMPTY_SKIN = new Skin(-1, "steve", STEVE_SKIN);
public static final byte[] ALEX_SKIN = new ProvidedSkin("bedrock/skin/skin_alex.png").getSkin();
public static final Skin EMPTY_SKIN_ALEX = new Skin(-1, "alex", ALEX_SKIN);
private static Map<UUID, Skin> cachedSkins = new ConcurrentHashMap<>();
private static Map<String, Skin> cachedSkins = new ConcurrentHashMap<>();
private static Map<UUID, CompletableFuture<Skin>> requestedSkins = new ConcurrentHashMap<>();

public static final Cape EMPTY_CAPE = new Cape("", "no-cape", new byte[0], -1, true);
Expand All @@ -68,7 +68,7 @@ public class SkinProvider {
public static String EARS_GEOMETRY_SLIM;

private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper();
private static final int CACHE_INTERVAL = 8 * 60 * 1000; // 8 minutes
private static final int CACHE_INTERVAL = 30 * 60 * 1000; // 30 minutes

static {
/* Load in the normal ears geometry */
Expand Down Expand Up @@ -103,10 +103,6 @@ public class SkinProvider {
EARS_GEOMETRY_SLIM = earsDataBuilder.toString();
}

public static boolean hasSkinCached(UUID uuid) {
return cachedSkins.containsKey(uuid);
}

public static boolean hasCapeCached(String capeUrl) {
return cachedCapes.containsKey(capeUrl);
}
Expand Down Expand Up @@ -138,26 +134,26 @@ public static CompletableFuture<Skin> requestSkin(UUID playerId, String textureU
if (textureUrl == null || textureUrl.isEmpty()) return CompletableFuture.completedFuture(EMPTY_SKIN);
if (requestedSkins.containsKey(playerId)) return requestedSkins.get(playerId); // already requested

if ((System.currentTimeMillis() - CACHE_INTERVAL) < cachedSkins.getOrDefault(playerId, EMPTY_SKIN).getRequestedOn()) {
if ((System.currentTimeMillis() - CACHE_INTERVAL) < cachedSkins.getOrDefault(textureUrl, EMPTY_SKIN).getRequestedOn()) {
// no need to update, still cached
return CompletableFuture.completedFuture(cachedSkins.get(playerId));
return CompletableFuture.completedFuture(cachedSkins.get(textureUrl));
}

CompletableFuture<Skin> future;
if (newThread) {
future = CompletableFuture.supplyAsync(() -> supplySkin(playerId, textureUrl), EXECUTOR_SERVICE)
.whenCompleteAsync((skin, throwable) -> {
if (!cachedSkins.getOrDefault(playerId, EMPTY_SKIN).getTextureUrl().equals(textureUrl)) {
if (!cachedSkins.getOrDefault(textureUrl, EMPTY_SKIN).getTextureUrl().equals(textureUrl)) {
skin.updated = true;
cachedSkins.put(playerId, skin);
cachedSkins.put(textureUrl, skin);
}
requestedSkins.remove(skin.getSkinOwner());
});
requestedSkins.put(playerId, future);
} else {
Skin skin = supplySkin(playerId, textureUrl);
future = CompletableFuture.completedFuture(skin);
cachedSkins.put(playerId, skin);
cachedSkins.put(textureUrl, skin);
}
return future;
}
Expand Down Expand Up @@ -255,7 +251,7 @@ public static CompletableFuture<SkinGeometry> requestBedrockGeometry(SkinGeometr

public static void storeBedrockSkin(UUID playerID, String skinID, byte[] skinData) {
Skin skin = new Skin(playerID, skinID, skinData, System.currentTimeMillis(), true, false);
cachedSkins.put(playerID, skin);
cachedSkins.put(skin.getTextureUrl(), skin);
}

public static void storeBedrockCape(UUID playerID, byte[] capeData) {
Expand All @@ -275,7 +271,7 @@ public static void storeBedrockGeometry(UUID playerID, byte[] geometryName, byte
* @param skin The skin to cache
*/
public static void storeEarSkin(UUID playerID, Skin skin) {
cachedSkins.put(playerID, skin);
cachedSkins.put(skin.getTextureUrl(), skin);
}

/**
Expand Down
93 changes: 50 additions & 43 deletions connector/src/main/java/org/geysermc/connector/utils/SkinUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import com.nukkitx.protocol.bedrock.data.SerializedSkin;
import com.nukkitx.protocol.bedrock.packet.PlayerListPacket;

import com.nukkitx.protocol.bedrock.packet.PlayerSkinPacket;
import lombok.AllArgsConstructor;
import lombok.Getter;

Expand Down Expand Up @@ -88,7 +89,7 @@ public static PlayerListPacket.Entry buildEntryManually(UUID uuid, String userna
String geometryName, String geometryData) {
SerializedSkin serializedSkin = SerializedSkin.of(
skinId, geometryName, ImageData.of(skinData), Collections.emptyList(),
ImageData.of(capeData), geometryData, "", true, false, !capeId.equals(SkinProvider.EMPTY_CAPE.getCapeId()), capeId, uuid.toString()
ImageData.of(capeData), geometryData, "", true, false, !capeId.equals(SkinProvider.EMPTY_CAPE.getCapeId()), capeId, skinId
);

PlayerListPacket.Entry entry = new PlayerListPacket.Entry(uuid);
Expand Down Expand Up @@ -202,48 +203,54 @@ public static void requestAndHandleSkinAndCape(PlayerEntity entity, GeyserSessio
}
}

if (entity.getLastSkinUpdate() < skin.getRequestedOn()) {
entity.setLastSkinUpdate(skin.getRequestedOn());

if (session.getUpstream().isInitialized()) {
PlayerListPacket.Entry updatedEntry = buildEntryManually(
entity.getUuid(),
entity.getUsername(),
entity.getGeyserId(),
entity.getUuid().toString(),
skin.getSkinData(),
cape.getCapeId(),
cape.getCapeData(),
geometry.getGeometryName(),
geometry.getGeometryData()
);

// If it is our skin we replace the UUID with the authdata UUID
if (session.getPlayerEntity() == entity) {
// Copy the entry with our identity instead.
PlayerListPacket.Entry copy = new PlayerListPacket.Entry(session.getAuthData().getUUID());
copy.setName(updatedEntry.getName());
copy.setEntityId(updatedEntry.getEntityId());
copy.setSkin(updatedEntry.getSkin());
copy.setXuid(updatedEntry.getXuid());
copy.setPlatformChatId(updatedEntry.getPlatformChatId());
copy.setTeacher(updatedEntry.isTeacher());
updatedEntry = copy;
}

PlayerListPacket playerRemovePacket = new PlayerListPacket();
playerRemovePacket.setAction(PlayerListPacket.Action.REMOVE);
playerRemovePacket.getEntries().add(updatedEntry);
session.sendUpstreamPacket(playerRemovePacket);

PlayerListPacket playerAddPacket = new PlayerListPacket();
playerAddPacket.setAction(PlayerListPacket.Action.ADD);
playerAddPacket.getEntries().add(updatedEntry);
session.sendUpstreamPacket(playerAddPacket);

if(entity.getUuid().equals(session.getPlayerEntity().getUuid())) {
session.fetchOurSkin(updatedEntry);
}
entity.setLastSkinUpdate(skin.getRequestedOn());

if (session.getUpstream().isInitialized()) {
PlayerListPacket.Entry updatedEntry = buildEntryManually(
entity.getUuid(),
entity.getUsername(),
entity.getGeyserId(),
skin.getTextureUrl(),
skin.getSkinData(),
cape.getCapeId(),
cape.getCapeData(),
geometry.getGeometryName(),
geometry.getGeometryData()
);

// If it is our skin we replace the UUID with the authdata UUID
if (session.getPlayerEntity() == entity) {
// Copy the entry with our identity instead.
PlayerListPacket.Entry copy = new PlayerListPacket.Entry(session.getAuthData().getUUID());
copy.setName(updatedEntry.getName());
copy.setEntityId(updatedEntry.getEntityId());
copy.setSkin(updatedEntry.getSkin());
copy.setXuid(updatedEntry.getXuid());
copy.setPlatformChatId(updatedEntry.getPlatformChatId());
copy.setTeacher(updatedEntry.isTeacher());
updatedEntry = copy;
}

PlayerListPacket playerRemovePacket = new PlayerListPacket();
playerRemovePacket.setAction(PlayerListPacket.Action.REMOVE);
playerRemovePacket.getEntries().add(updatedEntry);
session.sendUpstreamPacket(playerRemovePacket);

PlayerListPacket playerAddPacket = new PlayerListPacket();
playerAddPacket.setAction(PlayerListPacket.Action.ADD);
playerAddPacket.getEntries().add(updatedEntry);
session.sendUpstreamPacket(playerAddPacket);

if(entity.getUuid().equals(session.getPlayerEntity().getUuid())) {
session.fetchOurSkin(updatedEntry);
} else {
PlayerSkinPacket playerSkinPacket = new PlayerSkinPacket();
playerSkinPacket.setUuid(entity.getUuid());
playerSkinPacket.setSkin(updatedEntry.getSkin());
playerSkinPacket.setOldSkinName("OldName");
playerSkinPacket.setNewSkinName(String.valueOf(updatedEntry.getSkin().hashCode()));
playerSkinPacket.setTrustedSkin(true);
session.getUpstream().sendPacket(playerSkinPacket);
}
}
} catch (Exception e) {
Expand Down

0 comments on commit 956664d

Please sign in to comment.