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

Suggestion RPC Fix #53

Open
Woodi-dev opened this issue Feb 22, 2021 · 10 comments
Open

Suggestion RPC Fix #53

Woodi-dev opened this issue Feb 22, 2021 · 10 comments

Comments

@Woodi-dev
Copy link

Woodi-dev commented Feb 22, 2021

Hey first of all great Mod.
I saw you took some code snippets from my Sheriff mod Code. However, this was my first mod and there are some bugs included.

Please do not use
StartRpcImmediately with SendOption None for Network messages since the message may go lost.

Use
MessageWriter writer = FMLLKEACGIO.Instance.StartRpc(NET ID, RPC CALL, Hazel.SendOption.Reliable);
writer.EndMessage();

Regards,
Woodi

@NotHunter101
Copy link
Owner

Whoops, that may actually be why a lot of problems occur with applying roles. Thanks a lot, I'll fix this when I can. I loved your Sheriff mod, by the way!

@Woodi-dev
Copy link
Author

Thank you. No problem :)

@rpet91
Copy link

rpet91 commented Feb 27, 2021

Would love to see the Love Couple mod merged with this one. :D

@psibean
Copy link

psibean commented Mar 4, 2021

Does this mean ALL of the StartRpcImmediately calls need to be changed?

@Woodi-dev
Copy link
Author

Does this mean ALL of the StartRpcImmediately calls need to be changed?
No. You can use this method. It is faster than startrpc. But more important is the SendOption Reliable. Selecting the player role is some information which everybody has to get. If you wanna send other information like player position or smth you can use SendOption None.

@Popeye4242
Copy link
Contributor

Popeye4242 commented Mar 10, 2021

https://github.com/NuclearPowered/Reactor/blob/master/Reactor.Example/ExampleRpc.cs

All calls should be changed to Reactor CustomRpcs. Using them means this mod is compatible with other mods because the custom RPC id's don't overlap. Any mod should use this implementation.

 Rpc<ExampleRpc>.Instance.Send(new ExampleRpc.Data("Cześć :)"));
Rpc<ExampleRpc>.Instance.SendTo(AmongUsClient.Instance.HostId, new ExampleRpc.Data("host :O"));

@Woodi-dev
Copy link
Author

Woodi-dev commented Mar 11, 2021

https://github.com/NuclearPowered/Reactor/blob/master/Reactor.Example/ExampleRpc.cs

All calls should be changed to Reactor CustomRpcs. Using them means this mod is compatible with other mods because the custom RPC id's don't overlap. Any mod should use this implementation.

 Rpc<ExampleRpc>.Instance.Send(new ExampleRpc.Data("Cześć :)"));
Rpc<ExampleRpc>.Instance.SendTo(AmongUsClient.Instance.HostId, new ExampleRpc.Data("host :O"));

Of course we could change that easily. RPC calls like in my Sheriff mod were made before Reactor. The problem is that changing RPC net code makes automatically new mod versions incompatible to older versions

@NotHunter101
Copy link
Owner

Still thinking about whether I want to actually switch to the Reactor RPC implementation because of the reason above. Either way, I am going to be fixing this asap, I have just been really busy with mod commissions.

@Popeye4242
Copy link
Contributor

Fixed with #90

@Netskeh
Copy link

Netskeh commented Apr 9, 2021

might be a little late to chime in, here are my 2 cents:
since there is a mod breaking game update anyway (as far as I can tell, I did not play with the mod since) - one could use the opportunity to do this kind of change as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants