-
Notifications
You must be signed in to change notification settings - Fork 66
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
Rename packet 82 #159
Comments
Any updates? |
Don't know what a better name is than "LoadNetModule" and "NetModule" isn't a verb either. |
Perhaps |
|
If that's the case, is |
Our PacketTypes.cs naming contains a mix of |
I'm not entirely sure about renaming it either. I don't believe anything we change it to will actually provide extra clarity as to what the packet does. Since the packet and the logic behind it is quite complex there isn't really anything we can call it that makes it obvious and intuitive. |
I don't know what the packet does. |
& I've heard 10 different explanations for what it does. |
Marking as stalled. This is still a bad packet that does multiple things with no common base |
I suppose NetModule is terraria's next-gen network protocol, but they ended up with just two packets then back to the old one. I'd suggest to call it NetModule because it's different from others. |
This packet defines even more modules as of 1.4 |
I forgot this was this big of a bikeshed
Lucas Nicodemus
http://keybase.io/xn
…On Tue, May 19 2020 at 16:26, quake1337 < ***@***.*** > wrote:
This packet defines even more modules as of 1.4
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub (
#159 (comment)
) , or unsubscribe (
https://github.com/notifications/unsubscribe-auth/AAECBJ73HEQJINPFJTEOZYLRSMIQXANCNFSM4EJ7JGDQ
).
|
The name of packet [82] (https://github.com/Pryaxis/TerrariaAPI-Server/blob/28200f7d4447ee275e85856f97eee58fa156ffc4/TerrariaServerAPI/TerrariaApi.Server/PacketTypes.cs#L88) is misleading - it does not cause any netmodule to be loaded, as netmodules are loaded on server startup when
NetManager.Initialize()
is called.It's an issue rather than PR as we should decide on which name will fit the most, as we'd like to avoid incidents like d8c7be1, da8fded and 28200f7 due to poor communication of the change.
I personally opt to rename it to simply NetModule, as that's exactly what it contains. It's a minor change, though.
The text was updated successfully, but these errors were encountered: