fix: Inheriting an entity should not inherit its prefabs directly

fix: Loading from JAR resources not working tests: Add tests for issues above
MineInAbyss · Oct 27, 2024 · 7888481 · 7888481
1 parent c4f6958
commit 7888481
Show file tree

Hide file tree

Showing 13 changed files with 122 additions and 38 deletions.
diff --git a/addons/geary-prefabs/src/commonMain/kotlin/com/mineinabyss/geary/prefabs/PrefabLoader.kt b/addons/geary-prefabs/src/commonMain/kotlin/com/mineinabyss/geary/prefabs/PrefabLoader.kt
@@ -90,6 +90,7 @@ class PrefabLoader(
             )
             val serializer = PolymorphicListAsMapSerializer.ofComponents(config)
             val format = formats[formatExt] ?: throw IllegalArgumentException("Unknown file format $formatExt")
+            logger.v("Loading prefab $key from $source")
 
             format.decode(
                 serializer,

diff --git a/...ns/geary-prefabs/src/jvmMain/kotlin/com/mineinabyss/geary/prefabs/PrefabsDSLExtensions.kt b/...ns/geary-prefabs/src/jvmMain/kotlin/com/mineinabyss/geary/prefabs/PrefabsDSLExtensions.kt
@@ -3,10 +3,9 @@ package com.mineinabyss.geary.prefabs
 import kotlinx.io.asSource
 import kotlinx.io.buffered
 import kotlinx.io.files.Path
+import java.io.File
 import java.io.InputStream
-import java.net.URL
-import java.util.jar.JarFile
-import kotlin.io.path.pathString
+import kotlin.io.path.*
 import kotlin.reflect.KClass
 
 object PrefabsDSLExtensions {
@@ -17,15 +16,9 @@ object PrefabsDSLExtensions {
         val classLoader = classLoaderRef.java.classLoader
         prefabsBuilder.paths.add(
             PrefabPath(namespaced.namespace) {
-                resources.asSequence().map {
-                    PrefabSource(
-                        source = (classLoader.getResourceAsStream(it)
-                            ?: error("Resource $it not found when loading prefab"))
-                            .asSource().buffered(),
-                        key = PrefabKey.of(namespaced.namespace, it),
-                        formatExt = it.substringAfterLast('.')
-                    )
-                }
+                resources.asSequence()
+                    .map { getResource(classLoader, it) }
+                    .mapNotNull { it?.asPrefabSource(namespaced.namespace) }
             }
         )
     }
@@ -37,13 +30,7 @@ object PrefabsDSLExtensions {
         val classLoader = classLoaderRef.java.classLoader
         prefabsBuilder.paths.add(
             PrefabPath(namespaced.namespace) {
-                walkJarResources(classLoader, folder).map {
-                    PrefabSource(
-                        source = it.asSource().buffered(),
-                        key = PrefabKey.of(namespaced.namespace, it.toString()),
-                        formatExt = it.toString().substringAfterLast('.')
-                    )
-                }
+                walkJarResources(classLoader, folder).map { it.asPrefabSource(namespaced.namespace) }
             }
         )
     }
@@ -56,20 +43,24 @@ object PrefabsDSLExtensions {
         fromDirectory(Path(folder.pathString))
     }
 
+    @OptIn(ExperimentalPathApi::class)
     fun walkJarResources(
         classLoader: ClassLoader,
         directory: String,
-    ): Sequence<InputStream> = sequence {
-        val dirUrl: URL = classLoader.getResource(directory) ?: return@sequence
-        val jarPath = dirUrl.path.substringBefore("!").removePrefix("file:")
-        val jarFile = JarFile(jarPath)
-        val entries = jarFile.entries()
+    ): Sequence<java.nio.file.Path> {
+        val directoryPath = File(classLoader.getResource(directory)?.toURI() ?: return emptySequence()).toPath()
+        return directoryPath.walk().filter { it.isRegularFile() }
+    }
 
-        while (entries.hasMoreElements()) {
-            val entry = entries.nextElement()
-            if (entry.name.startsWith(directory) && !entry.isDirectory) {
-                yield(jarFile.getInputStream(entry))
-            }
-        }
+    fun getResource(classLoader: ClassLoader, path: String): java.nio.file.Path? {
+        return File(classLoader.getResource(path)?.toURI() ?: return null).toPath()
     }
+
+    private fun java.nio.file.Path.asPrefabSource(namespace: String) = PrefabSource(
+        source = inputStream().asSource().buffered(),
+        key = PrefabKey.of(namespace, nameWithoutExtension),
+        formatExt = extension
+    )
+
+    data class NameToStream(val name: String, val stream: InputStream)
 }
diff --git a/...geary-prefabs/src/jvmTest/kotlin/com/mineinabyss/geary/prefabs/PrefabFromResourcesTest.kt b/...geary-prefabs/src/jvmTest/kotlin/com/mineinabyss/geary/prefabs/PrefabFromResourcesTest.kt
@@ -0,0 +1,70 @@
+package com.mineinabyss.geary.prefabs
+
+import com.mineinabyss.geary.modules.TestEngineModule
+import com.mineinabyss.geary.modules.geary
+import com.mineinabyss.geary.prefabs.PrefabsDSLExtensions.fromFiles
+import com.mineinabyss.geary.prefabs.PrefabsDSLExtensions.fromJarResourceDirectory
+import com.mineinabyss.geary.prefabs.PrefabsDSLExtensions.fromJarResources
+import com.mineinabyss.geary.serialization.formats.YamlFormat
+import com.mineinabyss.geary.serialization.serialization
+import io.kotest.matchers.shouldBe
+import io.kotest.matchers.shouldNotBe
+import org.junit.jupiter.api.Test
+
+class PrefabFromResourcesTest {
+    private fun world() = geary(TestEngineModule) {
+        serialization {
+            format("yml", ::YamlFormat)
+        }
+    }
+
+    @Test
+    fun `should load prefabs from resource file`() {
+        val world = world().configure {
+            namespace("test") {
+                prefabs {
+                    fromJarResources(PrefabFromResourcesTest::class, "prefabs/prefabA.yml")
+                }
+            }
+        }.start()
+
+        with(world) {
+            entityOfOrNull(PrefabKey.of("test:prefabA")) shouldNotBe null
+            entityOfOrNull(PrefabKey.of("test:prefabB")) shouldBe null
+        }
+    }
+
+    @Test
+    fun `should load prefabs from resources directory`() {
+        val world = world().configure {
+            namespace("test") {
+                prefabs {
+                    fromJarResourceDirectory(PrefabFromResourcesTest::class, "prefabs")
+                }
+            }
+        }.start()
+        with(world) {
+            entityOfOrNull(PrefabKey.of("test:prefabA")) shouldNotBe null
+            entityOfOrNull(PrefabKey.of("test:prefabB")) shouldNotBe null
+        }
+    }
+
+    @Test
+    fun `should load prefabs from path source`() {
+        val world = world().configure {
+            namespace("test") {
+                prefabs {
+                    fromFiles(
+                        PrefabsDSLExtensions.getResource(
+                            PrefabFromResourcesTest::class.java.classLoader,
+                            "prefabs/prefabA.yml"
+                        )!!
+                    )
+                }
+            }
+        }.start()
+        with(world) {
+            entityOfOrNull(PrefabKey.of("test:prefabA")) shouldNotBe null
+        }
+    }
+}
diff --git a/addons/geary-prefabs/src/jvmTest/resources/prefabs/prefabA.yml b/addons/geary-prefabs/src/jvmTest/resources/prefabs/prefabA.yml
@@ -0,0 +1 @@
+{}
diff --git a/addons/geary-prefabs/src/jvmTest/resources/prefabs/prefabB.yml b/addons/geary-prefabs/src/jvmTest/resources/prefabs/prefabB.yml
@@ -0,0 +1 @@
+{}
diff --git a/addons/geary-serialization/build.gradle.kts b/addons/geary-serialization/build.gradle.kts
@@ -12,7 +12,7 @@ kotlin {
 
                 api(idofrontLibs.kotlinx.serialization.cbor)
                 api(idofrontLibs.kotlinx.serialization.json)
-                api(libs.kotlinx.io)
+                api(idofrontLibs.kotlinx.io)
             }
         }
         jvmMain {

diff --git a/geary-core/build.gradle.kts b/geary-core/build.gradle.kts
@@ -13,6 +13,7 @@ kotlin {
                 implementation(idofrontLibs.kotlin.reflect)
 
                 api(libs.koin.core)
+                api(idofrontLibs.kotlinx.io)
                 api(idofrontLibs.kermit)
                 api(idofrontLibs.kotlinx.coroutines)
             }

diff --git a/...in/kotlin/com/mineinabyss/geary/engine/archetypes/operations/ArchetypeMutateOperations.kt b/...in/kotlin/com/mineinabyss/geary/engine/archetypes/operations/ArchetypeMutateOperations.kt
@@ -63,21 +63,33 @@ class ArchetypeMutateOperations(
         var instanceArch = instanceArch
         var instanceRow = instanceRow
 
+        // Mark instance a InstanceOf baseEntity
         addComponent(instanceArch, instanceRow, Relation.of(components.instanceOf, baseEntity).id, true) { arch, row ->
             instanceArch = arch; instanceRow = row
         }
 
-        val noInheritComponents = baseArchetype.getRelationsByKind(components.noInherit).map { Relation.of(it).target }
+        val basePrefabs = baseArchetype.getRelationsByKind(components.instanceOf)
+
+        // Don't inherit components marked as NoInherit, nor baseEntity's prefabs
+        val noInheritComponents: List<EntityId> = baseArchetype
+            .getRelationsByKind(components.noInherit)
+            .map { Relation.of(it).target }
+            .plus(basePrefabs.map { it })
+
+        // Add all components without data
         baseArchetype.type.filter { !it.holdsData() && it !in noInheritComponents }.forEach {
             addComponent(instanceArch, instanceRow, it, true) { arch, row -> instanceArch = arch; instanceRow = row }
         }
+
+        // Add all components with data
         baseArchetype.dataHoldingType.forEach {
             if (it.withoutRole(HOLDS_DATA) in noInheritComponents) return@forEach
             setComponent(instanceArch, instanceRow, it, baseArchetype.getUnsafe(baseRow, it), true) { arch, row ->
                 instanceArch = arch; instanceRow = row
             }
         }
 
+        // Children of instantiated prefabs should be children of the instance
         queryManager.childrenOf(baseEntity).forEach { child ->
             // Add instanceEntity as parent
             addComponentFor(instanceEntity, components.couldHaveChildren, true)

diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/modules/Geary.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/modules/Geary.kt
@@ -167,3 +167,5 @@ inline fun <reified T : Any> Geary.observeWithData(): ObserverWithData<T> {
 
 inline fun Geary.findEntities(init: MutableFamily.Selector.And.() -> Unit) =
     findEntities(family(init))
+
+inline fun Geary.findEntities(query: Query) = findEntities(query.buildFamily())
diff --git a/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/modules/GearyModule.kt b/geary-core/src/commonMain/kotlin/com/mineinabyss/geary/modules/GearyModule.kt
@@ -2,7 +2,6 @@ package com.mineinabyss.geary.modules
 
 import org.koin.core.module.Module
 import org.koin.dsl.koinApplication
-import org.koin.dsl.module
 
 fun geary(
     module: GearyModule,
@@ -23,7 +22,7 @@ data class UninitializedGearyModule(
     val setup: GearySetup,
     val initializer: EngineInitializer,
 ) {
-    inline fun configure(configure: GearySetup.() -> Unit) = setup.configure()
+    inline fun configure(configure: GearySetup.() -> Unit): UninitializedGearyModule = apply { setup.configure() }
 
     fun start(): Geary {
         val world = Geary(setup.application)

diff --git a/geary-core/src/jvmTest/kotlin/com/mineinabyss/geary/instancing/InstancingTest.kt b/geary-core/src/jvmTest/kotlin/com/mineinabyss/geary/instancing/InstancingTest.kt
@@ -2,10 +2,10 @@ package com.mineinabyss.geary.instancing
 
 import com.mineinabyss.geary.components.relations.NoInherit
 import com.mineinabyss.geary.helpers.entity
-import com.mineinabyss.geary.test.GearyTest
 import com.mineinabyss.geary.modules.observe
 import com.mineinabyss.geary.modules.relationOf
 import com.mineinabyss.geary.observers.events.OnSet
+import com.mineinabyss.geary.test.GearyTest
 import io.kotest.matchers.nulls.shouldBeNull
 import io.kotest.matchers.shouldBe
 import org.junit.jupiter.api.BeforeEach
@@ -83,4 +83,12 @@ class InstancingTest : GearyTest() {
         instance.get<String>() shouldBe "test"
         instance.get<Int>() shouldBe 1
     }
+
+    @Test
+    fun `should not inherit parent prefabs`() {
+        val parent = entity()
+        val prefab = entity { extend(parent) }
+        val entity = entity { extend(prefab) }
+        entity.prefabs shouldBe setOf(prefab)
+    }
 }
diff --git a/gradle.properties b/gradle.properties
@@ -2,5 +2,5 @@ group=com.mineinabyss
 version=0.27
 # Workaround for dokka builds failing on CI, see https://github.com/Kotlin/dokka/issues/1405
 #org.gradle.jvmargs=-XX:MaxMetaspaceSize=512m
-idofrontVersion=0.25.13
+idofrontVersion=0.25-dev
 kotlin.native.ignoreDisabledTargets=true
diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -7,7 +7,6 @@ roaringbitmap = "1.0.6"
 statelyConcurrency = "2.0.7"
 koin = "4.0.0"
 junitJupiter = "5.8.1"
-kotlinxIO = "0.5.4"
 
 [libraries]
 androidx-collection = { module = "androidx.collection:collection", version.ref = "androidxCollection" }
@@ -19,4 +18,3 @@ stately-concurrency = { module = "co.touchlab:stately-concurrency", version.ref
 koin-core = { module = "io.insert-koin:koin-core", version.ref = "koin" }
 koin-test = { module = "io.insert-koin:koin-test", version.ref = "koin" }
 junit-jupiter = { group = "org.junit.jupiter", name = "junit-jupiter", version.ref = "junitJupiter" }
-kotlinx-io = { module = "org.jetbrains.kotlinx:kotlinx-io-core", version.ref = "kotlinxIO" }
Original file line number	Diff line number	Diff line change
Expand Up		@@ -167,3 +167,5 @@ inline fun <reified T : Any> Geary.observeWithData(): ObserverWithData<T> {

		inline fun Geary.findEntities(init: MutableFamily.Selector.And.() -> Unit) =
		findEntities(family(init))

		inline fun Geary.findEntities(query: Query) = findEntities(query.buildFamily())