Fix bug where rejoining players could crash the game

build.gradle.kts

@@ -45,7 +45,7 @@ dependencies {
         exclude(group = "com.google.code.gson", module = "gson")
     }
     include(modImplementation("net.fabricmc:fabric-language-kotlin:${property("fabric_kotlin_version")}")!!)
-    include(modImplementation("me.lucko:fabric-permissions-api:0.3-SNAPSHOT")!!)
+    include(modImplementation("me.lucko:fabric-permissions-api:${property("permissions_version")}")!!)
 
     modImplementation("com.github.gnembon:fabric-carpet:${property("carpet_version")}")
 

gradle.properties

@@ -11,6 +11,7 @@ minecraft_version=1.20.4
 parchment_version=1.20.1:2023.08.06
 loader_version=0.15.2
 fabric_kotlin_version=1.9.2+kotlin.1.8.10
+permissions_version=0.3-SNAPSHOT
 carpet_version=1.4.128
 mixin_extras_version=0.2.1-beta.2
 

src/main/kotlin/me/senseiwells/replay/player/PlayerRecorder.kt

@@ -155,10 +155,11 @@ class PlayerRecorder internal constructor(
         if (this.stopped) {
             return CompletableFuture.failedFuture(IllegalStateException("Cannot stop replay after already stopped"))
         }
-        PlayerRecorders.removeByUUID(this.profile.id)
 
         // We only save if the player has actually logged in...
-        return this.close(save && this.protocol == ConnectionProtocol.PLAY)
+        val future = this.close(save && this.protocol == ConnectionProtocol.PLAY)
+        PlayerRecorders.close(this.server, this.playerUUID, future)
+        return future
     }
 
     fun getRecordingTimeMS(): Long {

src/main/kotlin/me/senseiwells/replay/player/PlayerRecorders.kt

@@ -6,9 +6,12 @@ import me.senseiwells.replay.rejoin.RejoinedReplayPlayer
 import net.minecraft.server.MinecraftServer
 import net.minecraft.server.level.ServerPlayer
 import java.util.*
+import java.util.concurrent.CompletableFuture
+import kotlin.collections.HashMap
 
 object PlayerRecorders {
     private val players = LinkedHashMap<UUID, PlayerRecorder>()
+    private val closing = HashMap<UUID, CompletableFuture<Long>>()
 
     @JvmField
     var predicate = ReplayConfig.predicate
@@ -26,6 +29,12 @@ object PlayerRecorders {
         if (this.players.containsKey(profile.id)) {
             throw IllegalArgumentException("Player already has a recorder")
         }
+
+        // If a player rejoins before their previous one has fully closed,
+        // we wait for it to fully close; this blocks the main thread;
+        // however, it's unlikely that this will be significant.
+        this.closing[profile.id]?.join()
+
         val recorder = PlayerRecorder(
             server,
             profile,
@@ -50,11 +59,6 @@ object PlayerRecorders {
         return this.players[uuid];
     }
 
-    @JvmStatic
-    fun remove(player: ServerPlayer): PlayerRecorder? {
-        return this.removeByUUID(player.uuid)
-    }
-
     @JvmStatic
     fun removeByUUID(uuid: UUID): PlayerRecorder? {
         return this.players.remove(uuid)
@@ -64,4 +68,10 @@ object PlayerRecorders {
     fun all(): Collection<PlayerRecorder> {
         return this.players.values
     }
+
+    internal fun close(server: MinecraftServer, uuid: UUID, future: CompletableFuture<Long>) {
+        this.players.remove(uuid)
+        this.closing[uuid] = future
+        future.thenRunAsync({ this.closing.remove(uuid) }, server)
+    }
 }