junkdog / artemis-odb

A continuation of the popular Artemis ECS framework
BSD 2-Clause "Simplified" License
779 stars 112 forks source link

Add AllDerived, ExcludeDerived and OneDerived #519

Closed GMWolf closed 5 years ago

GMWolf commented 6 years ago

It would be great to have AllDerived, EcludeDerived and oneDerived methods in the Aspect.Builder class, which would add all derived components.

This is a functionality I have found myself needing more than once:

Say you have a group of components that are all related somehow (e.g. Buffs an entity are currently affected by) and you want to have a system iterate over all entities without any of these components (Say you want to give a buff to any entity that does not already have a buff). Traditionally you would need your buff-giving system to know about all buffs. That can become problematic once you start adding a lot of different buffs; It defeats the point of an ECS.

Instead, It would be great to have all BuffComponent extend a Buff interface, and then be able to call Aspect.excludeDerived(BuffComponentInterface.class).

All these functions would do (allDerived, excludeDerived and oneDerived) is use reflection to find derived classes, and then iteratively call the usual all, exclude and one functions. Maintenace should be minimal because of that.

junkdog commented 6 years ago

It's a bit too specialized for my tastes, though I like the idea. Never had the exact same use-case myself, but plenty that bear resemblance.

Aspect.Builder/Aspect is in need of some refactoring, but what about adding overloaded all/one/exclude accepting a single-method interface producing the component classes? For example:

// temp names all round
public interface AspectBuilderProvider {
    Iterable<Class<? extends Component>> componentsToAspect();
}

This way, you could have a public class BuffTypes implements AspectBuilderProvider and pass it to Aspect.Builder::all/one/exclude (well, probably not all in this case, but you get the idea).

All these functions would do (allDerived, excludeDerived and oneDerived) is use reflection to find derived classes, and then iteratively call the usual all, exclude and one functions. Maintenace should be minimal because of that.

I started replying with a much more convoluted solution until I read the last paragraph ;)

junkdog commented 6 years ago

Btw, I when I need more flexible Aspects, I usually keep a private static method in the system requiring it (or outside, if appropriate):

public RenderSystem(SubRenderer... renderers) {
    //noinspection unchecked
    super(wrap(renderers, // wrap = private static method 
        all(
            Position.class,
            Size.class,
            Layer.class)
        .one(
            ChildRenderable.class)
        .exclude(
            Culled.class)));

wrap() resolves and adds component types to the aspect's one, based on what the SubRenderers require.

GMWolf commented 6 years ago

Oohh I actually really like the idea of an aspect builder provider! That could lead to some really nice code. For instance you could build a provider to return every component an entity has, that way you can look for entities that have all the same components, etc...

It would easily achieve the task I pointed out, all while being far more general. I like it!

The wrap system is nice way to do that, but not super elegant...

junkdog commented 6 years ago

:+1:

I'll add a task for it; ping me if it's not done during the weekend. I'll need find some better names though.

For instance you could build a provider to return every component an entity has,

Neat, I never thought of adding some sort of select all-entities-of-same-type.

The wrap system is nice way to do that, but not super elegant...

Agreed. It's the poor man's extension methods :)

GMWolf commented 6 years ago

If you don't get the time I would be more than happy to give it a shot.

junkdog commented 6 years ago

Please do :)

GMWolf commented 6 years ago

I implemented the feature: https://github.com/GMWolf/artemis-odb/tree/519---AspectBuilderProvider

I named the new interface ComponentClassProvider. I would have called it ComponentTypeProvider but ComponentTypes are already a thing...

Should a file a pull request or are there changes to be made?

not sure what the contribution protocol is here...

junkdog commented 6 years ago

Sweet! I'll have a look at it tomorrow (had a busy weekend - time management is something I've heard of, not sure what it implies).

not sure what the contribution protocol is here...

Just create a pull request, no special protocols apply ;) I created the pull request for you: https://github.com/junkdog/artemis-odb/pull/521

OT: not the thesis I had in mind, but I guess I maybe misremember the choice of language: http://ics.upjs.sk/~krajci/skola/ine/SVK/pdf/Majerech.pdf - I also found https://github.com/SuperV1234/bcs_thesis, but I haven't had a chance to look at it yet.

DaanVanYperen commented 5 years ago

It's been quite some time! Sorry for the delay. @GMWolf wondering how (if) you ended up implementing a workaround? See PR for some comments.

If we provide something like Aspect.all(subclassesOf(Buff.class)) it would probably add the most value towards this ticket, though I agree with @junkdog it feels a bit out of scope.

I'm all for letting the users choose how to implement their model. Wonder if we should actively stimulate users to create large hierarchies for components, or if we should perhaps document other common solutions for the one-to-many/many-to-many problem in ECS.

Like entity-per-relationship:

class Buff extends Component {
      @EntityId int targetId;
      Effect[] effect;
}
DaanVanYperen commented 5 years ago

Closing because of stale ticket and PR. Feel free to post a new ticket if still relevant.