-
-
Notifications
You must be signed in to change notification settings - Fork 434
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
General refactor of MTA's code #3158
General refactor of MTA's code #3158
Conversation
i took a look at what did you do and i have few things to say: - reply << (unsigned char)(temp.str().length() + 1);
+ reply << (uchar)(temp.str().length() + 1); we should probably use types from https://en.cppreference.com/w/cpp/types/integer if we are going to refactor something, so in this case uchar should be uint8_t I'm not good at windows headers so i have a bit concern about changes you have made regarding that If function has depth of 1, maybe 2 levels we probably should keep it as is. Even if you compiled it, and it works, you should probably try to run some bigger gamemode to test more scenarious I think if you are doing such big refactor, split it into smaller parts, sections or however you name them, so in one pr you refactor all types like "unsigned short" to "ushort", in the next you decreasing depth of functions ect. it's a general good advice so reviewers have easier time |
well, then just a quick change from all |
The derefencing such as |
I agree on this one, it's not clear 100% clear what |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is too big of a PR....
And I've been reviewing it for a good 20-30 minutes
5853ba4
to
e2c1789
Compare
856f7eb
to
577832b
Compare
This draft pull request is stale because it has been open for at least 90 days with no activity. Please continue on your draft pull request or it will be closed in 30 days automatically. |
This draft pull request was closed because it has been marked stale for 30 days with no activity. |
I changed most of
#ifdef WIN32
to#ifdef _WIN32
becauseWIN32
was not always defined in some files._WIN32
however is always active