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

Can't compile with MSVC #24

Closed
jsonmona opened this issue Mar 1, 2024 · 2 comments
Closed

Can't compile with MSVC #24

jsonmona opened this issue Mar 1, 2024 · 2 comments

Comments

@jsonmona
Copy link

jsonmona commented Mar 1, 2024

Overview

There is a single use of variable length array in this project. MSVC won't support it, so the project fails to compile.

In function bool StateSaver::saveState(...):

char stateBuf[size];

Change Proposal

My suggestion is to use std::vector instead of VLA. The performance should be unaffected since that function is doing a file write, which would take significantly more time compared to a heap allocation.

I've written the patch, and verified that this project does compile with that change (although without zlib. I simply didn't test them).
I can submit a PR as long as you're okay with this change.

Resources

A Link to the PastPatch: https://github.com/jsonmona/gambatte-core/blob/2011b9981df76e02619e32e8c5249115c3f97f32/libgambatte/src/statesaver.cpp#L555

@CasualPokePlayer
Copy link
Member

CasualPokePlayer commented Mar 1, 2024

There is an existing PR already for this #20

I haven't bothered merging it yet as there's still some nits needing to be resolved, and there's not much point in supporting MSVC specifically when clang-cl exists (so it's rather low priority).

@jsonmona
Copy link
Author

jsonmona commented Mar 2, 2024

Oh. I didn't know it. I should've searched PRs too. My bad.

I'll close this issue.

@jsonmona jsonmona closed this as completed Mar 2, 2024
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

No branches or pull requests

2 participants