Hook for default base Aspect-mapping on all systems

[DaanVanYperen]

See below.

[junkdog]

We could create a hook for default base Aspect-mapping for all systems; holding it off until post-1.0, but I agree it'd be a better fit for contrib.

Something like:

Aspect.default().exclude(Disabled.class)

[DaanVanYperen]

oh me like! might be a good fit for core really.

[junkdog]

Aspect.default().exclude(Disabled.class)

The above would not play nice with multi-World configurations. Better to set the default aspect on the WorldConfiguration instead. But how to deal with aspects with and without the default aspect? Must be exposed by the AspectSubscriptionManager.

[junkdog]

ice-boxing

[schosin]

I'd be interested in this feature as well as contributing. My use case is exaclty the the given example.
Before I found this issue, my approach would have been to add the default components to all/one/exclude unless that component is already part of the given aspect.

Examples:

• default: Aspect.exclude(Disabled.class)
  system: Aspect.exclude(CompA.class)
  result: Aspect.exclude({Disabled.class, CompA.class})
• default: Aspect.exclude(Disabled.class)
  system: Aspect.all(Disabled.class)
  result: Aspect.all(Disabled.class)
• default: Aspect.exclude({Disabled.class, CompA.class})
  system: Aspect.all(Disabled.class)
  result: Aspect.exclude(CompA.class).all(Disabled.class).

I would also prefer defining the default aspect in the world configuration. For aspects with/without applying the default aspect, how about something like Aspect.Builder#defaults(boolean disabled), @AspectDescriptor(defaults = true|false), and something like @All(value = {...}, defaults = true|false) or a new annotation alltogether?

Greetings,
schosin

Edit: Ignore first commit below, pushed too early, did too much.

[schosin]

I've been working on this on my fork, and I had to touch more places than I anticipated.

Here's an overview what I've done:

• Default aspect is configured via WorldConfiguration(Builder), by default applied to all subscriptions
• Aspect.Builder
  • has a flag whether to apply defaults or not
  • has a couple of fields changed to package private for easier access in asm (not too happy about this)
• AspectSubscriptionManager
  • has optional field for a default aspect, set by WorldConfiguration
  • applies the default aspect according to the parameter and builder
• EntityTransmuter explicitly ignores default aspect regardless of the Aspect.Builder
• WorldConfiguration/Builder have optional default aspect
• AspectDescriptor: flag defaults() default true
• Exclude: flag excludeDefaults() default false (not sure this is the best way for All/One/Exclude)
• AspectFieldResolver updated for AspectDescriptor/Exclude
• Updated / added a bunch of tests

This should result in no runtime changes for existing applications and a slight overhead for creations of EntitySubscriptions.
I have only worked in the artemis-core/artemis module. All tests are passing, but I'm not certain whether I forgot anything (plugins, serialization, fluid, gwt, ...).

I'm currently using those changes in a project of mine and I would open a PR depending on feedback and how my tests go.

[junkdog]

Interesting. Sorry for taking too long to respond.

This should result in no runtime changes for existing applications and a slight overhead for creations of EntitySubscriptions.

How much additional overhead are we talking about here? Does it show in the benchmarks?

[schosin]

Haven't run any benchmarks, that statement was solely based on the fact that all the work is done when creating a EntitySubscription.
The biggest impact might be in BaseEntitySystem#getSubscription, as that method always computes the aspect instead of using the field when already initialized. Was there any reason for doing that?