libgdx / ashley

A Java entity system inspired by Ash & Artemis.
Apache License 2.0
875 stars 144 forks source link

Support for @NotNull annotation #277

Open dwursteisen opened 5 years ago

dsaltares commented 5 years ago

Hi @dwursteisen!

Sorry it took me so long to take a look. To be honest, I am not familiar with the @notNull annotation as I haven't touched Java in a LONG time 😅.

Does this simply state that the methods will not work with null values? I guess therefore it helps IDEs tell you when you could potentially be calling these methods with invalid parameters...?

dwursteisen commented 5 years ago

I guess therefore it helps IDEs tell you when you could potentially be calling these methods with invalid parameters...?

yes. I added it so when overriding a method from iterateSystem in Kotlin, for example, it will mark the parameter as non nullable, so you don't have to manage the null case, that can't happen.

I'll fix the conflict and add you more information, so you can merge it easily next week.

dwursteisen commented 5 years ago

Hello again!

Thanks for your patience.

I updated the pull request:

Please see bellow what means the change in IntelliJ:

Without @NotNull while setting a family with a null value:

Capture d’écran 2019-08-30 à 10 58 20

With @NotNull while setting a family with a null value:

Capture d’écran 2019-08-30 à 10 59 39

Please see the red wave

Without @NotNull while creating a new IteratingSystem:

Capture d’écran 2019-08-30 à 11 01 48

With @NotNull while creating a new IteratingSystem:

Capture d’écran 2019-08-30 à 11 01 26

Please see the Entity instead of Entity? (which mean that the entity argument can be null in this later case)

metaphore commented 4 years ago

As of now, there's an open discussion in the main LibGDX repo about which Nullable annotation to use.

Currently, there's some refactoring is done in https://github.com/libgdx/libgdx/pull/6256 utilizing the javax.annotation.Nullable from com.google.code.findbugs:jsr305 and most likely that's gonna go to the master branch soon.

I guess Ashley should follow the same approach and thus this PR might need to be migrated to the matching annotation.