-
Notifications
You must be signed in to change notification settings - Fork 40
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
Copy & paste as SNBT #22
base: master
Are you sure you want to change the base?
Conversation
It now requires the original & new types to match
Emscripten implementation of num_get does not allow hexadecimal digits (i. e. letters [a-f]) immediately after decimal floating-point numbers
resolved in 25fc15c
wr_snbt(ByteArrayTag) { | ||
stream << "[B;"; | ||
ByteTag numTag; | ||
uint32_t count = htonl(value.count); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you use htonl
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you use
htonl
here?
I just copied this line from here:
webNBT/nbt-utils/nbt_utils.cpp
Lines 211 to 214 in 1ec2913
wr_payload(ByteArrayTag) { | |
uint32_t count = htonl(value.count); | |
stream.write((char *)&count, 4); | |
stream.write((char *)value.data.get(), value.count); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Vftdan It is used there because NBT requires that integer values are serialized in network (big endian) order. That is not necessarily the endianness of the host system (which usually is little endian). Hence the conversion from host to network (hton
).
When you are doing this before your for
-loop, however, very nasty things happen if the host order and network order mismatch. An array that has only one element (value.count = 0x00000001
) will cause your local count
variable to have the value 0x01000000
, causing your loop to run 16777216 times even though there is only one element.
char c = *it; | ||
switch (c) { | ||
case '"': | ||
case '\'': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need to escape single quotes given that your output is always encapsulated by double quotes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, but I feel like it is less likely to cause problems if somebody would write a bad parser
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Vftdan the problem is that Minecraft will emit the error Invalid escape sequence \'
when you escape single quotes.
There seems to be a problem with copying doubles. They are exported in hexadecimal notation, which they are not supposed to be afaik. |
The error message when pasting something of incorrect type is confusing. It says "Parse error at position 0" even though the pasted content is syntactically correct. Perhaps it should say something along the lines of, e.g., "Cannot paste content of type 'Double' into property of type 'Long'". In the future, it would be cool to automatically cast at some of the types (in particular among the numerical types). But for now we can live without this :-) |
wr_snbt(FloatTag) { stream.setf(std::ios::fixed, std::ios::floatfield); stream.setf(std::ios::showpoint); stream << std::setprecision(std::numeric_limits<decltype(value)>::max_digits10) << value << 'f'; } | ||
|
||
rd_snbt(DoubleTag) { stream >> value; return !!stream; } | ||
wr_snbt(DoubleTag) { stream.setf(std::ios::floatfield, std::ios::floatfield); stream.setf(std::ios::showpoint); stream << std::setprecision(std::numeric_limits<decltype(value)>::max_digits10) << value; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first call to stream.setf
should not use std::ios::floatfield
as first argument. You should be using std::ios::fixed
here as well, as otherwise doubles are emitted in hexadecimal notation, which Minecraft is unable to parse.
Implement copying & pasting of subtrees as text, that should be valid SNBT (the syntax used in command blocks, but I have not tested).
Current limitations
Edit value...
instead)*ArrayTag
s), so there may be other problemsprompt
Closes #5, #15