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

APC interface no longer restricted to a single user at a time #32325

Closed

Conversation

hyphenationc
Copy link
Contributor

About the PR

APCs are no longer accessible by only one user at a time.

Why / Balance

AI should not be able to lock players out of APCs. Prevents #32312

Technical details

apc.yml has had singleUser value changed from true to false.

Media

Unnecessary.

Requirements

Changelog

@github-actions github-actions bot added the No C# For things that don't need code. label Sep 20, 2024
@slarticodefast slarticodefast added the Undergoing Maintainer Discussion This PR is currently going through an internal discussion by the maintainer team. label Sep 20, 2024
@lzk228
Copy link
Contributor

lzk228 commented Sep 20, 2024

have you tested it in game? does the ui work in right way?

@hyphenationc
Copy link
Contributor Author

hyphenationc commented Sep 20, 2024

have you tested it in game? does the ui work in right way?

Works as well as it usually does. Two users (clients) can open the UI at the same time, and even if one has the proper access required (access: [["Engineering"]]) the other user without requisite access is unable to toggle it, while the one with is.

Small strangeness where if you lose access at any point while in the menu it locks you out even if you regain that access, but really that's neither here nor there. It also happens already, and isn't something introduced with this change.

@eoineoineoin
Copy link
Contributor

The reason it's singleUser: false is that the UI doesn't properly work with multiple users. Here's a "captain" being unable to toggle the breaker, despite having proper access:

2024-09-21_09-23

In ApcSystem.cs, there's a TODO, which you'd at least need to fix before making the UI multi-user:

// TODO: this should be per-player not stored on the apc

@hyphenationc
Copy link
Contributor Author

Didn't see that todo. Probably shoulda checked, but I'll look into it now. Might be the cause of the

small strangeness where if you lose access at any point while in the menu it locks you out even if you regain that access
bit.

If nothing else, I can probably scrape code from the vending machines to have it check on every interaction, or if that isn't what's happening there, whatever I make here can be applied there to prevent the same issue. Or something like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
No C# For things that don't need code. Undergoing Maintainer Discussion This PR is currently going through an internal discussion by the maintainer team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants