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

Use vector instead of array, and proper referencing #196

Closed
wants to merge 2 commits into from

Conversation

tophyr
Copy link
Contributor

@tophyr tophyr commented Apr 27, 2024

Instead of using a hardcode-size array for app instances, use a vector instead. This cleans up some bad referencing as well.

Instead of using a hardcode-size array for app instances, use a vector
instead. This cleans up some bad referencing as well.
Enable DebugAsan builds so we can begin using ASAN without turning it on
for the whole debug build.
@Lgt2x Lgt2x self-assigned this Apr 28, 2024
@Lgt2x
Copy link
Collaborator

Lgt2x commented Apr 29, 2024

CMakeLists.txt conflict needs resolved. Does this PR actually solve any compilation or runtime problem ?

@winterheart
Copy link
Collaborator

CMakeLists.txt changes are not related and failing Windows build. Please remove them from PR.

@Lgt2x
Copy link
Collaborator

Lgt2x commented Apr 29, 2024

In addition, please refrain from refactoring complete files when not absolutely necessary. We don't have a full testing pipeline right now, we're just trying really hard to get the base game working, and full file refactors can easily introduce problems. I'd prefer a simpler fix for getting Asan to work if possible. Thanks!

@Lgt2x Lgt2x removed their assignment Apr 30, 2024
@Lgt2x
Copy link
Collaborator

Lgt2x commented May 3, 2024

In addition, please refrain from refactoring complete files when not absolutely necessary. We don't have a full testing pipeline right now, we're just trying really hard to get the base game working, and full file refactors can easily introduce problems. I'd prefer a simpler fix for getting Asan to work if possible. Thanks!

Feel free to re-open with a simpler fix!

@Lgt2x Lgt2x closed this May 3, 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

Successfully merging this pull request may close these issues.

None yet

3 participants