Update hashbrown to 0.15

[clarfonthey]

Updating dependencies; adopted version of #15696. (Supercedes #15696.)

Long answer: hashbrown is no longer using ahash by default, meaning that we can't use the default-hasher methods with ahasher. So, we have to use the longer-winded versions instead. This takes the opportunity to also switch our default hasher as well, but without actually enabling the default-hasher feature for hashbrown, meaning that we'll be able to change our hasher more easily at the cost of all of these method calls being obnoxious forever.

One large change from 0.15 is that insert_unique_unchecked is now unsafe, and for cases where unsafe code was denied at the crate level, I replaced it with insert.

Migration Guide

bevy_utils has updated its version of hashbrown to 0.15 and now defaults to foldhash instead of ahash. This means that if you've hard-coded your hasher to bevy_utils::AHasher or separately used the ahash crate in your code, you may need to switch to foldhash to ensure that everything works like it does in Bevy.

[mnmaita]

@clarfonthey looks like there's a formatting issue somewhere, could you try running the formatter and push again?

[clarfonthey]

Yeah, I was running into some difficulties fixing the lints but will try and fix this today or tomorrow.

[clarfonthey]

Rereading the changelog, I misunderstood what hashbrown did and they actually switched to a better hash that we should probably use instead. Gonna probably find a way to factor that in.

[clarfonthey]

I'm adding this comment to appease the lint, but I actually don't know if we do know if the ID is unique.

[mnmaita]

I´ve checked the places where Bundles::init_dynamic_info was being used and most of them are internal facing, but I found EntityWorldMut::insert_by_ids, EntityWorldMut::retain and EntityWorldMut::clear which might be public facing I think?

HashMap::insert_unique_unchecked says:

    /// This operation is safe if a key does not exist in the map.
    ///
    /// However, if a key exists in the map already, the behavior is unspecified:
    /// this operation may panic, loop forever, or any following operation with the map
    /// may panic, loop forever or return arbitrary result.

So I'm unsure if we should say something like the caller ensures the uniqueness, and even add some notes in the EntityWorldMut methods if applicable? I guess I'm thinking out loud. 😅

[clarfonthey]

So, I'm honestly fine just changing this to insert and leaving it at that :p

[clarfonthey]

Okay, so, I double-checked and the bundle ID is always increasing here, and without concurrency, we don't need to worry about data races. So, I updated the comment to clarify it is actually unique and leaving it.

[clarfonthey]

This should be ready for review, modulo whatever tiny nitpicks CI throws at me.

Not sure if the migration guide is adequate: not sure how much we care about bevy_utils' public API, as far as like, whether people should be using it or not.

[hymm]

What is better about foldhash vs ahash? Is it really worth taking the extra dependency?

[clarfonthey]

Final changes made:

• Update "ID is unique" comment to actually clarify why ID is unique
• Normalise let x: HashMap<_, _> = HashMap:: to let x = <HashMap<_, _>>:: and similarly for HashSet, to make things less verbose

[clarfonthey]

(If someone could mark this as ready for final review, I'd appreciate it. I think this should qualify as ready.)

[clarfonthey]

(I'll try and take a look at the failures soon)

[clarfonthey]

Ah, fun: the singular PR that got in the queue before this one didn't result in any merge conflicts, but it did add more uses of insert_unique_unchecked that now need unsafe blocks :(

Those have to be removed now, since bevy_reflect has deny(unsafe_code).

[clarfonthey]

(Merge conflict should be resolved; ready to merge now.)

[BenjaminBrienen]

[clarfonthey]

(kind of shocked that ci compile didn't catch that, tbh)

[clarfonthey]

(Okay, this time I waited until CI passed. Should be ready to merge now. <3)

[alice-i-cecile]

Thanks for shepherding this across the finish line <3

[clarfonthey]

So, I had no idea about these patches that were being run in CI, but they seem pretty brittle-- it would be much better (IMHO) to use #[cfg] or cfg! to enable/disable these features than use patches, IMHO, so that at least they would be visible to people searching through the code. That said, I'll try and see what I can do.

[clarfonthey]

I also have genuinely no idea why that patch failed on this PR. It appears to be referencing an earlier change where the patch should have broken, but it somehow did not trigger until now. And there were no recent changes to the CI job, either. It just makes absolutely no sense why this PR would be the one to trigger it, but at least, it did, and I fixed it.