TSAPI
TSAPI copied to clipboard
Pryaxis
Rename packet 82
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 d8c7be19843605a28d7f643652947f8dd87b9a2d, da8fdeddbd1857e241490c0c310857b9f2371caa and 28200f7d4447ee275e85856f97eee58fa156ffc4 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.
Don't know what a better name is than "LoadNetModule" and "NetModule" isn't a verb either.
Perhaps ReadNetModule? It just triggers a NetModule to be loaded from the stream.
Perhaps ReadNetModule? It just triggers a NetModule to be loaded from the stream.
If that's the case, is LoadNetModule not accurate?
Our PacketTypes.cs naming contains a mix of Verb ActualPacketName and plain ActualPacketName
(e.g., Create CombatText, Complete AnglerQuest, etc.)
I don't really see any renaming that provides extra clarity (from the suggestions provided thus far)
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.
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.
I forgot this was this big of a bikeshed
Lucas Nicodemus http://keybase.io/xn
On Tue, May 19 2020 at 16:26, quake1337 < [email protected] > 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 ( https://github.com/Pryaxis/TerrariaAPI-Server/issues/159#issuecomment-631139636 ) , or unsubscribe ( https://github.com/notifications/unsubscribe-auth/AAECBJ73HEQJINPFJTEOZYLRSMIQXANCNFSM4EJ7JGDQ ).