-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
A rudimentary implementation of multiple access levels for the purpose of restrictive cargo ordering #34755
Open
GansuLalan
wants to merge
5
commits into
space-wizards:master
Choose a base branch
from
GansuLalan:multiple-access-restricted-ordering
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
A rudimentary implementation of multiple access levels for the purpose of restrictive cargo ordering #34755
GansuLalan
wants to merge
5
commits into
space-wizards:master
from
GansuLalan:multiple-access-restricted-ordering
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Marking this as P1 because this directly contributes to #20064, mainly mechanically resisting powergaming. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
It is apparently failing because of FlammableComponent? No idea why |
gusxyz
reviewed
Feb 1, 2025
gusxyz
reviewed
Feb 1, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
A: Cargo/Salvage
Area: Cargo department or Salvage.
Changes: UI
Changes: Might require knowledge of UI design or code.
D3: Low
Difficulty: Some codebase knowledge required.
P1: High
Priority: Higher priority than other items, but isn't an emergency.
S: Needs Review
Status: Requires additional reviews before being fully accepted
size/M
Denotes a PR that changes 100-999 lines.
T: Balance Change
Type: Balance changes through direct value changes, or changes to mechanics that affect it
T: New Feature
Type: New feature or content, or extending existing content
T: Of Admin Interest
Type: Affects administration work a lot, and might require admins to weigh in on
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
About the PR
Originally wanted to fix #7838 since the grand lottery haunts my funds but found it would require implementing a method to have multiple access levels on the same device as it's currently only one per. I therefore implemented a system to allow for such checking while still remaining compatible with current access methods and then implemented the restrictive cargo ordering as something of a demo/ test for the system.
Why / Balance
While the general idea of wanting multiple access levels is somewhat self explanatory, speaking from the perspective of a person who likes to QM, it is very easy for rouge techie to just order whatever since there is no restriction on self approval. I have personally watched addicted techies gamble the departments money away and although you can deal with them after the fact the damage is already done so prevention is somewhat ideal.
There is a level of concern about the removal of the chaotic factor the current system brings however as the occasional rouge techie can be amusing though that is a personal preference thing.
Technical details
AccessReaderComponent has a new variable for additional access lists, this can then be checked against via the normal AccessReaderSystem method by adding an index of the list to be checked against, the system will fall back to the general list if the given index doesn't appear to exist. This is all optional and to my knowledge the system can be used while pretending none of these changed happened. While I originally intended to change AccessLists, this proved awkward due to how abundant its use is in various prototypes and systems and also breaking the tests so such a change would require a larger refactor.
Changes to the cargo systems are to allow for restrictive ordering. The cargo order ui has been changed to add a checkbox that syncs across clients (hopefully) and can be changed by someone with QM access, this will then be tracked via a hashSet with a data class and checked against when an approval is attempted and will be denied unless approved by QM access or is unrestricted and then general approval can be given again.
There's also a fix for a telepad thing as the animation was crashing me during testing due to it attempting to be set twice
Other notes:
If anyone knows a better way to do Content.Client/Cargo/BUI/CargoOrderConsoleBoundUserInterface.cs line 186 better, please say as this my first time using xmal outside of small instances
I'm still new to ECS stuff so things might just be flat out bad, if so please let me know so I can improve them
I realise this might not be how such a concept is wished to be handled/ implemented, if so there's no worries, was an interesting problem to tackle anyways
UI could probably be improved, just uses the basic checkboxes for now
Having double checked the clips, might be a problem with the radio but it's late as of writing so will check/fix later
Media
Clips are a bit wavy, just trying to show off the system working
output2.mp4
Just showing emag works, most of the clip is just waiting for the money to come in
output1.mp4
Requirements
Breaking changes
None that I'm aware of, I tried to keep things as unintrusive as possible, there are new classes though
Changelog
🆑 Gansu