TheBevyFlock / bevy_cli

A Bevy CLI tool.
Apache License 2.0
19 stars 4 forks source link

Lint suggestion: deny `Update` or `FixedUpdate` #99

Open alice-i-cecile opened 5 days ago

alice-i-cecile commented 5 days ago

As discussed in https://github.com/bevyengine/bevy/issues/14215 and https://github.com/bevyengine/bevy/pull/14911, some games will prefer to run their logic in Update, while others prefer to run it in FixedUpdate.

We should have a pair of style lints that let you pick which one you're using. Note that UI code generally belongs in Update, so for UI modules projects should have localized allow / denies to swap this!

BD103 commented 5 days ago

Totally agree! The documentation for this lint pair should recommend enabling them for just certain modules, not the entire crate, since there are reasons to use both.

janhohenheim commented 4 days ago

I'd love this lint! Subtle gotcha: when working with fixed update, the lint logic must be reverse for camera code. That should still go into Update and not FixedUpdate. A lint implementation should respect this, otherwise it would actively mislead users.

BD103 commented 4 days ago

Subtle gotcha: when working with fixed update, the lint logic must be reverse for camera code. That should still go into Update and not FixedUpdate.

What should the lints check for to find "camera code"? Should it check for references to camera components, or should it just document what schedules should handle what?

alice-i-cecile commented 4 days ago

I'd prefer just documentation, and add a seperate lint for camera code in fixed update under correctness.

janhohenheim commented 4 days ago

Sounds good, I'd just be very weary of adding a FixedUpdate lint without adding the camera lint at the same time, as putting your camera code in FixedUpdate is already a common mistake that we shouldn't accidentally encourage through a lint 😅