Player update and hunting performance (#822)

* Fix invalid area coordinates

* Tweaks to improve PlayerUpdateTask performance

* Replace CharacterMap with linked list for faster hunting lookup performance (and track player zones)

* Fix saving bots on shutdown

* Fix duplicate mage of zamorak spawn

Author: GregHib

data/area/kandarin/seers_village/seers_village.areas.toml

@@ -5,7 +5,7 @@ tags = ["penguin_area"]
 hint = "near where they see."
 
 [flax_field]
-x = [2772, 2728]
+x = [2728, 2772]
 y = [3424, 3455]
 tags = ["penguin_area"]
 hint = "near the bees and flax."

data/area/misthalin/varrock/varrock.npc-spawns.toml

@@ -175,7 +175,6 @@ spawns = [
   { id = "giant_rat_varrock", x = 3264, y = 3383, members = true },
   { id = "giant_rat_varrock", x = 3297, y = 3379, members = true },
   { id = "master_farmer_varrock", x = 3231, y = 3347, members = true },
-  { id = "mage_of_zamorak_varrock", x = 3260, y = 3383, members = true },
   { id = "tanner_varrock", x = 3188, y = 3407 },
   { id = "dreven", x = 3181, y = 3359 },
   { id = "treznor", x = 3226, y = 3458 },

engine/src/main/kotlin/world/gregs/voidps/engine/client/update/npc/NPCUpdateTask.kt

@@ -16,7 +16,7 @@ import world.gregs.voidps.type.RegionLevel
 
 class NPCUpdateTask(
     private val npcs: NPCs,
-    private val encoders: List<VisualEncoder<NPCVisuals>>,
+    private val encoders: Array<VisualEncoder<NPCVisuals>>,
 ) {
 
     fun run(player: Player) {
@@ -112,10 +112,10 @@ class NPCUpdateTask(
         var npc: NPC
         for (direction in Direction.reversed) {
             region = client.tile.regionLevel.add(direction)
-            for (index in npcs.getDirect(region) ?: continue) {
-                npc = npcs.indexed(index) ?: continue
+            npcs.regionMap.onEach(region.id) { index ->
+                npc = npcs.indexed(index) ?: return@onEach
                 if (!add(updates, sync, npc, client, viewport, set, index)) {
-                    continue
+                    return@onEach
                 }
                 val visuals = npc.visuals
                 var flag = visuals.flag

engine/src/main/kotlin/world/gregs/voidps/engine/client/update/player/PlayerUpdateTask.kt

@@ -6,19 +6,43 @@ import world.gregs.voidps.engine.client.update.view.Viewport
 import world.gregs.voidps.engine.entity.character.player.Player
 import world.gregs.voidps.engine.entity.character.player.Players
 import world.gregs.voidps.network.login.protocol.encode.updatePlayers
-import world.gregs.voidps.network.login.protocol.visual.PlayerVisuals
-import world.gregs.voidps.network.login.protocol.visual.VisualEncoder
+import world.gregs.voidps.network.login.protocol.visual.VisualMask
 import world.gregs.voidps.network.login.protocol.visual.VisualMask.APPEARANCE_MASK
+import world.gregs.voidps.network.login.protocol.visual.VisualMask.APPEARANCE_MASK_INV
+import world.gregs.voidps.network.login.protocol.visual.encode.SayEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.WatchEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.AppearanceEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.MovementTypeEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.PlayerAnimationEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.PlayerColourOverlayEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.PlayerExactMovementEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.PlayerFaceEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.PlayerHitsEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.PlayerPrimaryGraphicEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.PlayerSecondaryGraphicEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.PlayerTimeBarEncoder
+import world.gregs.voidps.network.login.protocol.visual.encode.player.TemporaryMoveTypeEncoder
 import world.gregs.voidps.type.Delta
 import kotlin.math.abs
 
 class PlayerUpdateTask(
     private val players: Players,
-    private val encoders: List<VisualEncoder<PlayerVisuals>>,
 ) {
-
-    private val initialEncoders = encoders.filter { it.initial }
-    private val initialFlag = initialEncoders.sumOf { it.mask }
+    private val watchEncoder = WatchEncoder(VisualMask.PLAYER_WATCH_MASK)
+    private val playerTimeBarEncoder = PlayerTimeBarEncoder()
+    private val sayEncoder = SayEncoder(VisualMask.PLAYER_SAY_MASK)
+    private val playerHitsEncoder = PlayerHitsEncoder()
+    private val playerFaceEncoder = PlayerFaceEncoder()
+    private val playerExactMovementEncoder = PlayerExactMovementEncoder()
+    private val playerSecondaryGraphicEncoder = PlayerSecondaryGraphicEncoder()
+    private val playerColourOverlayEncoder = PlayerColourOverlayEncoder()
+    private val movementTypeEncoder = MovementTypeEncoder()
+    private val playerPrimaryGraphicEncoder = PlayerPrimaryGraphicEncoder()
+    private val playerAnimationEncoder = PlayerAnimationEncoder()
+    private val appearanceEncoder = AppearanceEncoder()
+    private val temporaryMoveTypeEncoder = TemporaryMoveTypeEncoder()
+
+    private val initialFlag = VisualMask.PLAYER_FACE_MASK + VisualMask.MOVEMENT_TYPE_MASK + VisualMask.PLAYER_ANIMATION_MASK + VisualMask.APPEARANCE_MASK + VisualMask.TEMPORARY_MOVEMENT_TYPE_MASK
 
     fun run(player: Player) {
         val viewport = player.viewport ?: return
@@ -85,7 +109,7 @@ class PlayerUpdateTask(
             }
 
             encodeMovement(updateType, sync, viewport, player)
-            encodeVisuals(updates, flag, player, client, set, encoders)
+            encodeVisuals(updates, flag, player, client, set)
         }
 
         if (skip > -1) {
@@ -114,16 +138,49 @@ class PlayerUpdateTask(
         viewport.seen(player)
     }
 
-    private fun encodeVisuals(updates: Writer, flag: Int, player: Player, client: Player, set: PlayerTrackingSet, encoders: List<VisualEncoder<PlayerVisuals>>) {
+    private fun encodeVisuals(updates: Writer, flag: Int, player: Player, client: Player, set: PlayerTrackingSet) {
         if (flag == 0) {
             return
         }
         writeFlag(updates, flag)
-        for (encoder in encoders) {
-            if (flag and encoder.mask == 0) {
-                continue
-            }
-            encoder.encode(updates, player.visuals, client.index)
+        if (flag and VisualMask.PLAYER_WATCH_MASK != 0) {
+            watchEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.PLAYER_TIME_BAR_MASK != 0) {
+            playerTimeBarEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.PLAYER_SAY_MASK != 0) {
+            sayEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.PLAYER_HITS_MASK != 0) {
+            playerHitsEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.PLAYER_FACE_MASK != 0) {
+            playerFaceEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.PLAYER_EXACT_MOVEMENT_MASK != 0) {
+            playerExactMovementEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.PLAYER_GRAPHIC_2_MASK != 0) {
+            playerSecondaryGraphicEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.PLAYER_COLOUR_OVERLAY_MASK != 0) {
+            playerColourOverlayEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.MOVEMENT_TYPE_MASK != 0) {
+            movementTypeEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.PLAYER_GRAPHIC_1_MASK != 0) {
+            playerPrimaryGraphicEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.PLAYER_ANIMATION_MASK != 0) {
+            playerAnimationEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and APPEARANCE_MASK != 0) {
+            appearanceEncoder.encode(updates, player.visuals, client.index)
+        }
+        if (flag and VisualMask.TEMPORARY_MOVEMENT_TYPE_MASK != 0) {
+            temporaryMoveTypeEncoder.encode(updates, player.visuals, client.index)
         }
         if (flag and APPEARANCE_MASK != 0) {
             set.updateAppearance(player)
@@ -137,7 +194,7 @@ class PlayerUpdateTask(
     private fun updateFlag(updates: Writer, player: Player, set: PlayerTrackingSet): Int {
         val visuals = player.visuals
         if (updates.position() + visuals.appearance.length >= MAX_UPDATE_SIZE) {
-            return visuals.flag and APPEARANCE_MASK.inv()
+            return visuals.flag and APPEARANCE_MASK_INV
         }
         if (set.needsAppearanceUpdate(player)) {
             return visuals.flag or APPEARANCE_MASK
@@ -220,7 +277,7 @@ class PlayerUpdateTask(
             sync.writeBits(6, player.tile.y and 0x3f)
             sync.writeBits(1, appearance)
             if (appearance) {
-                encodeVisuals(updates, initialFlag, player, client, set, initialEncoders)
+                encodeVisuals(updates, initialFlag, player, client, set)
             }
         }
         if (skip > -1) {
@@ -234,9 +291,9 @@ class PlayerUpdateTask(
      * @return true when within [Viewport.radius] and packet has enough room
      */
     private fun add(player: Player, client: Player, viewport: Viewport, updates: Writer, sync: Writer): Boolean = player.client?.disconnected != true &&
-        player.tile.within(client.tile, viewport.radius) &&
-        updates.position() < MAX_UPDATE_SIZE &&
-        sync.position() < MAX_SYNC_SIZE
+            player.tile.within(client.tile, viewport.radius) &&
+            updates.position() < MAX_UPDATE_SIZE &&
+            sync.position() < MAX_SYNC_SIZE
 
     fun writeSkip(sync: Writer, skip: Int) {
         sync.writeBits(1, 0)

engine/src/main/kotlin/world/gregs/voidps/engine/data/SaveQueue.kt

@@ -5,6 +5,7 @@ import kotlinx.coroutines.*
 import world.gregs.voidps.engine.client.ui.chat.plural
 import world.gregs.voidps.engine.entity.character.player.Player
 import world.gregs.voidps.engine.entity.character.player.Players
+import world.gregs.voidps.engine.entity.character.player.name
 import java.lang.Runnable
 import java.util.concurrent.ConcurrentHashMap
 import kotlin.system.measureTimeMillis
@@ -32,7 +33,7 @@ class SaveQueue(
         scope.save(pending.values.toList())
     }
 
-    fun direct(players: Players): Job = scope.save(players.map { it.copy() })
+    fun direct(players: Players): Job = scope.save(players.filter { !it.contains("bot") }.map { it.copy() })
 
     private fun CoroutineScope.save(accounts: List<PlayerSave>) = launch(handler) {
         val took = measureTimeMillis {

engine/src/main/kotlin/world/gregs/voidps/engine/entity/character/CharacterIndexMap.kt

@@ -0,0 +1,60 @@
+package world.gregs.voidps.engine.entity.character
+
+import it.unimi.dsi.fastutil.ints.Int2IntOpenHashMap
+
+/**
+ * High-performance spatial index for grouping character indices by [world.gregs.voidps.type.Zone] or [world.gregs.voidps.type.Region]
+ * i.e. Map<Zone, List<Index>>
+ * It's a doubly linked list with head [table] for fast iteration.
+ */
+class CharacterIndexMap(size: Int) {
+    private val table = Int2IntOpenHashMap(size)
+    init {
+        table.defaultReturnValue(INVALID)
+    }
+    val next = IntArray(size) { INVALID }
+    val previous = IntArray(size) { INVALID }
+
+    fun add(id: Int, index: Int) {
+        previous[index] = INVALID
+        val head = table.get(id)
+        next[index] = head
+        if (head != INVALID) {
+            previous[head] = index
+        }
+        table.put(id, index)
+    }
+
+    fun remove(id: Int, index: Int) {
+        val p = previous[index]
+        val n = next[index]
+        if (p != INVALID) {
+            next[p] = n
+        } else {
+            table.put(id, n)
+        }
+        if (n != INVALID) {
+            previous[n] = p
+        }
+        previous[index] = INVALID
+        next[index] = INVALID
+    }
+
+    fun clear() {
+        table.clear()
+        next.fill(INVALID)
+        previous.fill(INVALID)
+    }
+
+    fun onEach(id: Int, action: (Int) -> Unit) {
+        var index = table.get(id)
+        while (index != INVALID) {
+            action(index)
+            index = next[index]
+        }
+    }
+
+    companion object {
+        private const val INVALID = -1
+    }
+}

engine/src/main/kotlin/world/gregs/voidps/engine/entity/character/CharacterMap.kt

@@ -7,8 +7,8 @@ import world.gregs.voidps.engine.data.definition.NPCDefinitions
 import world.gregs.voidps.engine.entity.Despawn
 import world.gregs.voidps.engine.entity.MAX_NPCS
 import world.gregs.voidps.engine.entity.Spawn
-import world.gregs.voidps.engine.entity.character.CharacterMap
 import world.gregs.voidps.engine.entity.character.CharacterSearch
+import world.gregs.voidps.engine.entity.character.CharacterIndexMap
 import world.gregs.voidps.engine.entity.character.mode.EmptyMode
 import world.gregs.voidps.engine.entity.character.mode.Wander
 import world.gregs.voidps.engine.entity.character.player.skill.Skill
@@ -35,7 +35,8 @@ data class NPCs(
     private var removeIndex = 0
     var size = 0
         private set
-    private val map: CharacterMap = CharacterMap()
+    private val zoneMap = CharacterIndexMap(MAX_NPCS)
+    internal val regionMap = CharacterIndexMap(MAX_NPCS)
     private val logger = InlineLogger()
 
     override fun run() {
@@ -46,7 +47,8 @@ data class NPCs(
             size--
             val npc = indexArray[index] ?: continue
             indexArray[index] = null
-            map.remove(npc.tile.regionLevel, npc)
+            regionMap.remove(npc.tile.regionLevel.id, npc.index)
+            zoneMap.remove(npc.tile.zone.id, npc.index)
             npc.index = -1
         }
         removeIndex = 0
@@ -90,17 +92,19 @@ data class NPCs(
 
     fun update(npc: NPC, from: Tile) {
         if (from.regionLevel != npc.tile.regionLevel) {
-            map.remove(from.regionLevel, npc)
-            map.add(npc.tile.regionLevel, npc)
+            regionMap.remove(from.regionLevel.id, npc.index)
+            regionMap.add(npc.tile.regionLevel.id, npc.index)
+        }
+        if (from.zone != npc.tile.zone) {
+            zoneMap.remove(from.zone.id, npc.index)
+            zoneMap.add(npc.tile.zone.id, npc.index)
         }
     }
 
-    fun getDirect(region: RegionLevel): List<Int>? = this.map[region]
-
     override operator fun get(tile: Tile): List<NPC> {
         val list = mutableListOf<NPC>()
-        for (index in map[tile.regionLevel] ?: return list) {
-            val npc = indexed(index) ?: continue
+        zoneMap.onEach(tile.zone.id) { index ->
+            val npc = indexed(index) ?: return@onEach
             if (npc.tile == tile) {
                 list.add(npc)
             }
@@ -110,19 +114,16 @@ data class NPCs(
 
     override operator fun get(zone: Zone): List<NPC> {
         val list = mutableListOf<NPC>()
-        for (index in map[zone.regionLevel] ?: return list) {
-            val npc = indexed(index) ?: continue
-            if (npc.tile.zone == zone) {
-                list.add(npc)
-            }
+        zoneMap.onEach(zone.id) { index ->
+            list.add(indexed(index) ?: return@onEach)
         }
         return list
     }
 
     operator fun get(region: RegionLevel): List<NPC> {
         val list = mutableListOf<NPC>()
-        for (index in map[region] ?: return list) {
-            list.add(indexed(index) ?: continue)
+        regionMap.onEach(region.id) { index ->
+            list.add(indexed(index) ?: return@onEach)
         }
         return list
     }
@@ -155,7 +156,8 @@ data class NPCs(
             npc.mode = Wander(npc, npc.tile)
         }
         npc.collision = collision.get(npc)
-        map.add(npc.tile.regionLevel, npc)
+        regionMap.add(npc.tile.regionLevel.id, npc.index)
+        zoneMap.add(npc.tile.zone.id, npc.index)
         val respawnDelay = npc.def.getOrNull<Int>("respawn_delay")
         if (respawnDelay != null && respawnDelay >= 0) {
             npc["respawn_tile"] = npc.tile
@@ -167,7 +169,7 @@ data class NPCs(
     }
 
     fun clear(region: RegionLevel) {
-        for (index in map[region] ?: return) {
+        regionMap.onEach(region.id) { index ->
             if (removeIndex < removeQueue.size) {
                 removeQueue[removeIndex++] = index
             }
@@ -182,6 +184,8 @@ data class NPCs(
         indexArray.fill(null)
         indexer = 1
         size = 0
+        regionMap.clear()
+        zoneMap.clear()
     }
 
     override fun iterator(): Iterator<NPC> = object : Iterator<NPC> {