Adding TagDelegate and tagFor

[pylvain]

#427

[pylvain]

After further testing, I think I want to force the user to specify a default value everytime. I did not think that with type inference, it would just look like:

var Entity.isVisible = tagFor(Visible())

which is perfectly fine IMO and avoids reflection. I will edit the pull request in consequence.

I was thinking of adding another delagated property that would be used just to check the existence of a component. Maybe i'll do another pr.

[czyzby]

I was thinking of adding another delagated property that would be used just to check the existence of a component. Maybe i'll do another pr.

I don't think that's necessary, since you can use a read-only TagDelegate to achieve just that.

[czyzby]

After further testing, I think I want to force the user to specify a default value everytime.

I think it makes sense to provide both options, depending on the use case - the user might need either a singleton that is always reused or a provider that creates a new instance (through lambda or with reflection). It mostly comes down to whether the component has any extra fields. You can apply your changes as you see fit and I'll likely merge this as is, but I might just add a few extensions to support more use cases.

[pylvain]

I was thinking of adding another delagated property that would be used just to check the existence of a component. Maybe i'll do another pr.

I don't think that's necessary, since you can use a read-only TagDelegate to achieve just that.

You can use TagDelegate to achieve that only if the Component has default value or no-arg constructor.

data class Location(x:Int,y:Int):Component
val Entity.hasLocation by tagFor<Location>() // throws an Exception since Location has no default value

I don't see a way around that using the tagFor() function, since the function cannot know if it will be assigned to a val or a var.

[czyzby]

I don't see a way around that using the tagFor() function, since the function cannot know if it will be assigned to a val or a var.

I have a couple ideas, lazy initiation should do the job. TagDelegate can store a () -> T reference that either returns a singleton or provides a new instance depending on what parameters were passed to tagFor.

[pylvain]

Updated Pr to make the functions tagFor and TagDelegate accept a provider function instead of a default value. Now tagFor works for readonly properties with no default constructor or value provided. It's some boilerplate code, but maybe i'll make a delagate that accept a Singleton instead of a provider ?

[czyzby]

It's some boilerplate code, but maybe i'll make a delagate that accept a Singleton instead of a provider ?

You can overload tagFor with an option that accepts a singleton and creates a provider for that. You can also convert TagDelegate to an interface and provide 2 separate implementations (singleton vs provider) - that's probably what I'd do to make it slightly simpler.

[pylvain]

Updated tagFor with Singleton/Provider Attributes I'm 100% happy with the behavious of this one, except one thing I cannot manage to solve by myself :

var Entity.isLocated by tagFor { Location(0,0) }  // Must specify <Location> after tagFor, overwise overload resolution ambiguity

That's so confusing for the user it must be changed. Obviously we can solve the problem with two functions singletonTagFor and providerTagFor but It would be better to have only one function tagFor. This goes beyond my competence however. Maybe you can figure it out ? I can try to write the tests after we're done with this pr if it's ok with you.

[czyzby]

This goes beyond my competence however. Maybe you can figure it out ?

I'll have a busy next few days, but I'll try to suggest something by the end of the week. I can also merge the PR then and update documentation (and so on), since it already looks pretty good.

I can try to write the tests after we're done with this pr if it's ok with you.

Let's keep this within a single PR. You can try to write the tests in the meanwhile, and I'll finish or refactor them if necessary. Just make sure that the pipeline passes. Check out this commit to see the changes that I've done to README, CHANGELOG and the tests after your previous PR. Thanks.

[pylvain]

Wrote some tests. I have little experience with that. Hope it will save you some time. I was thinking of creating a propertyWithDefaultFor in the same spirit of tagProperty, where a default value would be returned if the component doesn't exist supplied by a default value/singleton/provider/call to the no-arg constructor. I'll do that when the problem with tagFor is solved. I also think the functions tagFor and propertyFor should be the near the top for the readme file. IMO it's now the best way to interact with components, I think they deserve some visibility.

[czyzby]

I was thinking of creating a propertyWithDefaultFor in the same spirit of tagProperty, where a default value would be returned if the component doesn't exist supplied by a default value/singleton/provider/call to the no-arg constructor.

I think that's a bit error-prone, since you can accidentally create a component whilst making another check (such as entity.component != null). A default value might be nice for some use cases, but I'm a bit afraid it might be used over the optional property to get around null checks. The potential risks would have to be documented. You can propose another PR for this functionality if you want, but please checkout and select the develop when setting up the branches.

I also think the functions tagFor and propertyFor should be the near the top for the readme file. IMO it's now the best way to interact with components, I think they deserve some visibility.

Noted, I'll move them above custom mappers examples.

[czyzby]

Thanks for the PR. I'll merge it as is and adjust the tests to cover all use cases.