junkdog / artemis-odb

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

Request: Support nested @Wire on superclass. #162

Closed pakoito closed 8 years ago

pakoito commented 9 years ago

I have found that the functionality of Wire and Mapper is not available for my system, and I receive a NPE when trying to access the Component mapper that was supposed to be injected.

Line 30

https://github.com/pakoito/CardGameFramework/blob/master/cgfw-core/src/main/java/com/pacoworks/cardframework/systems/GameSystem.java

The world is created at lines 32+

https://github.com/pakoito/CardGameFramework/blob/master/cgfw-core/src/main/java/com/pacoworks/cardframework/framework/CardgameFramework.java

Does it have to do with the system being abstract and annotations not working on inheritance?

junkdog commented 9 years ago

Missed the issue tracker, but as I said on the mailinglist - @Wire(injectInherited=true) should work.

You shouldn't use @Mapper though, it's deprecated and will be removed in the coming months, as artemis bumps up to 1.0.0

pakoito commented 9 years ago

It does work, but it's not very friendly when using it to create inheritance-style APIs, I had to change it to composition with an extra interface.

junkdog commented 9 years ago

Which interface?

PS. Only really applies when you're having performance issues, but accessing fields from superclasses is noticeably slower than getting it from the this class - can compound to semi-high numbers when dealing with a lot of entities grabbing the component mapper each time.

pakoito commented 9 years ago

Instead of having those two overrideable methods I packed them as interface methods to be passed to the class in the constructor, thus making it non-abstract.

I understand the issue, I saw it in Jackson and ORM-lite before. Just wondering if there was a chance for this to be saved.

DaanVanYperen commented 9 years ago

@Junkdog think we discussed this before, for API's a quick hierarchy scan for @Wire on class is a desirable feature. Think you concluded it wasn't too much of an inconvenience to just @Wire the subclasses.

junkdog commented 9 years ago

Yeah, if all classes needed to be scanned for potential occurances of @Wire, it could cause some undesirable perf and GC characteristics on lesser hardware (droids and whatnot).

pakoito commented 9 years ago

I would make it configurable and leave it at programmer's discretion with a big WARN sign on perf issue.

My working field is Android and perf is an issue, but flexibility is nice too.

junkdog commented 9 years ago

But I'm like most programmers - we can't be trusted! (Also, I can't be trusted to find a new tweet to flaunt).

But in your particular case, isn't it just a matter of @Wire-ing the concrete classes, or am I misunderstanding?

edit: Ah, I see you use lombok! One of the best java hacks around!

pakoito commented 9 years ago

I'm from the C++ school of API design where we like to take our toasters to a bath in case we want to make poptarts :D

As long as the default state is NOT to use it and you hide the docu enough, everyone should be safe. Google is an expert on that.

DaanVanYperen commented 9 years ago

Is reflection really that horrible on android? Say you have to reflect 250 classes for annotations. (50 systems each with 5 superclasses).

Plus no need to scan stock classes.

junkdog commented 9 years ago

I'm from the C++ school of API design where we like to take our toasters to a bath in case we want to make poptarts :D

"C++ vs. C: While the chainsaws you're juggling are no longer on fire, you are now also juggling two poisonous daggers and an angry badger", finally a new tweet to quote!

Is reflection really that horrible on android? Say you have to reflect 250 classes for annotations. (50 systems each with 5 superclasses).

What kind of sick hierarchy are you proposing!!!???

I'll reevaluate when I've setup a bench harness for android, but for the time being it stays.

DaanVanYperen commented 9 years ago

What kind of sick hierarchy are you proposing!!!???

Extreme case! 50 times DelayedCooldownImprovedCustomEntityProcessManager.

DaanVanYperen commented 9 years ago

Euh, did I misclick and delete your post about documentation? XD

pakoito commented 9 years ago

I did delete it because I'm an idiot and forgot about the wiki, which I have visited before but completely forgot about. Nice stuff there, I'm already thinking about Wiring stuff I shouldn't into every system.

If my game action class contains the world, the world contains the systems and the systems contain the game class, I'm leaking like a champ, right? :P I need to create a centralized bus for communication oh wait I already have.

DaanVanYperen commented 9 years ago

We're no strangers to self answering questions here! I just spam the issues myself, but @junkdog wants me to talk more to rubber ducks and other stuffed animals instead. It's actually kinda scary.

pakoito commented 9 years ago

My coworkers think I'm a wacko for talking to myself while coding. At home what I'm doing is livestreaming myself and trying to explain what's on screen for the couple of bros who turned up.

I'll spam the google group every now and then, then.

junkdog commented 9 years ago

Answering the deleted comment, about all the new stuff: most of it happens behind the scenes - primarily to make transitioning from vanilla artemis as easy as possible. It's only recently - meaning that which will becomes 0.7.0 - that we've started deprecating like crazy.

There's a lot of goodies to be had from maven or the cli tool however.

@junkdog wants me to talk more to rubber ducks

We're all animals! :hatched_chick:

At home what I'm doing is livestreaming myself and trying to explain what's on screen.

Where can we watch?

pakoito commented 9 years ago

www.twitch.tv/mr_pakoito

I prototype fast stuff every odd weekend a month, starting from last Ludum Dare. Feels nice after a year with no pet projects :+1:

junkdog commented 9 years ago

Sweet! Currently experiencing some problems with my ISP, so I usually resort to downloading podcasts etc, but will definitely keep my eye on you ;)

On Sun, Sep 21, 2014 at 12:33 AM, pakoito notifications@github.com wrote:

www.twitch.tv/mr_pakoito

I prototype fast stuff every odd weekend a month.

— Reply to this email directly or view it on GitHub https://github.com/junkdog/artemis-odb/issues/162#issuecomment-56282493.

DaanVanYperen commented 8 years ago

Grooming issue. out of scope or in need of sponsor. Revisit later.

DaanVanYperen commented 8 years ago

Implemented ,@Wire is now default.

pakoito commented 8 years ago

Neato, thanks. I need to come back to libgdx and artemis one of these days. Too busy with work.