[Idea/Proposal] Refactoring

[milutinke]

Greetings!

I have a proposal.

About

As the code base has gotten bigger, it's starting to look really crammed, especially ProtocolHandler18.cs and the re-usability of the project is hard without ripping out a lot of code.

My proposal is to start working on a draft for a refactor and then proceed to refactor the project once the draft phase is done.

In this text I'll present some personal ideas, which are the first-pass ideas and should be discussed, feedback is needed, especially from more experienced developers.

Purpose/Motivation

This refactor would enable the app to be embedded/re-used in different environments and the maintenance would be easier.

Top-Level Diagram

Proposals

Protocol

Protocol part of the program should be it's own library with a good structure for ease of maintenance

The structure of the Protocol library would be:

• Protocol Interface (API)
• Protocol Registry (Would hold registries of packets for each version of the protocol)
• Registry_1_XX_XX (Would hold packets and their IDs for this specific version of the protocol)
• Packet classes (Would have write and read methods)

What it needs to be discussed is: Should we make a separate class for the each version of the packet or just use a single class?

---

Authentication

Authentication part of the program should also be ported to a separate library

The purpose of separation into a separate library would be re-usability.

Mojang authentication should be removed, since it's no longer working.

---

Program

Program class should be switched with a Client Builder:

The purpose of this class would be to build and configure a client, the configuration would be injected from the class using the builder, this would enable embedding the app into different environment, like Console, GUI and Mobile apps, etc..

Example of a class that would be injected externally would be the Logger.

Client class should delegate Commands and Chat Bots management/handling into separate classes like Command Manager and ChatBot Manager, the usage of the Client API should be done through composition like it's done currently.

---

Command Manager

For command management, I think we should use a port of Mojang/Brigadier for C# called Brigadier.NET which would enable making of complex commands and ease of their maintenance.

---

Chat Bot Manager

Should manage Chat Bots

---

Chat Bot API

Would pretty much be the same with some minor tweaks and addition of configuration handling, Chat Bots should have their own configuration separate of the client.

As how to configure them and how to store the configurations, this should be discussed.

---

Additional stuff

I think that we should have a separate repository for each of the libraries and the implementations, the current repository should be the Console Implementation, for GUI and Mobile app, they should be in separate repositories.

As for how to handle issues, this needs to be discussed.

[ORelio]

Hi,

Thanks for your continued work on MCC. I agree that many part of the code base should be modernized.

Your diagram looks good to me. Having a library for core functionality would facilitate integration in different applications (PC, mobile, custom work by other people, etc.)

Regarding protocol packets

What it needs to be discussed is: Should we make a separate class for the each version of the packet or just use a single class?

• Making a separate class for each version facilitates comparison with the protocol wiki, but may create more work if making a change that requires updating all versions of the packet. Plus, many packets stay the same between versions, so many classes will be a copy-and-paste of the previous one.
• Making a single class for each packet facilitates working on the packet but makes the code less readable.

I'm personally not sure which option is the best, so I'd suggestion you to pick the one you are more comfortable to work with.

Regarding Authentication:

• Making a reliable authentication mechanism is now much more complex with Microsoft API, so indeed, having that code as a library that may serve other projects is better for maintainability (plus, other users of the library might contribute).

Mojang authentication should be removed, since it's no longer working.

• I managed to log in with a legacy account right now (Legacy Minecraft account, not Mojang account), so I'm not sure whether that code should be removed yet. But indeed, that's legacy code. Maybe it's not necessary to move it the the library.

Regarding commands:

• I wrote custom code for managing internal commands, which, well, works, but is limited in functionality. Feel free to replace it with a full-featured command library if you feel it more appropriate 👍

Regarding configuration:

As how to configure them and how to store the configurations, this should be discussed.

• Maybe this could be addressed in the new TOML configuration file (#2225, #2237)

Regarding repositories:

I think that we should have a separate repository for each of the libraries and the implementations, the current repository should be the Console Implementation, for GUI and Mobile app, they should be in separate repositories. As for how to handle issues, this needs to be discussed.

Indeed, we could make separate repositories and put issues in each repository. Note that users may be unsure where is the issue (e.g. in GUI or in Network code) so issues may need to be transferred between repositories after triaging.

[milutinke]

Thanks for the feedback. As for making Packet classes, I think we should go with the each packet version in a separate class, as it would be a lot more readable, some packets are really hard to read, the problem of duplicate code could be solved by simply referencing the existing class in a newer protocol packet palette and overriding it's id if it has changed. This would also enable easier finding and solving of bugs in my opinion, as I had to spend a lot of time going through wiki and even the game code for multiple versions of the game in some cases to fix some bugs which either were not noticed before or made during updates of the packet.

For issues, people will 100% be confused, but it's not a big deal to transfer the issue in my opinion. We could partially lessen the confusion by having notes on where to submit which category in each implementation of the client and in each repository for it and on the website.

[ORelio]

the problem of duplicate code could be solved by simply referencing the existing class in a newer protocol packet palette and overriding it's id if it has changed

Looks good to me. Go for it 👍

[bunnyi116]

1. You are advised to separate the protocol library from the console client first
2. Make a client public API interface, use C# events, and let the client subscribe to events by itself. Interface properties are categorized, Example: Server: GetServerPort(), GetServerHost(), GetServerTPS() ,...... Player: GetCurrentLocation(), GetYaw(), GetPitch(), GetHealth() ,....... And so on to make the code more readable and better to use.

[milutinke]

1. You are advised to separate the protocol library from the console client first

2. Make a client public API interface, use C# events, and let the client subscribe to events by itself. Interface properties are categorized,
   Example:
   Server: GetServerPort(), GetServerHost(), GetServerTPS() ,......
   Player: GetCurrentLocation(), GetYaw(), GetPitch(), GetHealth() ,.......
   And so on to make the code more readable and better to use.

That is my plan, I want to make it so anyone could use it in their projects and that you can use whatever you need from the library by subscribing to events/delegates instead of having to extend a class or implement like 500 different interfaces. I've spent yesterday learning about C# delegates and events in depth for this purpose. I am currently busy, when I have some time 'll start drawing some more detailed diagrams for feedback.