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

Reflect::hash collides with Hash::hash #943

Closed
CleanCut opened this issue Nov 28, 2020 · 1 comment · Fixed by #954
Closed

Reflect::hash collides with Hash::hash #943

CleanCut opened this issue Nov 28, 2020 · 1 comment · Fixed by #954
Labels
C-Code-Quality A section of code that is hard to understand or change

Comments

@CleanCut
Copy link
Member

With the merging of @cart's #926, the hash method of bevy::prelude::Reflect now collides with the hash method from std::hash::Hash. You can workaround by disambiguating, so this isn't a showstopper. I'm reporting it in the hopes that there's an easy tweak to avoid having to do the workaround. If not, no big deal - and I'm fine closing this.

With the following imports:

use bevy::prelude::*;
use std::hash::Hash;

When you attempt to call the hash method on a primitive value like an integer (which is something you might want to do within an implementation of the Hash trait for a struct, such as on line 59 here -- PlayerID is a newtype over usize):

https://github.com/CleanCut/punchball/blob/9f8ddb2de2b8a91629307ee81b3f64aaa1eed1de/src/player/collision.rs#L52-L62

You get this error:

error[E0034]: multiple applicable items in scope
  --> src/player/collision.rs:58:23
   |
58 |             player_id.hash(state);
   |                       ^^^^ multiple `hash` found
   |
   = note: candidate #1 is defined in an impl of the trait `Hash` for the type `usize`
   = note: candidate #2 is defined in an impl of the trait `bevy::prelude::Reflect` for the type `usize`
help: disambiguate the associated function for candidate #1
   |
58 |             Hash::hash(&player_id, state);
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: disambiguate the associated function for candidate #2
   |
58 |             bevy::prelude::Reflect::hash(&player_id, state);
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: aborting due to previous error

For more information about this error, try `rustc --explain E0034`.

The workaround is to change player_id.hash(state); to Hash::hash(&player_id, state); to disambiguate.

https://github.com/CleanCut/punchball/blob/a6abd16d239fb7c6ea2656990c6da16056d4ffce/src/player/collision.rs#L52-L62

@cart
Copy link
Member

cart commented Nov 28, 2020

Hmm yeah thats a bit of a papercut. I think its worth resolving the conflict on the Reflect side. I'm thinking we should probably just rename each reflect "trait impl" method to something like reflect_hash().

@Moxinilian Moxinilian added the C-Code-Quality A section of code that is hard to understand or change label Nov 28, 2020
@cart cart closed this as completed in #954 Dec 1, 2020
cart pushed a commit that referenced this issue Dec 1, 2020
…eflect_partial_eq` (#954)

* Rename reflect 'hash' method to 'reflect_hash' to avoid colliding with std::hash::Hash::hash to resolve #943.

* Rename partial_eq to reflect_partial_eq to avoid collisions with implementations of PartialEq on primitives.
@fopsdev fopsdev mentioned this issue Jan 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Code-Quality A section of code that is hard to understand or change
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants