Fix plugin-sourced movement walking the player to the same tile as the NPC/Player/etc

.gitlab-ci.yml

@@ -25,11 +25,6 @@ cache:
 verify:jdk11:
   stage: build
   script:
-    - 'apt-get install -q -y --no-install-recommends mariadb-server'
-    - 'mysqld_safe &'
-    - 'sleep 3'
-    - 'mysql -u root < Server/db_exports/global.sql'
-    - 'mysql -u root < Server/db_exports/testuser.sql'
     - 'cd Server'
     - 'mvn $MAVEN_CLI_OPTS verify'
   except:

Server/src/main/core/game/interaction/MovementPulse.java

@@ -182,6 +182,9 @@ public abstract class MovementPulse extends Pulse {
             this.pathfinder = pathfinder;
         }
         this.forceRun = forceRun;
+
+        if (destination instanceof NPC || destination instanceof Player)
+            destinationFlag = DestinationFlag.ENTITY;
     }
 
     @Override

Server/src/test/kotlin/TestUtils.kt

@@ -11,26 +11,35 @@ import core.net.packet.IoBuffer
 import org.rs09.consts.Items
 import core.ServerConstants
 import core.api.log
+import core.game.node.Node
+import core.game.node.entity.npc.NPC
+import core.game.node.item.GroundItem
 import core.game.shops.Shop
 import core.game.shops.ShopItem
 import core.tools.SystemLogger
 import core.game.system.config.ConfigParser
 import core.game.system.config.ServerConfigParser
 import core.game.world.GameWorld
+import core.game.world.map.Location
+import core.game.world.map.RegionManager
 import core.game.world.repository.Repository
 import core.game.world.update.UpdateSequence
+import core.net.packet.PacketProcessor
 import core.tools.Log
+import org.rs09.consts.Scenery
+import java.io.Closeable
 import java.net.URI
 import java.nio.ByteBuffer
 
 object TestUtils {
     var uidCounter = 0
 
-    fun getMockPlayer(name: String, ironman: IronmanMode = IronmanMode.NONE, rights: Rights = Rights.ADMINISTRATOR): Player {
+    fun getMockPlayer(name: String, ironman: IronmanMode = IronmanMode.NONE, rights: Rights = Rights.ADMINISTRATOR): MockPlayer {
         val p = MockPlayer(name)
         p.ironmanManager.mode = ironman
         p.details.accountInfo.uid = uidCounter++
         p.setPlaying(true);
+        p.playerFlags.lastSceneGraph = p.location ?: ServerConstants.HOME_LOCATION
         Repository.addPlayer(p)
         //Update sequence has a separate list of players for some reason...
         UpdateSequence.renderablePlayers.add(p)
@@ -74,16 +83,46 @@ object TestUtils {
             GameWorld.majorUpdateWorker.handleTickActions(skipPulseUpdates)
         }
     }
+
+    fun simulateInteraction (player: Player, target: Node, optionIndex: Int, iface: Int = -1, child: Int = -1) {
+        when (target) {
+            is GroundItem -> PacketProcessor.enqueue(core.net.packet.`in`.Packet.GroundItemAction(player, optionIndex, target.id, target.location.x, target.location.y))
+            is Item -> PacketProcessor.enqueue(core.net.packet.`in`.Packet.ItemAction(player, optionIndex, target.id, target.slot, iface, child))
+            is NPC -> PacketProcessor.enqueue(core.net.packet.`in`.Packet.NpcAction(player, optionIndex, target.clientIndex))
+            is core.game.node.scenery.Scenery -> PacketProcessor.enqueue(core.net.packet.`in`.Packet.SceneryAction(player, optionIndex, target.id, target.location.x, target.location.y))
+        }
+        advanceTicks(1, true)
+    }
 }
 
-class MockPlayer(name: String) : Player(PlayerDetails(name)) {
+class MockPlayer(name: String) : Player(PlayerDetails(name)), AutoCloseable {
+    var hasInit = false
     init {
         this.details.session = MockSession()
+        init()
     }
 
     override fun update() {
         //do nothing. This is for rendering stuff. We don't render a mock player. Not until the spaghetti is less spaghetti.
     }
+
+    override fun init() {
+        if (hasInit) return
+        hasInit = true
+        super.init()
+    }
+
+    override fun close() {
+        Repository.removePlayer(this)
+        UpdateSequence.renderablePlayers.remove(this)
+        finishClear()
+    }
+
+    override fun setLocation(location: Location?) {
+        super.setLocation(location)
+        RegionManager.move(this)
+        this.playerFlags.lastSceneGraph = location
+    }
 }
 
 class MockSession : IoSession(null, null) {

Server/src/test/kotlin/core/PathfinderTests.kt

@@ -3,16 +3,27 @@ package core
 import TestUtils
 import content.global.skill.gather.GatheringSkillOptionListeners
 import content.global.skill.gather.woodcutting.WoodcuttingListener
+import core.api.log
+import core.cache.def.impl.NPCDefinition
+import core.game.interaction.*
 import core.game.node.scenery.Scenery
 import core.game.world.map.Location
 import core.game.world.map.RegionManager
 import org.junit.jupiter.api.Assertions
 import org.junit.jupiter.api.Test
-import core.game.interaction.IntType
-import core.game.interaction.InteractionListeners
+import core.game.node.Node
+import core.game.node.entity.impl.PulseType
+import core.game.node.entity.npc.NPC
+import core.game.node.entity.player.Player
+import core.game.world.GameWorld
+import core.net.packet.PacketProcessor
+import core.plugin.ClassScanner
+import core.plugin.Plugin
+import core.tools.Log
+import org.rs09.consts.NPCs
 
 class PathfinderTests {
-    companion object {init {TestUtils.preTestSetup(); GatheringSkillOptionListeners().defineListeners(); WoodcuttingListener().defineListeners() }}
+    companion object {init {TestUtils.preTestSetup(); GatheringSkillOptionListeners().defineListeners(); WoodcuttingListener().defineListeners() }; val NPC_TEST_LOC = ServerConstants.HOME_LOCATION!!.transform(2, 10, 0)}
 
     @Test fun getOccupiedTilesShouldReturnCorrectSetOfTilesThatAnObjectOccupiesAtAllRotations() {
         //clay fireplace - 13609 - sizex: 1, sizey: 2
@@ -46,4 +57,168 @@ class PathfinderTests {
         TestUtils.advanceTicks(20, false)
         Assertions.assertEquals(Location.create(2722, 3475, 0), p.location)
     }
+
+    @Test fun movementInteractionShouldTrigger() {
+        val npc = NPC.create(0, NPC_TEST_LOC)
+        npc.init()
+
+        var intListenerRan = false
+        InteractionListeners.add(0, IntType.NPC.ordinal, arrayOf("testoptlistener"), method = {player: Player, node: Node ->
+            intListenerRan = true
+            return@add true
+        })
+
+        var pluginRan = false
+        val option = Option("testoption", 4)
+        val option2 = Option("testoptlistener", 0)
+        val testHandler = object : OptionHandler() {
+            override fun newInstance(arg: Any?): Plugin<Any> {
+                NPCDefinition.forId(0).handlers["option:testoption"] = this
+                return this
+            }
+            override fun handle(player: Player?, node: Node?, option: String?): Boolean {
+                pluginRan = true
+                return true
+            }
+        }
+        testHandler.newInstance(null)
+        npc.interaction.set(option)
+        npc.interaction.set(option2)
+        option.handler = testHandler
+
+        TestUtils.getMockPlayer("interactionTest").use {p ->
+            p.location = ServerConstants.HOME_LOCATION
+            TestUtils.simulateInteraction(p, npc, 0)
+            TestUtils.advanceTicks(10, false)
+            Assertions.assertEquals(true, intListenerRan)
+            p.location = ServerConstants.HOME_LOCATION
+            TestUtils.simulateInteraction(p, npc, 4)
+            TestUtils.advanceTicks(10, false)
+            Assertions.assertEquals(true, pluginRan)
+        }
+    }
+
+    @Test fun entityMovingToStationaryNPCShouldNotIdleIndefinitely() {
+        TestUtils.getMockPlayer("idlenpcdest").use {p ->
+            val startLoc = ServerConstants.HOME_LOCATION
+            p.location = startLoc
+            val npc = NPC.create(0, NPC_TEST_LOC)
+            npc.isNeverWalks = true
+            npc.init()
+            GameWorld.Pulser.submit(object : MovementPulse(p, npc) {
+                override fun pulse(): Boolean {
+                    return true;
+                }
+            })
+            TestUtils.advanceTicks(10, false)
+            Assertions.assertNotEquals(startLoc, p.location)
+        }
+    }
+
+    @Test fun entityTargetMovementPulseShouldNotStopOnSameTileAsEntity() {
+        TestUtils.getMockPlayer("entitystoptest").use {p ->
+            p.location = ServerConstants.HOME_LOCATION
+            val npc = NPC.create(0, NPC_TEST_LOC)
+            npc.isNeverWalks = true
+            npc.init()
+            GameWorld.Pulser.submit(object : MovementPulse(p, npc) {
+                override fun pulse(): Boolean {
+                    return true;
+                }
+            })
+            TestUtils.advanceTicks(10, false)
+            Assertions.assertNotEquals(p.location, npc.location)
+            Assertions.assertEquals(1.0, p.location.getDistance(npc.location))
+        }
+    }
+
+    @Test fun entityTargetMovementPulseWithExplicitParamsShouldNotStopOnSameTile() {
+        TestUtils.getMockPlayer("entitystoptest2").use { p ->
+            p.location = ServerConstants.HOME_LOCATION
+            val npc = NPC.create(0, NPC_TEST_LOC)
+            npc.isNeverWalks = true
+            npc.init()
+            GameWorld.Pulser.submit(object : MovementPulse(p, npc, DestinationFlag.ENTITY) {
+                override fun pulse(): Boolean {
+                    return true;
+                }
+            })
+            TestUtils.advanceTicks(10, false)
+            Assertions.assertNotEquals(p.location, npc.location)
+            Assertions.assertEquals(1.0, p.location.getDistance(npc.location))
+        }
+    }
+
+    @Test fun doubleMovementPulseToEntityShouldNotStopOnSameTile() {
+        TestUtils.getMockPlayer("entitystoptest3").use { p ->
+            p.location = ServerConstants.HOME_LOCATION
+            val npc = NPC.create(0, NPC_TEST_LOC)
+            npc.isNeverWalks = true
+            npc.init()
+            p.pulseManager.run(object : MovementPulse(p, npc) {
+                override fun pulse(): Boolean {
+                    return true
+                }
+            })
+            p.pulseManager.run(object : MovementPulse(p, npc) {
+                override fun pulse(): Boolean {
+                    return true
+                }
+            }, PulseType.STANDARD)
+            TestUtils.advanceTicks(10, false)
+            Assertions.assertNotEquals(ServerConstants.HOME_LOCATION, p.location)
+            Assertions.assertNotEquals(p.location, npc.location)
+            Assertions.assertEquals(1.0, p.location.getDistance(npc.location))
+        }
+    }
+
+    @Test fun simulatedInteractionPacketWithMovementFromPluginShouldNotEndOnSameTile() {
+        val testHandler = object : OptionHandler() {
+            override fun newInstance(arg: Any?): Plugin<Any> {
+                NPCDefinition.forId(0).handlers["option:testoption"] = this
+                return this
+            }
+            override fun handle(player: Player?, node: Node?, option: String?): Boolean {
+                log(this::class.java, Log.ERR, "Interaction triggered")
+                return true
+            }
+        }
+        testHandler.newInstance(null)
+        val npc = NPC.create(0, NPC_TEST_LOC)
+        val option = Option("testoption", 4)
+        npc.interaction.set(option)
+        option.handler = testHandler
+
+        TestUtils.getMockPlayer("entitystoptest4").use { p ->
+            p.location = ServerConstants.HOME_LOCATION
+            npc.isNeverWalks = true
+            npc.init()
+            TestUtils.simulateInteraction(p, npc, 4)
+            TestUtils.advanceTicks(20, false)
+            Assertions.assertNotEquals(ServerConstants.HOME_LOCATION, p.location)
+            Assertions.assertNotEquals(p.location, npc.location)
+            Assertions.assertEquals(1.0, p.location.getDistance(npc.location))
+        }
+    }
+
+    @Test fun simulatedInteractionPacketWithMovementFromListenerShouldNotEndOnSameTile() {
+        val npc = NPC.create(0, NPC_TEST_LOC)
+        npc.isNeverWalks = true
+        npc.init()
+
+        InteractionListeners.add(0, IntType.NPC.ordinal, arrayOf("testoptlistener2"), method = {player: Player, node: Node ->
+            return@add true
+        })
+        val opt = Option("testoptlistener2", 1)
+        npc.interaction.set(opt)
+
+        TestUtils.getMockPlayer("entitystoptest5").use { p ->
+            p.location = ServerConstants.HOME_LOCATION
+            TestUtils.simulateInteraction(p, npc, 1)
+            TestUtils.advanceTicks(20, false)
+            Assertions.assertNotEquals(ServerConstants.HOME_LOCATION, p.location)
+            Assertions.assertNotEquals(p.location, npc.location)
+            Assertions.assertEquals(1.0, p.location.getDistance(npc.location))
+        }
+    }
 }

Server/src/test/kotlin/core/storage/SQLStorageProviderTests.kt

@@ -1,190 +0,0 @@
-package core.storage
-
-import org.junit.jupiter.api.AfterAll
-import org.junit.jupiter.api.Assertions
-import org.junit.jupiter.api.Test
-import core.ServerConstants
-import core.api.log
-import core.auth.UserAccountInfo
-import core.tools.Log
-import core.tools.SystemLogger
-import java.sql.SQLDataException
-
-class SQLStorageProviderTests {
-    companion object {
-        val storage = SQLStorageProvider()
-        val testAccountNames = ArrayList<String>()
-        init {
-            storage.configure("localhost", ServerConstants.DATABASE_NAME!!, ServerConstants.DATABASE_USER!!, ServerConstants.DATABASE_PASS!!)
-            val details = UserAccountInfo.createDefault()
-            details.rights = 2
-            details.username = "test"
-        }
-
-        @AfterAll @JvmStatic fun cleanup() {
-            log(this::class.java, Log.FINE,  "Cleaning up unit test accounts")
-            testAccountNames.forEach {name ->
-                log(this::class.java, Log.FINE,  "Removing test account $name")
-                val info = UserAccountInfo.createDefault()
-                info.username = name
-                storage.remove(info)
-            }
-        }
-    }
-
-    @Test fun shouldReturnCorrectUserData() {
-        val data = UserAccountInfo.createDefault()
-        data.username = "test111"
-        data.password = "test"
-        data.rights = 2
-        testAccountNames.add("test111")
-        storage.store(data)
-        val accountInfo = storage.getAccountInfo("test111")
-        Assertions.assertEquals(2, accountInfo.rights)
-    }
-
-    @Test fun shouldAllowStoreUserData() {
-        val userData = UserAccountInfo.createDefault()
-        userData.username = "storageTest"
-        userData.password = "test"
-        testAccountNames.add("storageTest")
-        storage.store(userData)
-        val exists = storage.checkUsernameTaken("storageTest")
-        Assertions.assertEquals(true, exists)
-    }
-
-    @Test fun shouldNotAllowDuplicateAccountStorage() {
-        val userData = UserAccountInfo.createDefault()
-        userData.username = "bilbo111"
-        userData.password = "test"
-        testAccountNames.add("bilbo111")
-        storage.store(userData)
-        Assertions.assertThrows(SQLDataException::class.java) {
-            storage.store(userData)
-        }
-    }
-
-    @Test fun shouldAllowRemoveUserInfo() {
-        val userData = UserAccountInfo.createDefault()
-        userData.username = "bepis"
-        userData.password = "test"
-        testAccountNames.add("bepis")
-        storage.store(userData)
-        storage.remove(userData)
-        Assertions.assertEquals(false, storage.checkUsernameTaken("bepis"))
-    }
-
-    @Test fun shouldUpdateUserData() {
-        val userData = UserAccountInfo.createDefault()
-        userData.username = "borpis"
-        userData.password = "test"
-        testAccountNames.add("borpis")
-        storage.store(userData)
-        userData.credits = 2
-        storage.update(userData)
-        val data = storage.getAccountInfo(userData.username)
-        Assertions.assertEquals(2, data.credits, "Wrong data: $data")
-    }
-
-    @Test fun shouldNotAllowStoreOrUpdateEmptyData() {
-        val info = UserAccountInfo.createDefault()
-        Assertions.assertThrows(IllegalStateException::class.java) {
-            storage.store(info)
-        }
-        Assertions.assertThrows(IllegalStateException::class.java) {
-            storage.update(info)
-        }
-        info.username = "test"
-        Assertions.assertThrows(IllegalStateException::class.java) {
-            storage.store(info)
-        }
-        Assertions.assertThrows(IllegalStateException::class.java) {
-            storage.update(info)
-        }
-    }
-
-    @Test fun shouldSetDefaultValuesWhenDBFieldIsNull() {
-        val defaultData = UserAccountInfo.createDefault()
-
-        //manually insert a definitely-mostly-null entry into the DB
-        val conn = storage.getConnection()
-        conn.use {
-            val stmt = conn.prepareStatement("INSERT INTO members (username) VALUES (?);")
-            stmt.setString(1, "nulltestacc")
-            testAccountNames.add("nulltestacc")
-            stmt.execute()
-        }
-
-        var data: UserAccountInfo = UserAccountInfo.createDefault()
-        Assertions.assertDoesNotThrow {
-            data = storage.getAccountInfo("nulltestacc")
-        }
-
-        Assertions.assertEquals(defaultData.password, data.password)
-        Assertions.assertEquals(defaultData.rights, data.rights)
-        Assertions.assertEquals(defaultData.credits, data.credits)
-        Assertions.assertEquals(defaultData.ip, data.ip)
-        Assertions.assertEquals(defaultData.lastUsedIp, data.lastUsedIp)
-        Assertions.assertEquals(defaultData.muteEndTime, data.muteEndTime)
-        Assertions.assertEquals(defaultData.banEndTime, data.banEndTime)
-        Assertions.assertEquals(defaultData.contacts, data.contacts)
-        Assertions.assertEquals(defaultData.blocked, data.blocked)
-        Assertions.assertEquals(defaultData.clanName, data.clanName)
-        Assertions.assertEquals(defaultData.currentClan, data.currentClan)
-        Assertions.assertEquals(defaultData.clanReqs, data.clanReqs)
-        Assertions.assertEquals(defaultData.timePlayed, data.timePlayed)
-        Assertions.assertEquals(defaultData.lastLogin, data.lastLogin)
-        Assertions.assertEquals(defaultData.online, data.online)
-    }
-
-    @Test fun updatingPropertiesOnTheDatabaseEndShouldBePreservedWhenFetchingAccountInfo() {
-        val conn = storage.getConnection()
-        conn.use {
-            val stmt = conn.prepareStatement("INSERT INTO members (username) VALUES (?);")
-            stmt.setString(1, "dbupdateacc")
-            testAccountNames.add("dbupdateacc")
-            stmt.execute()
-            stmt.close()
-
-            val stmt2 = conn.prepareStatement("UPDATE members SET rights = 2 WHERE username = \"dbupdateacc\";")
-            stmt2.execute()
-        }
-
-        val info = storage.getAccountInfo("dbupdateacc")
-        Assertions.assertEquals(2, info.rights)
-        info.rights = 1
-        storage.update(info)
-
-        val info2 = storage.getAccountInfo("dbupdateacc")
-        Assertions.assertEquals(1, info2.rights)
-
-        val conn2 = storage.getConnection()
-        conn2.use {
-            val stmt = conn2.prepareStatement("UPDATE members SET rights = 2 WHERE username = \"dbupdateacc\";")
-            stmt.execute()
-        }
-
-        val info3 = storage.getAccountInfo("dbupdateacc")
-        Assertions.assertEquals(2, info3.rights)
-    }
-
-    @Test fun shouldCorrectlyUpdateMultipleChangedValues() {
-        val userData = UserAccountInfo.createDefault()
-        userData.username = "borpis2"
-        userData.password = "test"
-        testAccountNames.add("borpis2")
-        storage.store(userData)
-
-        val lastLogin = System.currentTimeMillis()
-
-        userData.credits = 2
-        userData.lastLogin = lastLogin
-        userData.currentClan = "3009scape"
-        storage.update(userData)
-
-        val data = storage.getAccountInfo(userData.username)
-        Assertions.assertEquals(2, data.credits, "Wrong data: $data")
-        Assertions.assertEquals(lastLogin, data.lastLogin, "Wrong data: $data")
-        Assertions.assertEquals("3009scape", data.currentClan, "Wrong data: $data")
-    }
-}