Skip to content

Predict Admin Verbs (Not Done Yet) #39550

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 53 commits into
base: master
Choose a base branch
from
Open

Predict Admin Verbs (Not Done Yet) #39550

wants to merge 53 commits into from

Conversation

VerinSenpai
Copy link
Contributor

@VerinSenpai VerinSenpai commented Aug 11, 2025 •
edited
Loading

About the PR

Requires #40480 #39921 #39802 #40499

This PR predicts just about all admin verb creation. Several actions are still in need of prediction.

I'm creating this now to seek input and would you just look at that diff. Just look at it. Shield your eyes Beck.

Why / Balance

It's a prediction PR sillly :)

Technical details

Creates the following shared systems:

  • SharedAdminVerbSystem.Antag
  • SharedAdminVerbSystem.Debug
  • SharedAdminVerbSystem.Smites
  • SharedAdminVerbSystem.Tools
  • SharedAdminVerbSystem

Moves the vast majority of verb definitions to shared.
Creates virtual overrides for any verbs that can only be executed on the server.
TEMP Creates overrides for CanCommand on the server and client to allow for command permission checks in shared.
Conducts some conformity conversions to clean up some squigglies. That is to say, I specified ResPath types for any SpriteSpecifier.Rsi uses and changed all verbs to be defined in the same manner.

A BIG PR for a BIG BOY.

Requirements

@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/L Denotes a PR that changes 1000-4999 lines. labels Aug 11, 2025
@VerinSenpai
Copy link
Contributor Author

Any help testing is appreciated. I'm aware of some issues with verbs mispredicting their actions. This is happening because they're calling console commands and they're executing them several times as is expected for prediction. I will sort this out or die trying 👌

@VerinSenpai VerinSenpai added S: DO NOT MERGE Status: Open item that should NOT be merged. DNM. Allows test to run unlike draft. P3: Standard Priority: Default priority for repository items. T: Refactor Type: Refactor of notable amount of codebase T: Cleanup Type: Code clean-up, without being a full refactor or feature D1: High Difficulty: Extensive codebase knowledge required. A: Admin Tooling Area: Admin tooling and moderation. and removed S: Untriaged Status: Indicates an item has not been triaged and doesn't have appropriate labels. labels Aug 11, 2025
@slarticodefast slarticodefast marked this pull request as draft August 11, 2025 08:50
@slarticodefast slarticodefast removed the S: DO NOT MERGE Status: Open item that should NOT be merged. DNM. Allows test to run unlike draft. label Aug 11, 2025
@slarticodefast
Copy link
Member

We can just set it to a draft instead of using the DNM label.

@slarticodefast slarticodefast mentioned this pull request Aug 11, 2025
Draft
2 tasks
@VerinSenpai
Copy link
Contributor Author

DisposalTubeComponent needs moved to shared to finish out the debug verbs.

@VerinSenpai
Copy link
Contributor Author

BatteryComponent is also needed in shared for full prediction.

@slarticodefast slarticodefast added the T: Prediction Type: Moving code to Content.Shared and making it predicted. label Sep 11, 2025
@VerinSenpai
Copy link
Contributor Author

Truly beautifully

@VerinSenpai
Copy link
Contributor Author

I've updated my description to indicate it requires 3 additional prs. I haven't made those changes yet but I should be able to some time today.

@github-actions GitHub Actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions github-actions bot added the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Sep 22, 2025
@VerinSenpai
Copy link
Contributor Author

🫥

@PJBot PJBot added size/XL Denotes a PR that changes 5000+ lines. and removed size/L Denotes a PR that changes 1000-4999 lines. labels Sep 22, 2025
@github-actions github-actions bot removed the S: Merge Conflict Status: Needs to resolve merge conflicts before it can be accepted label Sep 22, 2025
@VerinSenpai
Copy link
Contributor Author

Test fail real 😞

I need the brain component moved to shared.

@VerinSenpai VerinSenpai mentioned this pull request Sep 22, 2025
Open
2 tasks
@VerinSenpai
Copy link
Contributor Author

I've added three of the required prs to this. I feel like I prolly went about it the wrong way since electro is now a participant here.

That said, those prs should be merged before this one so I assume it'll be alright.

@VerinSenpai
Copy link
Contributor Author

Every day we come closer to god prediction.

@VerinSenpai
Copy link
Contributor Author

right so its clear to me I brought over SharedAdminManager without fully understanding what I was looking at. I can not at least currently use SharedAdminManager as a dependency so it doesn't help me in this situation. I'll be reverting that when I get a chance.

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

@DrSmugleaf DrSmugleaf Awaiting requested review from DrSmugleaf DrSmugleaf is a code owner

@crazybrain23 crazybrain23 Awaiting requested review from crazybrain23 crazybrain23 is a code owner

Assignees
No one assigned
Labels
A: Admin Tooling Area: Admin tooling and moderation. D1: High Difficulty: Extensive codebase knowledge required. 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/XL Denotes a PR that changes 5000+ lines. T: Prediction Type: Moving code to Content.Shared and making it predicted.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@VerinSenpai @slarticodefast @PJBot @ElectroJr