Support for hashed block network ID's #97
Conversation
index.js
Outdated
| function getHashValue (states, name) { | ||
| const tag = nbt.comp({ | ||
| name: { type: 'string', value: name }, | ||
| states: nbt.comp(states) | ||
| }) | ||
| const s = nbt.writeUncompressed(tag, 'little').toString() | ||
| return fnv1a32(s) | ||
| } |
There was a problem hiding this comment.
As a static method:
static getHash (name, states) {
if (registry.supportFeature('blockHashes')) {
return ... compute hash ...
}
}inside the Block instance constructor:
if (registry.supportFeature('blockHashes')) {
this.hash = Block.getHash(this.name, this._properties)
}There was a problem hiding this comment.
Where should first edit be made?
There was a problem hiding this comment.
I think i get it now, ill try
There was a problem hiding this comment.
As a static method:
static getHash (name, states) { if (registry.supportFeature('blockHashes')) { return ... compute hash ... } }inside the Block instance constructor:
if (registry.supportFeature('blockHashes')) { this.hash = Block.getHash(this.name, this._properties) }
Should supportFeature be used as that "variable" that sets on start_game? Or other variable is still needed and supportFeature("blockHashes") will be true for mcbe >1.19.80?
There was a problem hiding this comment.
No, supportFeatures data should be inside minecraft-data features.json. It is used for handling features available in different minecraft versions
index.js
Outdated
| @@ -414,5 +414,5 @@ function computeFnv1a32Hash (buf) { | |||
| h ^= buf[i] & 0xff | |||
| h += (h << 1) + (h << 4) + (h << 7) + (h << 8) + (h << 24) | |||
| } | |||
| return h | |||
| return h & h | |||
There was a problem hiding this comment.
Hash only matched for that one block tested, other ones I tested didnt, not sure why this works either, I changed the test to block that didnt match. Also checked hashing online "hello" and with h & h it matched
There was a problem hiding this comment.
If you're not sure then please revert. The implication here is that that has a substantive effect and if it does it's not obvious to me (bitwise AND on two numbers yields the number itself, it does not turn signed to unsigned or anything like that). The only effect I can think would be to turn a floating point into an integer, but getting a float is not possible in the operation here.
If it breaks without it then that needs some investigation because it doesn't seem to make sense.
There was a problem hiding this comment.
If you're not sure then please revert. The implication here is that that has a substantive effect and if it does it's not obvious to me (bitwise AND on two numbers yields the number itself, it does not turn signed to unsigned or anything like that). The only effect I can think would be to turn a floating point into an integer, but getting a float is not possible in the operation here.
If it breaks without it then that needs some investigation because it doesn't seem to make sense.
I tested with my bot now and just returning "h" only works for some blocks, others hash collide
There was a problem hiding this comment.
Which ones? Can you be more specific with the inputs to trigger?
There was a problem hiding this comment.
The one block I changed in tests fails without "h & h", could be used as an example (i think its soul_soil or other soul_...)
There was a problem hiding this comment.
Please add tests that are failing or that show the collision. This duplication comment doesn't make any sense.
There was a problem hiding this comment.
I have added new tests that illustrate difference in outputted numbers
There was a problem hiding this comment.
Ah, this is because of dumb JavaScript behavior. The hash is exceeding the 32 bit boundary and because JS turns all Numbers into 32 bit integers when doing bitwise operations, h & h was effectively truncating the bits. This is very unclear so it should be moved to explicitly mask the number to 0xFF FF FF FF
|
@extremeheat do you think this is ready? |
…te-hashes # Conflicts: # test/basic.test.js
|
Please update the doc |
|
@extremeheat I think the whole pack is ready, let me know if any changes required |
|
I meant update with what you're exposing in the PR |
I think I did, whatever is in index.d.ts now is in docs + some extra that wasnt documented/had it outdated |
|
Ok I have forgot getHash() and hash fields, indeed |
|
Should be fine now @extremeheat |
Aims to add support for hashed block network ids.
WIP so more commits to follow