stream: Implement BinaryStreamFormat#237
Conversation
7ba5a8c to
0009ae7
Compare
Fuzzy2319
left a comment
There was a problem hiding this comment.
I managed to get BinaryStreamFormat::readF32 matching sead::BinaryStreamFormat::readF32(sead::StreamSrc*, sead::Endian::Types) | decomp.me I will investigate further to avoid making everithing public.
@Fuzzy2319 made 1 comment.
Reviewable status: 0 of 4 files reviewed, all discussions resolved.
|
@Fuzzy2319 Did you find a better match? |
Fuzzy2319
left a comment
There was a problem hiding this comment.
sead::BinaryStreamFormat::readF32(sead::StreamSrc*, sead::Endian::Types) | decomp.me
@Fuzzy2319 made 1 comment.
Reviewable status: 0 of 4 files reviewed, all discussions resolved.
3205288 to
06a32c1
Compare
|
Rebased to include fuzzy changes. I had to work the solution a bit since it broke writeF32 |
|
Maybe you can work something out with this? |
|
Thanks that's the inspiration I needed. After some cleanup turns out my code was correct but trim needed a slight modification. |
3dd01d8 to
d15b3e9
Compare
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 reviewed 5 files and all commit messages, and made 5 comments.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on german77).
modules/src/stream/seadStreamFormat.cpp line 72 at r3 (raw file):
void BinaryStreamFormat::readBit(StreamSrc* src, void* data, u32 bits) {
Suggestion:
// NOTE: leaves higher bits of last byte of data at their previous value
void BinaryStreamFormat::readBit(StreamSrc* src, void* data, u32 bits)
{modules/src/stream/seadStreamFormat.cpp line 91 at r3 (raw file):
void BinaryStreamFormat::readString(StreamSrc* src, BufferedSafeString* str, u32 size) {
Suggestion:
// NOTE: if size > str->getBufferSize(), it wraps around and starts reading to the start again.
// if size > str->getBufferSize()*2, the second iteration continues writing out-of-bounds.
void BinaryStreamFormat::readString(StreamSrc* src, BufferedSafeString* str, u32 size)
{modules/src/stream/seadStreamFormat.cpp line 108 at r3 (raw file):
if (remainingSize != 0) { // Note: what happens if remaining size is bigger than the buffer?
(moved to above function)
modules/src/stream/seadStreamFormat.cpp line 173 at r3 (raw file):
void BinaryStreamFormat::writeBit(StreamSrc* src, const void* data, u32 bits) {
Suggestion:
// NOTE: writes extra bits in last byte into stream normally
void BinaryStreamFormat::writeBit(StreamSrc* src, const void* data, u32 bits)
{modules/src/stream/seadStreamFormat.cpp line 186 at r3 (raw file):
} void BinaryStreamFormat::writeString(StreamSrc* src, const SafeString& str, u32 size)
Suggestion:
// NOTE: if str.calcLength() > size, it still writes it in full
void BinaryStreamFormat::writeString(StreamSrc* src, const SafeString& str, u32 size)
german77
left a comment
There was a problem hiding this comment.
@german77 made 5 comments.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on MonsterDruide1).
modules/src/stream/seadStreamFormat.cpp line 108 at r3 (raw file):
Previously, MonsterDruide1 wrote…
(moved to above function)
Done.
modules/src/stream/seadStreamFormat.cpp line 72 at r3 (raw file):
void BinaryStreamFormat::readBit(StreamSrc* src, void* data, u32 bits) {
Done.
modules/src/stream/seadStreamFormat.cpp line 91 at r3 (raw file):
void BinaryStreamFormat::readString(StreamSrc* src, BufferedSafeString* str, u32 size) {
Done.
modules/src/stream/seadStreamFormat.cpp line 173 at r3 (raw file):
void BinaryStreamFormat::writeBit(StreamSrc* src, const void* data, u32 bits) {
Done.
modules/src/stream/seadStreamFormat.cpp line 186 at r3 (raw file):
} void BinaryStreamFormat::writeString(StreamSrc* src, const SafeString& str, u32 size)
You might misinterpreted this one if str.calcLength() > size it gets clamped to size. str.calcLength() < size it fills the remaining bytes with zeroes.
This means long strings will get chopped off to size and small strings will have the data reserved. This is as far as I can tell the desired behavior with nothing special to note about.
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 reviewed 1 file and all commit messages, made 1 comment, and resolved 5 discussions.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on german77).
modules/src/stream/seadStreamFormat.cpp line 186 at r3 (raw file):
Previously, german77 (Narr the Reg) wrote…
You might misinterpreted this one if str.calcLength() > size it gets clamped to size. str.calcLength() < size it fills the remaining bytes with zeroes.
This means long strings will get chopped off to size and small strings will have the data reserved. This is as far as I can tell the desired behavior with nothing special to note about.
Yes, I did mis-read it. Thanks for correcting me!
Stream formats aren't that useful if the actual memory operations aren't implemented. This fixes fromHostXXX functions that referenced the wrong table.
Matched by fuzzyBinaryStreamFormat::readF32isn't matching. Floats are loaded different due casting from raw to float but I couldn't spot the error. https://decomp.me/scratch/L5MO9FixedBinaryStreamFormat::readStringisn't matching. This one just feels wrong, the implementation I come up leaves me a lot of questions. The write operation just takes the smaller size and fills the rest with null terminators. https://decomp.me/scratch/jCek1This change is