Objective

The type AssetLoadError has PartialEq and Eq impls, which is problematic due to the fact that the AssetLoaderError and AddAsyncError variants lie in their impls: they will return true for any Box<dyn Error> with the same TypeId, even if the actual value is different. This can lead to subtle bugs if a user relies on the equality comparison to ensure that two values are equal.

The same is true for DependencyLoadState, RecursiveDependencyLoadState.

More generally, it is an anti-pattern for large error types involving dynamic dispatch, such as AssetLoadError, to have equality comparisons. Directly comparing two errors for equality is usually not desired -- if some logic needs to branch based on the value of an error, it is usually more correct to check for specific variants and inspect their fields.

As far as I can tell, the only reason these errors have equality comparisons is because the LoadState enum wraps AssetLoadError for its Failed variant. This equality comparison is only used to check for == LoadState::Loaded, which we can easily replace with an is_loaded method.

Solution

Remove the {Partial}Eq impls from LoadState, which also allows us to remove it from the error types.

Adopted from #14428 by @JoJoJet.

Migration Guide

The types bevy_asset::AssetLoadError and bevy_asset::LoadState no longer support equality comparisons. If you need to check for an asset's load state, consider checking for a specific variant using LoadState::is_loaded or the matches! macro. Similarly, consider using the matches! macro to check for specific variants of the AssetLoadError type if you need to inspect the value of an asset load error in your code.

DependencyLoadState and RecursiveDependencyLoadState are not released yet, so no migration needed,