Quillraven / Fleks

Fast, lightweight, multi-platform entity component system in Kotlin
MIT License
174 stars 19 forks source link

FleksNoSuchEntityComponentException: Entity has no component of type 'BComponent'. #125

Closed ludek17052 closed 9 months ago

ludek17052 commented 9 months ago

Hello, Is it a bug or am I doing something wrong? I add Component (name = BComponent) to Entity and in System class itterating over entities with BComponent is running. I can check if Entity has a BComponent: entity.has(BComponent) = true But if I call entity.get(BComponent), it will stops with FleksNoSuchEntityComponentException.

I suppose if "entity.has(BComponent)" is true, then "entity.get(BComponent)" should be not null or Exception.

Output in my console:

CmpSystem One onTickEntity CmpSystem Two onTickEntity entity.has(BComponent) = true Exception in thread "main" com.github.quillraven.fleks.FleksNoSuchEntityComponentException: Entity 'Entity(id=0, version=0)' has no component of type 'BComponent'.

class MOneTest : KtxGame<KtxScreen>() {

    override fun create() {
        addScreen(GameScreen())
        setScreen<GameScreen>()
    }

    override fun render() {
        currentScreen.render(Gdx.graphics.deltaTime)
    }
}

class GameScreen() : KtxScreen {

    private val eWorld = configureWorld {
        systems {
            add(CmpSystemOne())
            add(CmpSystemTwo())
        }
    }

    init {
        eWorld.entity {
            it += AComponent
        }
    }

    override fun render(delta: Float) {
        eWorld.update(delta)
    }
}

class AComponent() : Component<AComponent> {
    override fun type() = AComponent

    companion object : ComponentType<AComponent>()
}

class BComponent() : Component<BComponent> {
    override fun type() = BComponent

    companion object : ComponentType<BComponent>()
}

class CmpSystemOne() : IteratingSystem(World.family { all(AComponent) }) {
    override fun onTickEntity(entity: Entity) {
        println("CmpSystem One onTickEntity")
        entity.configure {
            it += BComponent
        }
    }
}

class CmpSystemTwo() : IteratingSystem(World.family { all(BComponent) }) {
    override fun onTickEntity(entity: Entity) {
        println("CmpSystem Two onTickEntity")
        println("entity.has(BComponent) = " + entity.has(BComponent))
        val bCmp = entity.get(BComponent)
    }
}
Quillraven commented 9 months ago

You did something wrong which was unfortunately introduced in the last version when we introduced Tags (=component without data, like a flag).

Your line it += AComponent is wrong. You need to write it += AComponent() (note the brackets). Before this was a compile error but with the introduction of Tags this is now allowed but should only be used in combination with tags.

I will try to figure out, if it is possible to make that better again to get a compilation error again for components or at least throw an exception if it is wrongly used.

edit: to elaborate some more. You are basically adding a ComponentType to the entity but not the component itself. That's why has returns true because the type is set for the entity, which is all we need for Tags. However, you did not add the component instance itself because of the missing brackets and that's why you get the no component exception.

Quillraven commented 9 months ago

@czyzby: Hello my Kotlin mentor once again! Do you have an idea how to solve that in a convenient way in Kotlin? Before, we just had a Component class in Fleks to uniquely assign an ID to a custom component. Since the last release we also have Tag which also has a unique ID but does not require a component class. It is just an ID.

To add a component you need to write: entity += Component().

For tags it is entity += Tag (=without brackets because it is just a "global" ID and not a specific instance of an object).

Unfortunately, this now allows the following: entity += Component (=again without brackets) which is wrong and actually threw a compiler error in previous versions.

Is there an easy way to solve that? Here are the relevant code segments:

// component.kt

/**
 * An interface that specifies a unique [id].
 * This [id] is used internally by Fleks as an index for some arrays.
 */
// interface was necessary for (#118) to support enum classes as entity tags
// because enum classes can only inherit from an interface and not from an abstract class.
interface UniqueId<T> {
    val id: Int

    @ThreadLocal
    companion object {
        internal var nextId = 0
    }
}

/**
 * An abstract class that assigns a unique [id] per type of [Component] starting from 0.
 * Every [Component] class must have at least one [ComponentType] which serves
 * as a [UniqueId].
 */
@Serializable
abstract class ComponentType<T> : UniqueId<T> {
    override val id: Int = UniqueId.nextId++
}

/**
 * Type alias for a special type of [ComponentType] that is used to tag [entities][Entity].
 * A tag is a special form of a [Component] that does not have any data. It is stored
 * more efficiently when compared to an empty [Component] and should therefore be preferred
 * in those cases.
 */
typealias EntityTag = ComponentType<Any>

/**
 * Type alias for a special type of [UniqueId]. It can be used to make values of an enum
 * class an [EntityTag].
 *
 * ```
 * enum class MyTags : EntityTags by entityTagOf() {
 *     TAG_A, TAG_B
 * }
 * ```
 */
typealias EntityTags = UniqueId<Any>
// entity.kt

/**
 * A class that extends the extension functionality of an [EntityComponentContext] by also providing
 * the possibility to create [components][Component].
 */
open class EntityCreateContext(
    compService: ComponentService,
    @PublishedApi
    internal val compMasks: Bag<BitArray>,
) : EntityComponentContext(compService) {

    /**
     * Adds the [component] to the [entity][Entity].
     *
     * The [onAdd][Component.onAdd] lifecycle method
     * gets called after the [component] is assigned to the [entity][Entity].
     *
     * If the [entity][Entity] already had such a [component] then the [onRemove][Component.onRemove]
     * lifecycle method gets called on the previously assigned component before the [onAdd][Component.onAdd]
     * lifecycle method is called on the new component.
     */
    inline operator fun <reified T : Component<T>> Entity.plusAssign(component: T) {    }

    /**
     * Adds the [components] to the [entity][Entity]. This function should only be used
     * in exceptional cases.
     * It is preferred to use the [plusAssign] function whenever possible to have type-safety.
     *
     * The [onAdd][Component.onAdd] lifecycle method
     * gets called after each component is assigned to the [entity][Entity].
     *
     * If the [entity][Entity] already has such a component then the [onRemove][Component.onRemove]
     * lifecycle method gets called on the previously assigned component before the [onAdd][Component.onAdd]
     * lifecycle method is called on the new component.
     */
    operator fun Entity.plusAssign(components: List<Component<*>>) {    }

    /**
     * Sets the [tag][EntityTag] to the [entity][Entity].
     */
    operator fun Entity.plusAssign(tag: UniqueId<*>) {    }

    /**
     * Sets all [tags][EntityTag] on the given [entity][Entity].
     */
    @JvmName("plusAssignTags")
    operator fun Entity.plusAssign(tags: List<UniqueId<*>>) {    }

}

The problems are the new plusAssign functions that take a UniqueId. I'd need something like "UniqueId which is NOT a ComponentType".

Quillraven commented 9 months ago

This might be a working solution but didn't try it yet in Fleks directly. Maybe there is an easier/better solution.

1) I made EntityTag now also an abstract class (that might not be needed) 2) I added a new plusAssign for EntityTags enum classes 3) change the plusAssing UniqueId function to plusAssign EntityTag

This throws a compile error for the invalid Component1 line, which is what we need. IDs are also correctly assigned according to the println statements. Need to check if this is also properly working in Fleks, when I do those adjustments ;)

interface UniqueId<T> {
    val id: Int

    companion object {
        internal var nextId = 0
    }
}

abstract class ComponentType<T> : UniqueId<T> {
    override val id: Int = UniqueId.nextId++
}

abstract class EntityTag : UniqueId<Any> {
    override val id: Int = UniqueId.nextId++
}

typealias EntityTags = UniqueId<Any>

fun entityTagOf(): EntityTag = object : EntityTag() {}

interface Component<T> {
    fun type(): ComponentType<T>
}

data class Entity(val id: Int, val version: UInt) {
    companion object {
        val NONE = Entity(-1, 0u)
    }
}

inline operator fun <reified T : Component<T>> Entity.plusAssign(component: T) {}

operator fun Entity.plusAssign(tag: EntityTag) {}

operator fun Entity.plusAssign(tag: EntityTags) {}

class Component1 : Component<Component1> {
    override fun type() = Component1

    companion object : ComponentType<Component1>()
}

enum class MyTags : EntityTags by entityTagOf() {
    TAG1
}

data object TAG2 : EntityTag()

fun main() {
    val entity = Entity(0, 0U)

    entity += Component1()
    // entity += Component1 <- compile error

    entity += MyTags.TAG1
    entity += TAG2

    println(Component1.id)
    println(MyTags.TAG1.id)
    println(TAG2.id)
}
Quillraven commented 9 months ago

@ludek17052: please try the SNAPSHOT release that should be ready soon. This should give you a compile error again, when you assign a component incorrectly.

ludek17052 commented 9 months ago

Thank you. Yes, now there is compile error. I think it looks better.

czyzby commented 9 months ago

@Quillraven I'd do pretty much the same thing, except abstract class is probably not needed. Use distinct or stricter interfaces for components and tags so that the component classes do not match the plusAssign method that expects the tags. Changing those type aliases into interfaces, and using those for the operator methods should do it.