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

Fix achievement mgr crashing on uninitialized data #834

Merged
merged 1 commit into from
Jul 17, 2024

Conversation

Aldarrion
Copy link
Contributor

Both the achievement manager and the achievement data were globals in separate compilation units. Order of initialization is not defined in that case which sometimes caused the mgr to load uninitialized data and crash when an achievement should have been achieved but was not found.

Both the achievement manager and the achievement data were globals in
separate compilation units. Order of initialization is not defined in
that case which sometimes caused the mgr to load uninitialized data and
crash when an achievement should have been achieved but was not found.
@wolfpld
Copy link
Owner

wolfpld commented Jul 17, 2024

What's the impact of the issue you are fixing here?

@Aldarrion
Copy link
Contributor Author

Aldarrion commented Jul 17, 2024

If the linker decides to link in a way where the mgr is constructed first we don't have correct achievement data. It does not happen in your pre-build binaries but when I've built locally on Windows/Debug it happened to me. Opening global settings asserted on missing achievement and crashed.

@wolfpld
Copy link
Owner

wolfpld commented Jul 17, 2024

It does not happen in your pre-build binaries

Ok, so no new release is needed.

@wolfpld wolfpld merged commit 68357bd into wolfpld:master Jul 17, 2024
4 checks passed
@Aldarrion
Copy link
Contributor Author

Yes, fortunately not :) But it will prevent potential issues in future releases.

@Aldarrion Aldarrion deleted the fix-achievement-init-order-crash branch July 17, 2024 20:26
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