Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for @Wire superclass properties to subclasses to allow game/library setup #72

Closed
DaanVanYperen opened this issue Apr 21, 2014 · 11 comments

Comments

@DaanVanYperen
Copy link
Collaborator

Warming up for the Ludum Dare! 馃懐 Attempting to compile a library of components and systems commonly used in Jams. Map loading, Physics, animation rendering and the like. I'm running into some issues setting up a solid design, maybe you have some insight, since you're a lot more familiar with entity systems.

My main issue seems to be @Wired systems being so tightly integrated I am having trouble separating them into common library and game logic.

As a concrete example, assume I have a map system in my common library, that deals with map loading. During map loading it needs to instance entities, which is delegated to a second system.

Both very common systems in my games that I'd like to add them to a library, but I need to somehow be able to extend them with game specific code.

considered some options.

  • circumventing @Wire and just inject systems as needed via constructor, but that gets a bit messy.
  • extend this specific system to rely on json to assemble entities for example, but that wouldn't be very flexible.
  • Don't make a library and just copy the systems I need.
  • extend @Wire so it can wire a subclass to a superclass property, like you see in other DI packages. That way the library systems can refer to interfaces or base systems, and @Wire to game classes.
  • Ask junkdog for tips. XD

Any thoughts?

@junkdog
Copy link
Owner

junkdog commented Apr 22, 2014

Most of the time I copy the code over - like you've observed, they tend to want some custom code here and there.

What about making the systems abstract so that they contain most of what's required to make them work? Or am I misunderstanding?

@DaanVanYperen
Copy link
Collaborator Author

That's a solution I'd like, but isn't @Wire limited in that regard?

Concrete example:

In common library:

@Wire
class TiledMapSystem extends VoidEntitySystem {
   // no concrete factory available in library, game should provide.
   protected AbstractEntityFactory factory; 
}

In actual game:

class GameEntityFactory extends AbstractEntityFactory {
}

world.setSystem(new TiledMapSystem());
world.setSystem(new GameEntityFactory());

As far as I know @Wire can't resolve AbstractEntityFactory to GameEntityFactory right?

@junkdog
Copy link
Owner

junkdog commented Apr 25, 2014

I've reemerged(!) - my previous endeavor took much longer than expected due to lack of language familiarity; it did result in a minimal artemis c++ port however.

I can think of a few approaches to solving this, not sure anyone is elegant though:

  • Look up any subclasses of AbstractEntityFactory at runtime and wire automatically:
    • If more than one class extend AbstractEntityFactory, how do we know which one to wire?
    • It would require the base class to be of an applicable artemis type, if we stick with the current way of doing things.
    • Somehow need to figure out that AbstractEntityFactory should be injected. Probably needs to be explicit in the base class, which isn't that big of a deal if it's in a reusable system/manager.
  • Annotate the child class with an annotation like @WireAs(AbstractEntityFactory.class) or something.
    • The annotation would need to be resolved before actual injection takes place. A similar step would of course be required for the above solution too.
  • Do something similar to Guice's modules:
    • Not fond of it myself; feels like too much enterprisey configuration for something that should be resolved "automatically". In guice, there's no guarantee that the classloader has loaded all the appropriate classes - with artemis we know they're already loaded.

Hmm... I'm out of ideas. I think the 2nd alternative might be the cleanest and has the least amount of overhead. If you have a better idea (or just dislike the solution in general), I'm all ears.

@DaanVanYperen
Copy link
Collaborator Author

Look up any subclasses of AbstractEntityFactory at runtime and wire automatically:
If more than one class extend AbstractEntityFactory, how do we know which one to wire?

The user already has to explicitly register systems so they get the version they are interested in. I imagine @Wire would first look for exact matches, and if it fails, cycle through the registered Systems and hard-abort when there is more than one candidate.

Right now @Wire will hard-fail when it cannot find a suitable Manager, correct? So legacy users won't be affected.

It would require the base class to be of an applicable artemis type, if we stick with the current way of doing things.

That's fine.

Somehow need to figure out that AbstractEntityFactory should be injected. Probably needs to be explicit in the base class, which isn't that big of a deal if it's in a reusable system/manager.

I'd imagine AbstractEntityFactory would extend the artemis types, which qualifies it for @Wire

Annotate the child class with an annotation like @WireAs(AbstractEntityFactory.class) or something.

This would add the ability to have two subclasses of AbstractEntityFactory registered as system. Can't really tell if there are use-cases for that.

Edit: @wire must wonder why we keep talking about him. ;)

@junkdog
Copy link
Owner

junkdog commented Apr 25, 2014

Right now @wire will hard-fail when it cannot find a suitable Manager, correct? So legacy users won't be affected.

Correct. Also, there hasn't been an official release with @Wire in it yet, so I don't really mind changing things around if they're only present in SNAPSHOTs.

This would add the ability to have two subclasses of AbstractEntityFactory registered as system. Can't really tell if there are use-cases for that.

Probably not, though there might be. Can't think of any right now. Also, this might be cleaner since it doesn't introduce yet another annotation type.

I have some free time for the next couple of hours, I'll try to get the UuidEntityManager and this implemented before LD starts. You'd probably have to attend to GWT if it breaks though (I'll have some time tomorrow too, but just in case).

Edit: @Wire must wonder why we keep talking about him. ;)

s/he has become a crucial part of our development efforts!

@DaanVanYperen
Copy link
Collaborator Author

have some free time for the next couple of hours, I'll try to get the UuidEntityManager and this implemented before LD starts. You'd probably have to attend to GWT if it breaks though (I'll have some time tomorrow too, but just in case).

Yay! And also, eek! XD I've got a halfway decent library done, just keeping it all in one template https://github.com/DaanVanYperen/libgdx-artemis-quickstart to work around the tight integration for now.

Having this feature in the future will be great though! As long as it uses the reflection helper class and introduces no argumented annotations GWT should be fine.

You should jam with us, promote your framework a bit! ;)

@junkdog
Copy link
Owner

junkdog commented Apr 25, 2014

I've been on a social hiatus while coding for the past 4w, so I'm going to try to catch up on that this weekend. I'll def partake in the next LD though.

@junkdog
Copy link
Owner

junkdog commented Apr 25, 2014

Looks like I won't have time to finish the improved @Wire support today; have to leave in a little bit. I'll finish it tomorrow though.

@junkdog
Copy link
Owner

junkdog commented Apr 25, 2014

Ok, during the last minute! Hope it works.

Edit: And snapshot deployed to maven;s repo,

@DaanVanYperen
Copy link
Collaborator Author

You are making this Ludum Dare exciting, I better figure out how to refer to a specific snapshot before tomorrow if things break! ;)

@DaanVanYperen
Copy link
Collaborator Author

Works, close when you're ready!

@junkdog junkdog closed this as completed May 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants