From 0854baa98634abec4589f65fa2c11bfc9dfc2a5b Mon Sep 17 00:00:00 2001
From: DBotThePony <dbotthepony@yandex.ru>
Date: Sun, 28 Apr 2024 18:18:00 +0700
Subject: [PATCH] Semi-fix microdungeons overlap

---
 ADDITIONS.md                                  | 11 ++++-
 .../kstarbound/defs/dungeon/DungeonWorld.kt   | 18 +------
 .../kstarbound/server/world/ServerChunk.kt    | 48 +++++++++----------
 .../kstarbound/server/world/ServerWorld.kt    | 44 +++++++++++++++++
 .../kstarbound/util/ExecutionTimePacer.kt     | 26 +++++++---
 5 files changed, 98 insertions(+), 49 deletions(-)

diff --git a/ADDITIONS.md b/ADDITIONS.md
index 8b77b1af..b6dcbf7e 100644
--- a/ADDITIONS.md
+++ b/ADDITIONS.md
@@ -135,10 +135,10 @@ val color: TileColor = TileColor.DEFAULT
 ## Deterministic world generation
 
 In new engine, entirety of world generation is made deterministic. What this means that given one world seed, engine will
-generate _exactly the same_ world each time it is requested to generate one (given prototype definitions which influence
+generate _exactly the same_ (on best effort*) world each time it is requested to generate one (given prototype definitions which influence
 world generation are the same between generations).
 
-To put it simply, when you visit a planet on your friend's server, it is _guaranteed_ that in your singleplayer
+To put it simply, when you visit a planet on your friend's server, it is expected* that in your singleplayer
 or on other server, given same set of mods installed (and both players are using new engine server or new engine client),
 you will get exactly the same planet as you saw before.
 
@@ -159,6 +159,13 @@ there is `seed` specified for such world `/instance_worlds.config`. And since va
 If you are mod creator, **PLEASE** update your mod(s), and remove `seed` from your dungeon worlds!
 Both new and old engines will provide random seed for you if you don't specify one inside `/instance_worlds.config`.
 
+*On best effort - due to how worldgen code flow is structured, engine _may_ rearrange generation events, which can yield
+_slightly_ different results from execution to execution,
+such as one microdungeon taking precedence over another microdungeon
+if they happen to generate in proximity on chunk border (one dungeon generated in chunk A, second generated in chunk B,
+and they happened to overlap each other),
+and which one gets placed is determined by who finishes generating first.
+
 ---------------
 
 ## Behavior
diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/defs/dungeon/DungeonWorld.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/defs/dungeon/DungeonWorld.kt
index da6ef5b6..a95dc18f 100644
--- a/src/main/kotlin/ru/dbotthepony/kstarbound/defs/dungeon/DungeonWorld.kt
+++ b/src/main/kotlin/ru/dbotthepony/kstarbound/defs/dungeon/DungeonWorld.kt
@@ -94,8 +94,8 @@ class DungeonWorld(val parent: ServerWorld, val random: RandomGenerator, val mar
 	// entities themselves to be removed
 	private val tileEntitiesToRemove = HashSet<TileEntity>(2048, 0.5f)
 
-	private val touchedTiles = HashSet<Vector2i>(16384, 0.5f)
-	private val protectTile = HashSet<Vector2i>(16384, 0.5f)
+	val touchedTiles = HashSet<Vector2i>(16384, 0.5f)
+	val protectTile = HashSet<Vector2i>(16384, 0.5f)
 
 	private val boundingBoxes = ArrayList<AABBi>()
 
@@ -624,20 +624,6 @@ class DungeonWorld(val parent: ServerWorld, val random: RandomGenerator, val mar
 					}
 				}
 			}.await()
-
-			if (targetChunkState != ChunkState.FULL) {
-				// and finally, schedule chunks to be loaded into FULL state
-				// this way, dungeons won't get cut off when chunks being saved
-				// to disk because of dungeon bleeding into neighbour chunks who
-				// never get promoted further
-				// But this might trigger cascading world generation
-				// (dungeon generates another dungeon, and another, and so on),
-				// tough, so need to take care!
-				for (box in boundingBoxes) {
-					// specify timer as 0 so ticket gets removed on next world tick
-					parent.temporaryChunkTicket(box, 0, ChunkState.FULL)
-				}
-			}
 		} finally {
 			tickets.forEach { it.cancel() }
 		}
diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/server/world/ServerChunk.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/server/world/ServerChunk.kt
index 52da6942..9e8ba9ca 100644
--- a/src/main/kotlin/ru/dbotthepony/kstarbound/server/world/ServerChunk.kt
+++ b/src/main/kotlin/ru/dbotthepony/kstarbound/server/world/ServerChunk.kt
@@ -1,20 +1,14 @@
 package ru.dbotthepony.kstarbound.server.world
 
-import com.google.gson.JsonObject
 import it.unimi.dsi.fastutil.objects.ObjectAVLTreeSet
 import kotlinx.coroutines.channels.Channel
-import kotlinx.coroutines.delay
 import kotlinx.coroutines.future.await
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.suspendCancellableCoroutine
 import org.apache.logging.log4j.LogManager
 import ru.dbotthepony.kommons.arrays.Object2DArray
-import ru.dbotthepony.kommons.gson.JsonArray
-import ru.dbotthepony.kommons.gson.set
 import ru.dbotthepony.kommons.guava.immutableList
 import ru.dbotthepony.kstarbound.math.AABBi
-import ru.dbotthepony.kommons.util.KOptional
-import ru.dbotthepony.kstarbound.Registries
 import ru.dbotthepony.kstarbound.math.vector.Vector2d
 import ru.dbotthepony.kstarbound.math.vector.Vector2i
 import ru.dbotthepony.kstarbound.Starbound
@@ -37,10 +31,8 @@ import ru.dbotthepony.kstarbound.defs.world.Biome
 import ru.dbotthepony.kstarbound.defs.world.BiomePlaceables
 import ru.dbotthepony.kstarbound.defs.world.FloatingDungeonWorldParameters
 import ru.dbotthepony.kstarbound.defs.world.WorldTemplate
-import ru.dbotthepony.kstarbound.json.jsonArrayOf
 import ru.dbotthepony.kstarbound.network.LegacyNetworkCellState
 import ru.dbotthepony.kstarbound.network.packets.clientbound.TileDamageUpdatePacket
-import ru.dbotthepony.kstarbound.util.ExecutionTimePacer
 import ru.dbotthepony.kstarbound.util.random.random
 import ru.dbotthepony.kstarbound.util.random.staticRandom64
 import ru.dbotthepony.kstarbound.util.random.staticRandomDouble
@@ -48,7 +40,6 @@ import ru.dbotthepony.kstarbound.world.CHUNK_SIZE_FF
 import ru.dbotthepony.kstarbound.world.Chunk
 import ru.dbotthepony.kstarbound.world.ChunkPos
 import ru.dbotthepony.kstarbound.world.ChunkState
-import ru.dbotthepony.kstarbound.world.Direction
 import ru.dbotthepony.kstarbound.world.IChunkListener
 import ru.dbotthepony.kstarbound.world.TileHealth
 import ru.dbotthepony.kstarbound.world.api.AbstractCell
@@ -58,20 +49,16 @@ import ru.dbotthepony.kstarbound.world.api.MutableTileState
 import ru.dbotthepony.kstarbound.world.api.TileColor
 import ru.dbotthepony.kstarbound.world.entities.AbstractEntity
 import ru.dbotthepony.kstarbound.world.entities.ItemDropEntity
-import ru.dbotthepony.kstarbound.world.entities.tile.PlantEntity
-import ru.dbotthepony.kstarbound.world.entities.tile.WorldObject
 import java.util.concurrent.CompletableFuture
 import java.util.concurrent.CopyOnWriteArrayList
 import java.util.concurrent.TimeUnit
 import java.util.concurrent.locks.ReentrantLock
 import java.util.function.Predicate
 import java.util.function.Supplier
-import java.util.random.RandomGenerator
 import kotlin.concurrent.withLock
 import kotlin.coroutines.cancellation.CancellationException
 import kotlin.coroutines.resume
 import kotlin.coroutines.resumeWithException
-import kotlin.math.min
 
 class ServerChunk(world: ServerWorld, pos: ChunkPos) : Chunk<ServerWorld, ServerChunk>(world, pos) {
 	override var state: ChunkState = ChunkState.FRESH
@@ -772,22 +759,33 @@ class ServerChunk(world: ServerWorld, pos: ChunkPos) : Chunk<ServerWorld, Server
 						if (!bounds.isInside(pos) || !bounds.isInside(pos + anchor.reader.size - Vector2i.POSITIVE_XY))
 							continue
 
-						// this is quite ugly code flow, but we should try to avoid double-walking
-						// over all dungeon tiles (DungeonTile#canPlace already checks for absence of other dungeons on their place,
-						// so we only need to tell DungeonPart to not force-place)
-						if (anchor.canPlace(pos.x, pos.y, world, false)) {
-							try {
-								dungeon.value!!.build(anchor, world, random, pos.x, pos.y, dungeonID = MICRO_DUNGEON_ID).await()
-							} catch (err: Throwable) {
-								LOGGER.error("Error while generating microdungeon ${dungeon.key.left()} at $pos", err)
+						val placed = world.queueMicrodungeonPlacement(pos.x, pos.y) {
+							// this is quite ugly code flow, but we should try to avoid double-walking
+							// over all dungeon tiles (DungeonTile#canPlace already checks for absence of other dungeons on their place,
+							// so we only need to tell DungeonPart to not force-place)
+							if (it.measureAndSuspend { anchor.canPlace(pos.x, pos.y, world, false) }) {
+								try {
+									val dungeonWorld = dungeon.value!!.build(anchor, world, random, pos.x, pos.y, dungeonID = MICRO_DUNGEON_ID, commit = false).await()
+									val placementIsFree = dungeonWorld.touchedTiles.all { world.getCell(it).dungeonId == NO_DUNGEON_ID }
+
+									if (placementIsFree) {
+										dungeonWorld.commit()
+									} else {
+										LOGGER.debug("Dungeons overlap somewhere around {} after built new dungeon, not placing {}", pos, dungeon.key.left())
+									}
+								} catch (err: Throwable) {
+									LOGGER.error("Error while generating microdungeon ${dungeon.key.left()} at $pos", err)
+								}
+
+								return@queueMicrodungeonPlacement true
 							}
 
-							break
+							return@queueMicrodungeonPlacement false
 						}
 
-						// some breathing room for other code, since placement checking is performance intense operation
-						if (!world.isInPreparation && world.clients.isNotEmpty())
-							delay(min(60L, anchor.reader.size.x * anchor.reader.size.y / 240L))
+						if (placed) {
+							break
+						}
 					}
 				}
 			}
diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/server/world/ServerWorld.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/server/world/ServerWorld.kt
index eb459a2c..36ce0e85 100644
--- a/src/main/kotlin/ru/dbotthepony/kstarbound/server/world/ServerWorld.kt
+++ b/src/main/kotlin/ru/dbotthepony/kstarbound/server/world/ServerWorld.kt
@@ -9,6 +9,7 @@ import it.unimi.dsi.fastutil.objects.ObjectArraySet
 import kotlinx.coroutines.async
 import kotlinx.coroutines.future.asCompletableFuture
 import kotlinx.coroutines.future.await
+import kotlinx.coroutines.launch
 import kotlinx.coroutines.runBlocking
 import org.apache.logging.log4j.LogManager
 import ru.dbotthepony.kstarbound.math.AABB
@@ -43,6 +44,7 @@ import ru.dbotthepony.kstarbound.server.ServerConnection
 import ru.dbotthepony.kstarbound.util.AssetPathStack
 import ru.dbotthepony.kstarbound.util.BlockableEventLoop
 import ru.dbotthepony.kstarbound.util.ActionPacer
+import ru.dbotthepony.kstarbound.util.ExecutionTimePacer
 import ru.dbotthepony.kstarbound.util.random.random
 import ru.dbotthepony.kstarbound.world.ChunkPos
 import ru.dbotthepony.kstarbound.world.ChunkState
@@ -52,11 +54,14 @@ import ru.dbotthepony.kstarbound.world.entities.AbstractEntity
 import ru.dbotthepony.kstarbound.world.entities.tile.TileEntity
 import ru.dbotthepony.kstarbound.world.entities.tile.WorldObject
 import ru.dbotthepony.kstarbound.world.physics.CollisionType
+import java.util.PriorityQueue
 import java.util.concurrent.CompletableFuture
 import java.util.concurrent.CopyOnWriteArrayList
 import java.util.concurrent.RejectedExecutionException
 import java.util.concurrent.TimeUnit
 import java.util.function.Supplier
+import kotlin.coroutines.Continuation
+import kotlin.coroutines.suspendCoroutine
 
 class ServerWorld private constructor(
 	val server: StarboundServer,
@@ -177,6 +182,45 @@ class ServerWorld private constructor(
 		}, 0L, Starbound.TIMESTEP_NANOS, TimeUnit.NANOSECONDS)
 	}
 
+	private var placementTaskID = 0L
+
+	private data class PlacementElement<T>(val x: Int, val y: Int, val id: Long, val callback: suspend (ExecutionTimePacer) -> T, val future: Continuation<T>) : Comparable<PlacementElement<*>> {
+		override fun compareTo(other: PlacementElement<*>): Int {
+			var cmp = x.compareTo(other.x)
+			if (cmp == 0) cmp = y.compareTo(other.y)
+			if (cmp == 0) cmp = id.compareTo(other.id)
+			return cmp
+		}
+	}
+
+	// not Channel because we must be able to select elements by priority
+	private val placementQueue = PriorityQueue<PlacementElement<*>>()
+	private var placementQueueIsActive = false
+	private val placementPacer = ExecutionTimePacer(5_000_000L, 16L)
+
+	private suspend fun placementQueueLoop() {
+		placementQueueIsActive = true
+
+		while (placementQueue.isNotEmpty()) {
+			val next = placementQueue.remove() as PlacementElement<Any?>
+			next.future.resumeWith(Result.success(next.callback(if (isInPreparation || clients.isEmpty()) ExecutionTimePacer.UNLIMITED else placementPacer)))
+		}
+
+		placementQueueIsActive = false
+	}
+
+	// this is used for scheduling and resolving microdungeon placement
+	// tries to early-resolve artifacts like this: https://i.dbotthepony.ru/2024/04/28/gb6GdbLox7.png
+	suspend fun <T> queueMicrodungeonPlacement(x: Int, y: Int, callback: suspend (ExecutionTimePacer) -> T): T {
+		return suspendCoroutine {
+			placementQueue.add(PlacementElement(x, y, placementTaskID++, callback, it))
+
+			if (!placementQueueIsActive) {
+				eventLoop.scope.launch { placementQueueLoop() }
+			}
+		}
+	}
+
 	override fun toString(): String {
 		return "Server World $worldID"
 	}
diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/util/ExecutionTimePacer.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/util/ExecutionTimePacer.kt
index aaf28d54..e517d08c 100644
--- a/src/main/kotlin/ru/dbotthepony/kstarbound/util/ExecutionTimePacer.kt
+++ b/src/main/kotlin/ru/dbotthepony/kstarbound/util/ExecutionTimePacer.kt
@@ -2,13 +2,27 @@ package ru.dbotthepony.kstarbound.util
 
 import kotlinx.coroutines.delay
 
-class ExecutionTimePacer(private val budget: Long, private val pause: Long) {
-	private var origin = System.nanoTime()
+class ExecutionTimePacer(val budget: Long, val pause: Long) {
+	var elapsed = 0L
 
-	suspend fun measureAndSuspend() {
-		if (System.nanoTime() - origin >= budget) {
-			delay(pause)
-			origin = System.nanoTime()
+	suspend inline fun <T> measureAndSuspend(block: () -> T): T {
+		if (budget <= 0L || pause <= 0L) {
+			return block()
 		}
+
+		val origin = System.nanoTime()
+		val result = block()
+		elapsed += System.nanoTime() - origin
+
+		if (elapsed >= budget) {
+			elapsed = 0
+			delay(pause)
+		}
+
+		return result
+	}
+
+	companion object {
+		val UNLIMITED = ExecutionTimePacer(0L, 0L)
 	}
 }