Skip to content

Fix: Set DenyReason when access check fails #41012

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix: Set DenyReason when access check fails #41012

wants to merge 1 commit into from
+2 −0

Conversation

Uberration
Copy link

@Uberration Uberration commented Oct 20, 2025 •
edited
Loading

Fixes #40987
Related to PR #40883

Set DenyReason in CheckUserHasLockAccessEvent when access is denied to display "Access denied." popup to users. Uses ??= to avoid overriding reasons from other lock systems.

About the PR

Added an else clause to AccessReaderSystem.OnCheckLockAccess that sets the DenyReason field when a user is denied access to a locked entity.

Why / Balance

After PR #40883 refactored the lock system to use an event-based architecture, users stopped receiving the correct text popup when attempting to interact with locked entities (lockers, doors, etc.) without proper access. This restores the "Access denied." popup text.

Technical details

  • Modified AccessReaderSystem.OnCheckLockAccess to set args.DenyReason using the existing "lock-comp-has-user-access-fail" localization key when IsAllowed returns false
  • Uses ??= (null-coalescing assignment) to ensure we don't override deny reasons that may have been set by other lock type systems (e.g., fingerprint readers)

Media

Content Client_lMj4LdkduG

Requirements

Breaking changes

None

🆑 Changelog

  • fix: Fixed "Access denied." popup text when attempting to interact with locked entities without proper access

@Uberration
Update AccessReaderSystem.cs
0f05758 
Fixes space-wizards#40987
Related to PR space-wizards#40883

Set DenyReason in CheckUserHasLockAccessEvent when access is denied to display "Access denied." popup to users. Uses ??= to avoid overriding reasons from possible systems.
@PJBot PJBot added S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. size/XS Denotes a PR that changes 0-9 lines. labels Oct 20, 2025
@FairlySadPanda FairlySadPanda added P3: Standard Priority: Default priority for repository items. T: New Feature Type: New feature or content, or extending existing content DB: Beginner Friendly Difficulty: Great for beginners. Unambiguous in scope, and explains how to achieve the result. A: General Interactions Area: General in-game interactions that don't relate to another area. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Oct 21, 2025
@slarticodefast slarticodefast added T: Bugfix Type: Bugs and/or bugfixes and removed T: New Feature Type: New feature or content, or extending existing content labels Oct 21, 2025
PJB3005
PJB3005 reviewed Oct 21, 2025
View reviewed changes
Copy link
Member

@PJB3005 PJB3005 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. The original issue is clearly that somebody forgot a Loc.GetString somewhere. How does null-coalescing a value fix that?

@PJB3005 PJB3005 changed the title Update AccessReaderSystem.cs Fix access denial popups Oct 21, 2025
@PJB3005
Copy link
Member

PJB3005 commented Oct 21, 2025

Please give your pull requests an appropriate title.

@Uberration Uberration changed the title Fix access denial popups Fix: Set DenyReason when access check fails Oct 21, 2025
@Uberration
Copy link
Author

I don't understand. The original issue is clearly that somebody forgot a Loc.GetString somewhere. How does null-coalescing a value fix that?

??= is for defensive programming.

If another system has already set DenyReason, this won't overwrite it. If that's not a concern, a simple = assignment would work fine as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PJB3005 PJB3005 PJB3005 left review comments

@ScarKy0 ScarKy0 Awaiting requested review from ScarKy0

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

A: General Interactions Area: General in-game interactions that don't relate to another area. DB: Beginner Friendly Difficulty: Great for beginners. Unambiguous in scope, and explains how to achieve the result. P3: Standard Priority: Default priority for repository items. S: Needs Review Status: Requires additional reviews before being fully accepted. Not to be replaced by S: Approved. size/XS Denotes a PR that changes 0-9 lines. T: Bugfix Type: Bugs and/or bugfixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Attempting to open a locker without access gives a nonexistent popup

5 participants

@Uberration @PJB3005 @FairlySadPanda @PJBot @slarticodefast