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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENHANCEMENT] Implement null safety #3267

Open
EliteMasterEric opened this issue Oct 15, 2024 · 2 comments
Open

[ENHANCEMENT] Implement null safety #3267

EliteMasterEric opened this issue Oct 15, 2024 · 2 comments

Comments

@EliteMasterEric
Copy link
Contributor

EliteMasterEric commented Oct 15, 2024

Null safety is an optional feature of the Haxe compiler which enforces compile-time checking of nullable values.

By implementing null safety throughout HaxeFlixel, we can greatly reduce the frequency of crashes and other errors. This can be done using the @:nullSafety annotation at the top of each class, and can be done incrementally until every class in the library is compliant.

This would not break existing code which has null safety enabled, but it would:

  • Improve documentation by informing users which variables are Null-able (such as FlxG.sound.music).
  • Reduce the incidence of certain crashes within the engine by guaranteeing they are caught and handled.
  • Improve the compatibility with other code where null safety is enabled.

As an example of the last case, I have started to enable null safety in Funkin', and have had to create blocks of code where null safety is not enabled in several places, simply because Flixel itself regularly neglects to use Null<T> for nullable values (again, trying to set FlxG.sound.music = null makes the compiler complain until you specifically exclude that line).

One major obstacle to implementing null safety is that null safety is not necessarily finalized (see this issue or any of these issues). Overall though, I would like to see it become a long-term goal of the engine, especially since it can be done in a non-breaking manner in most cases.

@Geokureli
Copy link
Member

Geokureli commented Oct 17, 2024

I've been considering null safety when adding new features and modifying existing ones, and I'm all for it. For stuff like this I tend to prefer a bunch of specific changes rather than a giant sweeping change across the entire repo, as it's usually much easier to test.

For instance, your recommended change to SoundFrontEnd would be great, along with anything else in that class. In general whenever I think about null safety, I first see if I can make it so that the value is never null, rather than nulling out values in destroy, can they instead be a final var that gets cleared. I've also trying to reduce nullable fields by using enums, where the field is only defined in the cases that use it. I think there's many more cases that can be improved using these methods.

There are cases that really baffle me, though. For instance FlxG.state, theres definitely a period in every flixel game where this can be null, but the idea of null checking this every time you use it, or doing FlxG.state.sure() every time, seems like an absolute pain especially since it's very easy to avoid null refs with this particular field.

Null-safety is something enabled per-module, right? We could make a checklist of modules that are already null-safe or modules that can easily become null-safe and enable/finalize them

@EliteMasterEric
Copy link
Contributor Author

EliteMasterEric commented Oct 17, 2024

Null-safety is something enabled per-module, right? We could make a checklist of modules that are already null-safe or modules that can easily become null-safe and enable/finalize them

Correct! You can enforce null safety on a class by annotating the class itself with @:nullSafety at the top of a class in a module, like so:

https://try.haxe.org/#C74a0800

Note there are additional modes for this, they mainly make the code more strict to handle edge cases (such as a variable becoming null after a check, or being set by another thread), but the default "Loose" mode is more than enough to handle things.

Note that if there are specific places you need to turn it off internally, you can annotate specific blocks (see the above example again).

Null safety will definitely need to be enabled in stages, since enabling it on a large class would cascade out to other classes. We can also look into refactoring chunks of code in order to make it more null-safe (such as making a non-nullable property which fetches an internal nullable one).

Null safety doesn't "cascade" out though, so as long as the changes you make to force variables to be non-null don't break existing apps, it should be fine to just add to classes one at a time.

I wonder if there's an easy way to generate a module dependency tree...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants