Closed Nefarion closed 3 years ago
Was looking through the code out of curiosity since I had a traffic simulation project in university. This is exactly the PR that starts solving the issue I wanted to open just now.
Currently it's impossible to figure out what is going on with all the magic numbers, deep if/else trees etc. I wish more things would be moved into methods with descriptive names so that people could understand what the code does without having to read every single line.
Was the original code decompiled, or how come that variables are all named num1
, num2
etc?
Anyway, big :+1:
@Nefarion You know, "Redundant Qualifiers" are not necessarily bad. You can notice that, even in classes fully created by me, I do use the "this" keyword and the class name before static methods. It's a matter of legibility. The "this" keyword makes the class members stand out from the local variables and the class name for static methods clearly tells me that the method is static - and this is even more important when the static method or class member is inherited and not present in the file.
Though, in this case, the "this" is not entirely necessary since members are preceded by "m_", I would appreciate if you kept the class names before the static calls. (Note that I don't use them on constants because they stand out for themselves, being all caps)
Sealed classes are also something I want to avoid, unless it proves to have a significant effect on performance. We can't predict the future of this project nor how it will be used and I don't want to stop people from referencing this mod and extend these classes.
Still, you're doing great work and I really appreciate it! I'm going to have all kinds of troubles merging this with my changes but I'll deal with that. Let me know when you're done for me to fully review it. Didn't you have an exam tomorrow, though? Shouldn't you be studying? :P
@TimBeyer Not necessarily impossible, it's just in hardcore mode :D Yes, it's decompiled code.
Yeah, i am studying :> if you prefer to leave them in, i can just revert the commit.
(I started with a base performance of 2.5ms - 3ms for Pathfinding)