filedevice: Implement BufferFileDevice(Write/Read)Stream#263
Conversation
4fe55db to
a8164b3
Compare
|
I narrowed down the mismatch to compile order. |
|
Found the mistake in the file location with asserts from other games. Now everything matches! |
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 partially reviewed 9 files and all commit messages, and made 6 comments.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on german77).
include/prim/seadPtrUtil.h line 39 at r2 (raw file):
static void* align(const void* ptr, u64 n) { const uintptr_t result = (uintptr_t(ptr) + n - 1) & -n;
I'm not sure if this is equivalent
Suggestion:
const uintptr_t result = (uintptr_t(ptr) + n - 1) & ~n;include/resource/seadResource.h line 61 at r2 (raw file):
public: IndirectResource(); ~IndirectResource() override;
compiler-generated
Suggestion:
IndirectResource();modules/src/stream/seadBufferStream.cpp line 45 at r2 (raw file):
} u32 BufferReadStreamSrc::skip(s32 offset)
Suggestion:
// NOTE: cannot take negative `offset`, but expects `mSrc->skip(X)` to work with negatives
s32 BufferReadStreamSrc::skip(s32 offset)modules/src/stream/seadBufferStream.cpp line 133 at r2 (raw file):
bool isDone = mSrc->write(mBuffer, mCurrentPos) >= mCurrentPos; mCurrentPos = 0; return isDone;
Suggestion:
bool success = mSrc->write(mBuffer, mCurrentPos) >= mCurrentPos;
mCurrentPos = 0;
return success;include/stream/seadFileDeviceStream.h line 47 at r2 (raw file):
void setFileHandle(sead::FileHandle* fileHandle); FileDeviceStreamSrc* getSrc() { return &src; }
Suggestion:
FileDeviceStreamSrc* getSrc() const { return &src; }include/stream/seadFileDeviceStream.h line 64 at r2 (raw file):
void setFileHandle(sead::FileHandle* fileHandle); FileDeviceStreamSrc* getSrc() { return &src; }
Suggestion:
FileDeviceStreamSrc* getSrc() const { return &src; }
german77
left a comment
There was a problem hiding this comment.
@german77 made 5 comments.
Reviewable status: 2 of 9 files reviewed, 6 unresolved discussions (waiting on MonsterDruide1).
include/prim/seadPtrUtil.h line 39 at r2 (raw file):
Previously, MonsterDruide1 wrote…
I'm not sure if this is equivalent
Done. ~n is not equivalent but (~n + 1) is
include/resource/seadResource.h line 61 at r2 (raw file):
Previously, MonsterDruide1 wrote…
compiler-generated
Done.
modules/src/stream/seadBufferStream.cpp line 45 at r2 (raw file):
} u32 BufferReadStreamSrc::skip(s32 offset)
Done.
include/stream/seadFileDeviceStream.h line 47 at r2 (raw file):
void setFileHandle(sead::FileHandle* fileHandle); FileDeviceStreamSrc* getSrc() { return &src; }
I need a non const reference as this object is passed to another to be read or written.
include/stream/seadFileDeviceStream.h line 64 at r2 (raw file):
void setFileHandle(sead::FileHandle* fileHandle); FileDeviceStreamSrc* getSrc() { return &src; }
I need a non const reference as this object is passed to another to be read or written.
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 partially reviewed 7 files and all commit messages, made 1 comment, and resolved 4 discussions.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on german77).
include/prim/seadPtrUtil.h line 39 at r2 (raw file):
Previously, german77 (Narr the Reg) wrote…
Done. ~n is not equivalent but (~n + 1) is
Ah, I think the logic is best described by ~(n - 1) then (subtract one to turn for example 8 to 0b111, then invert all bits to effectively set the lowest three bits to zero).
german77
left a comment
There was a problem hiding this comment.
@german77 made 1 comment.
Reviewable status: 8 of 9 files reviewed, 2 unresolved discussions (waiting on MonsterDruide1).
include/prim/seadPtrUtil.h line 39 at r2 (raw file):
Previously, MonsterDruide1 wrote…
Ah, I think the logic is best described by
~(n - 1)then (subtract one to turn for example8to0b111, then invert all bits to effectively set the lowest three bits to zero).
Done.
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on german77).
modules/src/stream/seadBufferStream.cpp line 133 at r2 (raw file):
bool isDone = mSrc->write(mBuffer, mCurrentPos) >= mCurrentPos; mCurrentPos = 0; return isDone;
Here's still a comment ^
isDone seems like it could be finished later - which is wrong, the function rather indicates whether it was successful or failed in the process of writing.
12ce676 to
e6ef469
Compare
german77
left a comment
There was a problem hiding this comment.
@german77 made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on MonsterDruide1).
modules/src/stream/seadBufferStream.cpp line 133 at r2 (raw file):
Previously, MonsterDruide1 wrote…
Here's still a comment ^
isDoneseems like it could be finished later - which is wrong, the function rather indicates whether it was successful or failed in the process of writing.
Done. Not sure what's up with reviewable is hiding review comments
MonsterDruide1
left a comment
There was a problem hiding this comment.
@MonsterDruide1 reviewed 2 files and all commit messages, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on german77).
Originally the scope of this PR was to implement
BufferFileDeviceWriteStreamandBufferFileDeviceReadStream. However due some issues I ended up implementingBufferReadStreamSrc,BufferWriteStreamSrc,BufferWriteStream,BufferReadStream,IndirectResourceandIndirectResourceFactoryBaseas well.BufferFileDeviceReadStream dtor is not matching. The asm says it shouldn't be inlined but my dtor is fully inlined. I can't move it to another cpp file because the ctor is fully inlined. Decompme doesn't want to generate this scratch so there's no link this time.fixedThis change is