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

Fix parsing longs and strings #2

Merged
merged 3 commits into from
Mar 16, 2014
Merged

Conversation

deathcap
Copy link
Contributor

Includes several fixes:

with jdataview 64-bit integers, split into two 32-bits:

"firstPlayed": {
  "lo": 2016155682,
  "hi": 318
},

which matches the value compared with NBTExplorer (318*Math.pow(2,32) + 2016155682 = 1367815755810):

screen shot 2014-03-15 at 3 30 33 pm

  • Parses strings as UTF-8 instead of 'binary':

screen shot 2014-03-15 at 4 30 52 pm

parsed correctly as '§eElectric' (raw bytes: \xc2\xa7 utf8, decode as U+00A7). Before this would decode as '§eElectric'


P.S. @maxogden: any interest in merging this module with @sjmulder's https://github.com/sjmulder/nbt-js ('nbt' in npm)? It has been inactive (3 years) not sure if @sjmulder is still up for maintaining it but I think it has a cleaner implementation than this module ('minecraft-nbt'), written directly in JavaScript instead of updated translated from CoffeeScript; though it is using a bundled binary decoding library instead of jdataview. Would be cool to have one ultimate NBT library =) considering preserving the interface of minecraft-nbt (so minecraft-mca/region or any other users wouldn't have to change) but replacing the guts with 'nbt'. Maybe https://github.com/andrewrk/node-minecraft-protocol could even use it too? (cc @andrewrk @roblabla)

@max-mapper
Copy link
Owner

thanks for this! IMO the best way to do it today would be to write a new parsing using Buffer in Node, as it runs in all browsers now thanks to https://www.npmjs.org/package/buffer

max-mapper pushed a commit that referenced this pull request Mar 16, 2014
Fix parsing longs and strings
@max-mapper max-mapper merged commit a6d5184 into max-mapper:master Mar 16, 2014
max-mapper added a commit that referenced this pull request Mar 16, 2014
@sjmulder
Copy link

Hey, it’s true that nbt.js hasn’t been getting much love since… years. I’m all up for fixing it up though, updating it to use newer libraries. It’s originally from the Node 0.4 era iirc, which is ancient.

@sjmulder
Copy link

In case you do ever intend to use nbt.js, I’ve just updated it to use Node’s Buffer API instead of the old bundled binary decoding library.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants