Kromster80 / kam_remake

"KaM Remake" is an RTS game remake written in Delphi from scratch.
http://www.kamremake.com
GNU Affero General Public License v3.0
360 stars 90 forks source link

KM_Networking unit refactoring #208

Open reyandme opened 7 years ago

reyandme commented 7 years ago

It's too big, so hard to maintain and use it. I can separate this work into 2 parts:

  1. Split method PacketRecieve into many small methods. Its about 1000 lines now. Just because its easier and faster

  2. Split KM_Networking further. We can extract message handling logic from there to message handlers (new entity). Then KM_Networking will just dispatch messages, not handle them. Also some messages are used only by Host, some only by joiners, some by both, then different handlers could manage them.

@Kromster80 @lewinjh What do you think?

lewinjh commented 7 years ago

Yes it definitely needs refactoring, Krom and I planned to do it ages ago but never did.

Splitting into handlers might help. One of the main problems I see is that there's a lot of "global state" variables in TKMNetworking (examples: fVoteReturnToLobbySucceeded, fMissingFileType, fReturnedToLobby, fEnteringPassword, fIgnorePings). This code is fragile and hard to understand. There's no clear transition from one state to another and all these variables are available to the entire TKMNetworking class, not just the part that cares about them.

Maybe we could take this into account with the way we split it. Try to move some of the state variables into subclasses where possible (e.g. all of the lobby specific variables and packet handlers could be in some kind of lobby handler subclass).

I'm not really sure what the best way to approach it is. There's a lot of fragile subtle things about the way it works (relying on doing things in specific orders and packets being sent/received in the right order for the correct chain of events to occur).