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

Ensure API starts up with LocalUser in correct state #31507

Merged
merged 1 commit into from
Jan 14, 2025

Conversation

peppy
Copy link
Member

@peppy peppy commented Jan 14, 2025

I noticed in passing that in a very edge case scenario where the API's run thread doesn't run before it is loaded into the game, something could access it and get a guest LocalUser when the local user actually has a valid login.

Put another way, the protected HasLogin could be true while LocalUser is Guest.

I think we want to avoid this, so I've moved the initial set of the local user earlier in the initialisation process.

If this is controversial in any way, the PR can be closed and we can assume no one is ever going to run into this scenario (or that it doesn't matter enough even if they did).

I noticed in passing that in a very edge case scenario where the API's
`run` thread doesn't run before it is loaded into the game, something
could access it and get a guest `LocalUser` when the local user actually
has a valid login.

Put another way, the `protected HasLogin` could be `true` while
`LocalUser` is `Guest`.

I think we want to avoid this, so I've moved the initial set of the
local user earlier in the initialisation process.

If this is controversial in any way, the PR can be closed and we can
assume no one is ever going to run into this scenario (or that it
doesn't matter enough even if they did).
@smoogipoo smoogipoo merged commit f2b7984 into ppy:master Jan 14, 2025
10 checks passed
@peppy peppy deleted the api-startup-user branch January 14, 2025 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants