Place block refactor #1686
Place block refactor #1686
Conversation
std::hash is problematic in gcc / clang, one has a class, the other a struct.
|
Does this fix #1567 ? |
|
I haven't checked, it may indeed fix that issue as well. |
|
Yes, this also fixes #1567 |
|
Did you update the documentation? Otherwise I'd say merge :) |
|
You're right, I didn't. I'll do that. |
The hooks now have fewer parameters but are called on all player-placed blocks (#1618).
|
I think the problem with std::hash was that operator() needs to be const. I'm fixing that now. |
|
@worktycho check the gcc and clang failures. gcc requires the std::hash specialization to be a struct, while clang requires it to be a class and MSVC doesn't care. Rather than have a mess of preprocessor macros, I think I prefer the explicit hash type. |
|
Okay. That's weird as they should both be using the same include files. Merge this if you want but I'm going to take a closer look at this because something in the build is not doing what its supposed to. |
|
Found it. If you're going to specialise std::hash, make sure you include the original definition. For some reason clang is getting confused by the friend definition in vector. |
|
Still nope :P |
|
I though I had the same libstdc++ as travis. However, it looks like the ubuntu 14.04 release has backported the change that fixed the Immediate issue with struct/class. The commit that fixed it says its just a warning issue so I'm currently investigating the c++ standard to see whether there is a bug in clang or libstdc++. |
worktycho
force-pushed
the
PlaceBlockRefactor
branch
from
fc327b1
to
5609d76
Compare
|
Well, bug or no bug, we want this change to get in. I'll revert your changes, if you don't mind, and we can later investigate this further. Deal? |
|
I've already reverted. Its a bug in libstdc++. The problem is not that the definition differs but that std::hash is declared as a struct and a class within libstdc++ prior to 4.9. gcc ignores the difference but clang doesn't. I'm writing this up because we may hit it again if we include the functional and vector headers in the same translation unit we have the same problem. |
|
You destroyed skulls :( |
|
Hm, I thought I checked them |
|
Oh, also you destroyed doors and 2 block high plants. |
|
Yes, I have tested doors and plants, they both worked. What client are you using? |
|
Ech, those were some client-side phantasmagoria again. Fixing now. |
|
Fixed in master |
|
Great! |
|
Mob heads are still broken. Working on that... |
This changes the player-caused block placing to go through the hooks for each block placed, even for multi-block changes (bed, door, golems, ...)