Using property delegates with ashley

[pylvain]

Hi,

I'm surprised that the many conveniences ashley.ktx offers don't contain property delegates to simplify components access. We can declare the following delegate :

class MapperDelegate<T : Component>(val mapper: ComponentMapper<T>) {
    operator fun getValue(thisRef: Entity, property: KProperty<*>): T {
        return mapper[thisRef]
    }

    operator fun setValue(thisRef: Entity, property: KProperty<*>, value: T) {
        thisRef.add(value)
    }
}

With a simple component, we can then do that :

data class Location(var x: Int = 0, var y: Int = 0) : Component
val LocationMapper = mapperFor<Location>()
var Entity.location: Location by MapperDelegate(LocationMapper)

And then using it like that

val engine = Engine()
val e = Entity()
engine.addEntity(e)
e.add(Location(3,2))

println(e.location.x) // prints 3

Is this a good idea, or am I missing something ?

[czyzby]

You're not missing anything, that's actually an interesting idea to expand the Ashley DSL. @Jkly has designed the ktx-ashley module, so they might have some input on this, but I think we can add this as a utility as proposed. I'd go as far as to provide an additional utility function to avoid defining a mapper altogether with something along the lines of var Entity.location: Location by mapperPropertyFor<Location>().

@pylvain, would you like to submit a pull request?

[czyzby]

#422

[pylvain]

I'm not used to github, but it will be my pleasure to submit a pull request. I will work on that this week. Thank you for your return.

[czyzby]

@pylvain Take your time. From the looks of it, it's already mostly implemented and just needs some unit tests. :) If you have any problems, don't hesitate to ask here or on libGDX Discord.

[pylvain]

Listening to your suggestion, I wrote the following function :

inline fun < reified T : Component> propertyFor() = MapperDelegate<T>(mapperFor<T>())
var Entity.location by propertyFor<Location>()

Which is indeed much nicer.
I'm just wondering about a performance issue : does mapperFor() instanciate a new ComponentMapper each time it is called, or does it just give a reference to an existing mapper (Maybe I shoud read the source )?

[pylvain]

That's fine for me, but that only works if there is already a component. I was thinking of the case where the function is called with null on an Entity with no such component. Ashley's function remove allows the removal of non existent components by returning null, but we cannot really do such thing in an elegant way in setValue. Also I cannot call the remove function from the delegate, because I don't have access to the reified type of T (but we can surely find a workaround).
I would rather throw an exception with a null argument supplied and have the user remove the component explicitly, I think it's better practice.

[czyzby]

1. optionalPropertyFor can be an inline function that extracts and assigns the component type as a private variable, similarly to how it initiates a private mapper.
2. Setting a mutable nullable variable to null twice is a valid piece of code, even if it has no effect. It's true that we're not able to differentiate between removing a component and a no-op that only attempts to do so, but this is an acceptable side effect of the property API. If the user does need the information if a component was removed, they can still use remove directly.

[czyzby]

Consider what we're trying to replicate here. This is a valid piece of code:

class A(var b: String? = null)

fun noOpSetter() {
  val a = A()
  a.b = null
  a.b = null
}

Just like the user would not expect a NPE in this situation, we also shouldn't differentiate between a proper removal and a no-op.

[pylvain]

Ok, I can understand your point of view. It's now implemented that way.
I've found a workaround, the optionalPropertyFor function is implemented.
I'll wait for suggestions for a little bit, try it for myself a bit too and submit the pull.

[czyzby]

Thanks.

By the way, @Quillraven you might want to look into this.

[czyzby]

This functionality was added in #422. Thanks again for the suggestion and implementation, @pylvain!

[Quillraven]

I never used delegation so far, I think. How does the generated byte code look like, that would be my only concern because you want to have your component access as fast as possible.

It looks very similar to the approach that I always used where I was wondering if there is a way to have a general solution, and it looks like there is with this delegation approach!

Here is an example how I did it for every necessary component:

class StateComponent: Component {
companion object {
    val MAPPER = mapperFor<StateComponent>()
  }
}

/**
 * Returns a [StateComponent] or throws a [GdxRuntimeException] if it doesn't exist.
 */
val Entity.stateCmp: StateComponent
  get() = this[StateComponent.MAPPER]
    ?: throw GdxRuntimeException("StateComponent for entity '$this' is null")

[czyzby]

@Quillraven

I never used delegation so far, I think.

You did if you have ever used by lazy {}. :)

How does the generated byte code look like, that would be my only concern because you want to have your component access as fast as possible.

It generates a static class with a static delegate instance. Property getter and setter are both static methods that consume the Entity as a parameter and call the delegate methods. KProperty instances necessary to support the delegates are also cached and defined at compile time. Delegate is pretty much a standard class that keeps the mapper reference as a private variable. The only thing that might have some slight overhead is the handling of a KProperty parameter (reflection support), but since it remains unused, I don't think that's a major problem - it might cause an extra null check on every call, but a smart JVM might probably skip that since the parameter is otherwise unused. So, TL;DR: compared to using a mapper directly, there's an overhead of 2 function calls tops, one static and one final.

[Quillraven]

Okay cool, thanks for the explanation! I wrote "I think" because I wasn't sure if any of the Kotlin stuff I use does that for me under the hood and indeed I use by lazy from time to time ;)

[pylvain]

I added a tag functionality, in the same spirit :

class Visible : Component
var Entity.isVisible:Boolean by tag<Visible>()

The get returns true or false if the component is present or not
The set removes the component if called with false, or create it if called with true

    val engine = Engine()
    val e = Entity()
    engine.addEntity(e)

    println(e.isVisible) // false

    e.isVisible = true
    println(e.isVisible) // true

    e.isVisible = false
    println(e.isVisible) //false

I will be happy to submit another pull request if you want me to

[pylvain]

I can also add a notTagged() function that returns a delegates with opposite value

[czyzby]

For consistency, let's name it tagFor. I think the opposite check is unnecessary (you can always do !e.isVisible).

[czyzby]

You can go ahead with the pull request, I think it might be pretty useful.