Add more unit tests for shops and quest related architecture

Fixed bug where quests could be repeatedly finished
Fixed bug where ironman status wasn't checked when buying overstocked items
Fixed bug where selling multiple items at a time that weren't listed in a shop would not succeed
Fixed bug where shop restocking would sometimes interrupt if a shop stock item was in a null slot

Server/src/main/java/core/game/node/entity/player/link/quest/Quest.java

@@ -117,6 +117,9 @@ public abstract class Quest implements Plugin<Object> {
 	 * @param player The player.
 	 */
 	public void finish(Player player) {
+		if(player.getQuestRepository().isComplete(name)) {
+			throw new IllegalStateException("Tried to complete quest " + name + " twice, which is not allowed!");
+		}
 		for (int i = 0; i < 18; i++) {
 			if (i == 9 || i == 3 || i == 6) {
 				continue;

Server/src/main/kotlin/rs09/game/content/global/shops/Shop.kt

@@ -39,8 +39,8 @@ class Shop(val title: String, val stock: Array<ShopItem>, val general: Boolean =
 {
     val stockInstances = HashMap<Int, Container>()
     val playerStock = if (general) generalPlayerStock else Container(40, ContainerType.SHOP)
-    private val needsUpdate = HashMap<Int, Boolean>()
-    private val restockRates = HashMap<Int,Int>()
+    val needsUpdate = HashMap<Int, Boolean>()
+    val restockRates = HashMap<Int,Int>()
 
     init {
         if(!getServerConfig().getBoolean(Shops.personalizedShops, false))
@@ -93,7 +93,7 @@ class Shop(val title: String, val stock: Array<ShopItem>, val general: Boolean =
         setAttribute(player, "shop-main", main)
     }
 
-    private fun getContainer(player: Player) : Container
+    public fun getContainer(player: Player) : Container
     {
         val container = if(getServerConfig().getBoolean(Shops.personalizedShops, false))
             stockInstances[player.username.hashCode()] ?: generateStockContainer().also { stockInstances[player.username.hashCode()] = it }
@@ -131,6 +131,7 @@ class Shop(val title: String, val stock: Array<ShopItem>, val general: Boolean =
         stockInstances.filter { needsUpdate[it.key] == true }.forEach{ (player,cont) ->
             for(i in 0 until cont.capacity())
             {
+                if(cont[i] == null) continue
                 if(stock.size < i + 1) break
                 if(GameWorld.ticks % stock[i].restockRate != 0) continue
 
@@ -236,29 +237,29 @@ class Shop(val title: String, val stock: Array<ShopItem>, val general: Boolean =
         return max(price, 1)
     }
 
-    fun buy(player: Player, slot: Int, amount: Int)
+    fun buy(player: Player, slot: Int, amount: Int) : TransactionStatus
     {
-        if(amount !in 1..Integer.MAX_VALUE) return
+        if(amount !in 1..Integer.MAX_VALUE) return TransactionStatus.Failure("Invalid amount: $amount")
         val isMainStock = getAttribute(player, "shop-main", false)
         if(!isMainStock && player.ironmanManager.isIronman)
         {
             sendDialogue(player, "As an ironman, you cannot buy from player stock in shops.")
-            return
+            return TransactionStatus.Failure("Ironman buying from player stock")
         }
-        val cont = if (isMainStock) getAttribute<Container?>(player, "shop-cont", null) ?: return else playerStock
+        val cont = if (isMainStock) getAttribute<Container?>(player, "shop-cont", null) ?: return TransactionStatus.Failure("Invalid shop-cont attr") else playerStock
         val inStock = cont[slot]
         val item = Item(inStock.id, amount)
         if(inStock.amount < amount)
             item.amount = inStock.amount
 
-        if(inStock.amount > stock[slot].amount && !getServerConfig().getBoolean(Shops.personalizedShops, false))
+        if(inStock.amount > stock[slot].amount && !getServerConfig().getBoolean(Shops.personalizedShops, false) && player.ironmanManager.isIronman)
         {
             sendDialogue(player, "As an ironman, you cannot buy overstocked items from shops.")
-            return
+            return TransactionStatus.Failure("Ironman overstock purchase")
         }
 
         val cost = getBuyPrice(player, slot)
-        if(cost.id == -1) sendMessage(player, "This shop cannot sell that item.").also { return }
+        if(cost.id == -1) sendMessage(player, "This shop cannot sell that item.").also { return TransactionStatus.Failure("Shop cannot sell this item")}
 
         if(currency == Items.COINS_995){
             var amt = item.amount
@@ -276,7 +277,7 @@ class Shop(val title: String, val stock: Array<ShopItem>, val general: Boolean =
                 if(!hasSpaceFor(player, item)) {
                     addItem(player, cost.id, cost.amount)
                     sendMessage(player, "You don't have enough inventory space to buy that many.")
-                    return
+                    return TransactionStatus.Failure("Not enough inventory space")
                 }
 
                 if(!isMainStock && cont[slot].amount - item.amount == 0)
@@ -303,20 +304,22 @@ class Shop(val title: String, val stock: Array<ShopItem>, val general: Boolean =
         {
             sendMessage(player, "You don't have enough ${cost.name.toLowerCase()} to buy that many.")
         }
+
+        return TransactionStatus.Success()
     }
 
-    fun sell(player: Player, slot: Int, amount: Int)
+    fun sell(player: Player, slot: Int, amount: Int) : TransactionStatus
     {
-        if(amount !in 1..Integer.MAX_VALUE) return
+        if(amount !in 1..Integer.MAX_VALUE) return TransactionStatus.Failure("Invalid amount: $amount")
         val playerInventory = player.inventory[slot]
         if(playerInventory.id in intArrayOf(Items.COINS_995, Items.TOKKUL_6529, Items.ARCHERY_TICKET_1464))
         {
             sendMessage(player, "You can't sell currency to a shop.")
-            return
+            return TransactionStatus.Failure("Tried to sell currency - ${playerInventory.id}")
         }
         val item = Item(playerInventory.id, amount)
         val (container,profit) = getSellPrice(player, slot)
-        if(profit.amount == -1) sendMessage(player, "This item can't be sold to this shop.").also { return }
+        if(profit.amount == -1) sendMessage(player, "This item can't be sold to this shop.").also { return TransactionStatus.Failure("Can't sell this item to this shop - ${playerInventory.id}, general: $general, price: $profit") }
         if(amount > player.inventory.getAmount(item.id))
             item.amount = player.inventory.getAmount(item.id)
 
@@ -336,7 +339,7 @@ class Shop(val title: String, val stock: Array<ShopItem>, val general: Boolean =
             if(!hasSpaceFor(player, profit)){
                 sendMessage(player, "You don't have enough space to do that.")
                 addItem(player, item.id, item.amount)
-                return
+                return TransactionStatus.Failure("Did not have enough inventory space")
             }
             if(container == playerStock && getAttribute(player, "shop-main", false)){
                 showTab(player, false)
@@ -358,6 +361,7 @@ class Shop(val title: String, val stock: Array<ShopItem>, val general: Boolean =
                 needsUpdate[ServerConstants.SERVER_NAME.hashCode()] = true
             }
         }
+        return TransactionStatus.Success()
     }
 
     fun getStockSlot(itemId: Int): Pair<Boolean, Int>
@@ -382,6 +386,7 @@ class Shop(val title: String, val stock: Array<ShopItem>, val general: Boolean =
             }
         }
 
+        if(shopSlot == -1) isPlayerStock = true
         return Pair(isPlayerStock, shopSlot)
     }
 
@@ -390,4 +395,9 @@ class Shop(val title: String, val stock: Array<ShopItem>, val general: Boolean =
         val generalPlayerStock = Container(40, ContainerType.SHOP)
         val listenerInstances = HashMap<Int, ShopListener>()
     }
+
+    sealed class TransactionStatus {
+        class Success : TransactionStatus()
+        class Failure(val reason: String) : TransactionStatus()
+    }
 }

Server/src/test/kotlin/APITests.kt

@@ -13,8 +13,8 @@ import rs09.game.node.entity.skill.slayer.SlayerManager
 import rs09.game.system.SystemLogger
 
 object APITests {
-    val testPlayer = Player(PlayerDetails("test", "testing"))
-    val testPlayer2 = Player(PlayerDetails("test2", "testing"))
+    val testPlayer = TestUtils.getMockPlayer("test")
+    val testPlayer2 = TestUtils.getMockPlayer("test2")
 
     @Test fun testIfaceSettings(){
         var builder = IfaceSettingsBuilder()

Server/src/test/kotlin/QuestTests.kt

@@ -0,0 +1,56 @@
+import core.game.node.entity.player.link.quest.Quest
+import core.game.node.entity.player.link.quest.QuestRepository
+import org.junit.jupiter.api.Assertions
+import org.junit.jupiter.api.Test
+
+class QuestTests {
+    val testPlayer = TestUtils.getMockPlayer("test")
+    class TestQuest : Quest("Test Quest", 0, 0, 1, 1, 0, 1, 2) {
+        override fun newInstance(`object`: Any?): Quest {
+            return this
+        }
+    }
+    val testQuest = TestQuest()
+
+    @Test fun getIndexShouldNotThrowException() {
+        Assertions.assertDoesNotThrow {
+            testQuest.index
+        }
+    }
+
+    @Test fun registerShouldMakeQuestImmediatelyAvailable() {
+        QuestRepository.register(testQuest)
+        Assertions.assertNotNull(QuestRepository.getQuests()[testQuest.name])
+    }
+
+    @Test fun registerShouldMakeQuestImmediatelyAvailableToInstances() {
+        QuestRepository.register(testQuest)
+        val instance = QuestRepository(testPlayer)
+        Assertions.assertNotNull(instance.getQuest(testQuest.name))
+    }
+
+    @Test fun getStageOnUnstartedQuestShouldNotThrowException() {
+        QuestRepository.register(testQuest)
+        val instance = QuestRepository(testPlayer)
+        Assertions.assertDoesNotThrow {
+            instance.getStage(testQuest)
+        }
+    }
+
+    @Test fun setStageOnUnstartedQuestShouldNotThrowException() {
+        QuestRepository.register(testQuest)
+        val instance = QuestRepository(testPlayer)
+        Assertions.assertDoesNotThrow {
+            instance.setStage(testQuest, 10)
+        }
+    }
+
+    @Test fun completeQuestShouldThrowExceptionIfAlreadyComplete() {
+        Assertions.assertThrows(IllegalStateException::class.java, {
+            QuestRepository.register(testQuest)
+            val repo = QuestRepository(testPlayer)
+            repo.getQuest("Test Quest").finish(testPlayer)
+            repo.getQuest("Test Quest").finish(testPlayer)
+        }, "Quest completed twice without throwing an exception or threw wrong exception!")
+    }
+}

Server/src/test/kotlin/ShopTests.kt

@@ -0,0 +1,117 @@
+import core.game.node.entity.player.link.IronmanMode
+import core.game.node.item.Item
+import org.junit.Assert
+import org.junit.jupiter.api.Assertions
+import org.junit.jupiter.api.Test
+import rs09.game.content.global.shops.Shop
+
+class ShopTests {
+    val testPlayer = TestUtils.getMockPlayer("test")
+    val testIronman = TestUtils.getMockPlayer("test2", IronmanMode.STANDARD)
+    val nonGeneral = TestUtils.getMockShop("Not General", false, Item(4151, 1))
+    val general = TestUtils.getMockShop("General", true, Item(4151, 1))
+
+    @Test fun shouldSellItemToStore() {
+        testPlayer.inventory.add(Item(4151, 1))
+        testPlayer.setAttribute("shop-cont", general.getContainer(testPlayer))
+        val status = general.sell(testPlayer, 0, 1)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Success, "Transaction failed: ${if(status is Shop.TransactionStatus.Failure) status.reason else ""}")
+    }
+
+    @Test fun shouldNotSellUnstockedItemToStandardStore() {
+        testPlayer.inventory.add(Item(1, 1))
+        testPlayer.setAttribute("shop-cont", nonGeneral.getContainer(testPlayer))
+        val status = nonGeneral.sell(testPlayer, 0, 1)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Failure)
+    }
+
+    @Test fun shouldSellUnstockedItemToGeneralStore() {
+        testPlayer.inventory.add(Item(1, 1))
+        testPlayer.setAttribute("shop-cont", general.getContainer(testPlayer))
+        val status = general.sell(testPlayer, 0, 1)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Success, "Transaction failure: ${if(status is Shop.TransactionStatus.Failure) status.reason else ""}")
+    }
+
+    @Test fun shouldSellUnstockedItemToGeneralStoreAsIronman() {
+        testIronman.inventory.add(Item(1, 1))
+        testIronman.setAttribute("shop-cont", general.getContainer(testPlayer))
+        val status = general.sell(testIronman, 0, 1)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Success, "Transaction failure: ${if(status is Shop.TransactionStatus.Failure) status.reason else ""}")
+    }
+
+    @Test fun shouldSellStackOfUnstockedItemsToPlayerStock() {
+        testPlayer.inventory.add(Item(1, 20))
+        testPlayer.setAttribute("shop-cont", general.getContainer(testPlayer))
+        val status = general.sell(testPlayer, 0, 20)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Success, "Transaction failure: ${if(status is Shop.TransactionStatus.Failure) status.reason else ""}")
+    }
+
+    @Test fun shouldPutSoldUnstockedItemsInPlayerStock() {
+        testPlayer.inventory.add(Item(2,1))
+        testPlayer.setAttribute("shop-cont", general.getContainer(testPlayer))
+        val status = general.sell(testPlayer, 0, 1)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Success, "Transaction failure: ${if(status is Shop.TransactionStatus.Failure) status.reason else ""}")
+        Assertions.assertEquals(1, general.playerStock.getAmount(2))
+        Assertions.assertEquals(0, general.getContainer(testPlayer).getAmount(2))
+    }
+
+    @Test fun shouldAllowStandardPlayerToBuy() {
+        testPlayer.inventory.add(Item(995, 100000))
+        testPlayer.setAttribute("shop-cont", general.getContainer(testPlayer))
+        testPlayer.setAttribute("shop-main", true)
+        val status = general.buy(testPlayer, 0, 1)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Success, "Transaction failure: ${if(status is Shop.TransactionStatus.Failure) status.reason else ""}")
+    }
+
+    @Test fun shouldAllowStandardPlayerToBuyOverstock() {
+        testPlayer.inventory.add(Item(995, 100000))
+        testPlayer.setAttribute("shop-cont", general.getContainer(testPlayer))
+        testPlayer.setAttribute("shop-main", true)
+        general.getContainer(testPlayer).add(Item(4151, 100))
+        val status = general.buy(testPlayer, 0, 1)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Success, "Transaction failure: ${if(status is Shop.TransactionStatus.Failure) status.reason else ""}")
+    }
+
+    @Test fun shouldAllowStandardPlayerToBuyPlayerStock() {
+        testPlayer.inventory.add(Item(995, 100000))
+        testPlayer.setAttribute("shop-cont", general.getContainer(testPlayer))
+        testPlayer.setAttribute("shop-main", false)
+        general.playerStock.add(Item(4151, 100))
+        val status = general.buy(testPlayer, 0, 1)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Success, "Transaction failure: ${if(status is Shop.TransactionStatus.Failure) status.reason else ""}")
+    }
+
+    @Test fun shouldNotAllowIronmanToBuyOverstock() {
+        testIronman.inventory.add(Item(995, 100000))
+        testIronman.setAttribute("shop-cont", general.getContainer(testIronman))
+        testIronman.setAttribute("shop-main", true)
+        general.getContainer(testIronman).add(Item(4151, 100))
+        val status = general.buy(testIronman, 0, 1)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Failure)
+    }
+
+    @Test fun shouldNotAllowIronmanToBuyPlayerStock() {
+        testIronman.inventory.add(Item(995, 100000))
+        testIronman.setAttribute("shop-cont", general.playerStock)
+        testIronman.setAttribute("shop-main", false)
+        general.playerStock.add(Item(4151, 1))
+        val status = general.buy(testIronman, 0, 1)
+        Assertions.assertEquals(true, status is Shop.TransactionStatus.Failure)
+    }
+
+    @Test fun openShopShouldNotThrowException() {
+        Assertions.assertDoesNotThrow {
+            general.openFor(testPlayer)
+        }
+    }
+
+    @Test fun shouldNotThrowExceptionWhenRestockingStockWithNullSlot() {
+        Assertions.assertDoesNotThrow {
+            general.getContainer(testPlayer).add(Item(1, 100))
+            general.getContainer(testPlayer).add(Item(2, 100))
+            general.getContainer(testPlayer).replace(null, 0) //replace item in slot 0 with null
+            for ((k,_) in general.stockInstances) general.needsUpdate[k] = true
+            general.restock()
+        }
+    }
+}

Server/src/test/kotlin/TestUtils.kt

@@ -0,0 +1,20 @@
+import core.game.node.entity.player.Player
+import core.game.node.entity.player.info.PlayerDetails
+import core.game.node.entity.player.link.IronmanMode
+import core.game.node.item.Item
+import rs09.game.ai.ArtificialSession
+import rs09.game.content.global.shops.Shop
+import rs09.game.content.global.shops.ShopItem
+
+object TestUtils {
+    fun getMockPlayer(name: String, ironman: IronmanMode = IronmanMode.NONE): Player {
+        val p = Player(PlayerDetails(name, name))
+        p.details.session = ArtificialSession.getSingleton()
+        p.ironmanManager.mode = ironman
+        return p
+    }
+
+    fun getMockShop(name: String, general: Boolean, vararg stock: Item) : Shop {
+        return Shop(name, stock.map { ShopItem(it.id, it.amount, 100) }.toTypedArray(), general)
+    }
+}