Skip to content

Implements gsys::MemBuffer#161

Merged
Pistonight merged 1 commit into
zeldaret:masterfrom
VINZlehacker:implements/gsysMemBuffer
Apr 5, 2026
Merged

Implements gsys::MemBuffer#161
Pistonight merged 1 commit into
zeldaret:masterfrom
VINZlehacker:implements/gsysMemBuffer

Conversation

@VINZlehacker

@VINZlehacker VINZlehacker commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

I decompiled the gsys::MemBuffer class.


This change is Reviewable

@Pistonight Pistonight left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pistonight reviewed 3 files and all commit messages, and made 2 comments.
Reviewable status: 3 of 4 files reviewed, 2 unresolved discussions (waiting on VINZlehacker).


lib/gsys/src/gsys/gsysMemBuffer.cpp line 10 at r1 (raw file):

MemBuffer::MemBuffer() = default;
MemBuffer::~MemBuffer() = default;

Does it work when moving these to the header? (At least should be able to do the dtor virtual ~MemBuffer() = default;


lib/gsys/src/gsys/gsysMemBuffer.cpp line 29 at r1 (raw file):

void MemBuffer::free() {
    if (mBuffer)

Can you try without this if?

@VINZlehacker VINZlehacker left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VINZlehacker made 2 comments.
Reviewable status: 2 of 4 files reviewed, 2 unresolved discussions (waiting on Pistonight).


lib/gsys/src/gsys/gsysMemBuffer.cpp line 10 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Does it work when moving these to the header? (At least should be able to do the dtor virtual ~MemBuffer() = default;

No, if I do that, it will inline the constructor, and for the virtual destructor, it is sometimes called directly (without going through the vtable), and if I put it in the header, it will inline it in those cases.


lib/gsys/src/gsys/gsysMemBuffer.cpp line 29 at r1 (raw file):

Previously, Pistonight (Michael Zhao) wrote…

Can you try without this if?

Done.

@VINZlehacker

Copy link
Copy Markdown
Contributor Author

lib/gsys/src/gsys/gsysMemBuffer.cpp line 10 at r1 (raw file):

Previously, VINZlehacker (auguste120) wrote…

No, if I do that, it will inline the constructor, and for the virtual destructor, it is sometimes called directly (without going through the vtable), and if I put it in the header, it will inline it in those cases.

Done.

@Pistonight Pistonight left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM now - one more thing: can you rebase and squash all the commits into one?

git remote add upstream git@github.com:zeldaret/botw
git fetch upstream
git rebase -i upstream/master

Change pick to fixup for all except the first commit in the rebase_todo file

git push -f

@Pistonight reviewed 2 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on VINZlehacker).

@VINZlehacker VINZlehacker force-pushed the implements/gsysMemBuffer branch from 7f281c3 to 3a2c2b4 Compare April 3, 2026 19:31

@VINZlehacker VINZlehacker left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@VINZlehacker made 1 comment.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on VINZlehacker).

@VINZlehacker VINZlehacker left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@VINZlehacker made 1 comment.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on VINZlehacker).

@Pistonight Pistonight merged commit 1b79b40 into zeldaret:master Apr 5, 2026
3 of 4 checks passed
@Pistonight

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

@VINZlehacker VINZlehacker deleted the implements/gsysMemBuffer branch April 6, 2026 10:33
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.

2 participants