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

Set spamms CustomRPC to SendOption.None & Remove Unused #1505

Merged
merged 6 commits into from
Mar 1, 2025

Conversation

Tommy-XL
Copy link
Collaborator

@Tommy-XL Tommy-XL commented Mar 1, 2025

Instead of sending every CustomRPC as SendOption.None, it is better to send only spammed Rpc as SendOption.None

@Tommy-XL Tommy-XL added the Fix bug Something isn't working label Mar 1, 2025
@Tommy-XL Tommy-XL requested a review from NikoCat233 March 1, 2025 06:48
@Tommy-XL Tommy-XL changed the title Set spamms RPC To None & Remove Unused Set spamms CustomRPC to SendOption.None & Remove Unused Mar 1, 2025
@Tommy-XL
Copy link
Collaborator Author

Tommy-XL commented Mar 1, 2025

I didn’t know that in CustomRpc's there are about 10 unused Rpc's until I checked each one through a search, lmao

@NikoCat233 NikoCat233 requested a review from Copilot March 1, 2025 09:27
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

The purpose of this PR is to change the RPC sending behavior such that only spammed RPCs use SendOption.None (now replaced by a specialized RpcSendOption) and to remove unused enum members and redundant logic. Key changes include:

  • Updating the CustomRPC enum by removing unused members and adjusting comments to reflect current usage.
  • Removing the fallback RPC message creation logic in the StartRpcImmediately Harmony patch and changing its signature.
  • Updating various role-specific RPC methods to use RpcSendOption and to conditionally send RPCs only to non-host modded clients.

Reviewed Changes

File Description
Modules/RPC.cs Updated CustomRPC enum values and removed legacy RPC message creation logic in a Harmony patch
Modules/ExtendedPlayerControl.cs Replaced SendOption.None with RpcSendOption in various RPC methods to refine RPC sending
Roles/Crewmate/Chameleon.cs
Roles/Crewmate/Alchemist.cs
Roles/Coven/Sacrifist.cs
Roles/Impostor/Swooper.cs
Roles/Crewmate/Overseer.cs
Roles/Coven/PotionMaster.cs
Roles/Neutral/Wraith.cs
Adjusted role-specific RPC calls to use the new RpcSendOption and updated method visibility as needed

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

Modules/RPC.cs:19

  • The comment update from '184/255 USED' to '174/255 USED' and the reordering/removal of enum members may affect consumers relying on specific enum values. Verify that all external references and RPC mappings are updated accordingly.
public enum CustomRPC : byte // 174/255 USED

Roles/Coven/Sacrifist.cs:77

  • Changing the SendRPC method from public to private static may impact any external callers. Please confirm that this method is only used internally.
private static void SendRPC(PlayerControl pc)

Modules/ExtendedPlayerControl.cs:833

  • The change from using SendOption.None to using RpcSendOption alters the RPC sending behavior. Ensure that RpcSendOption is correctly defined and that this change produces the intended message delivery semantics.
MessageWriter writer = AmongUsClient.Instance.StartRpcImmediately(target.NetId, (byte)RpcCalls.ProtectPlayer, RpcSendOption, target.GetClientId());

@NikoCat233
Copy link
Collaborator

Try add a custom rpc RequestResendCustomRoles
If mod client finds that its missing some people's roles, it can send this rpc and ask host to sync again.

@NikoCat233 NikoCat233 merged commit 02b6150 into dev_2.3.0 Mar 1, 2025
1 check passed
@NikoCat233 NikoCat233 deleted the SetSpammsRPCToNoneAndRemoveUnused branch March 1, 2025 13:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fix bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants