2 hours ago, JoeJ said:
IIRC Carmacks valid argument here is: If piece of code is used only once and in one place, there is no need to clutter it to smaller functions - it becomes just more likely to introduce bugs and harder to read.
"Harder to read" is quite subjective.
Having worked on code written this way, and having refactored said code to use multiple smaller functions, I find it much easier to follow the code blocks when they were shorter. It is claimed that large, monolithic functions are better because they don't introduce unnecessary boilerplate, which is true - but this ignores the fact that the "boilerplate" can be hugely useful when actually navigating the code after the fact - which is important because on large legacy projects we tend to spend more time navigating code than actually writing it. The cost of writing the boilerplate is negligible over the lifetime of the project and in the case of simply breaking a function out into multiple functions, does not generally contribute to code bloat if it is done at the proper granularity level, unlike certain other kinds of boilerplate that do little more than add layers of abstraction. Breaking a monolithic function into smaller functions is a transformation that adds value in the long term.
Modern languages also allow us to make "local functions" - in C++, through the use of local lambdas with captures. One can contain any "clutter" that results from breaking a monolithic function down into sub-functions within the monolithic function itself, rather than polluting the local namespace with external functions, so that particular argument (which I know wasn't mentioned here, but I've heard it argued) doesn't really need to apply anymore.
2 hours ago, JoeJ said:
Keeping out commented code is no bad practice either. Better than just leaving it in while unused, better than rewriting it again if it turns out you need it, and it reminds yourself of what have been issues or alternatives. Probably they had better things to do than removing this before they released it to the public so we can learn.
I fundamentally disagree. Commented out code is, at best, an inferior way to duplicate the functionality of your version control system, and outright misleading at worst. Pray tell, do you actually maintain your commented out code through refactorings and bug fixes, "just in case"? If you really need that code back, you can just go back through your version control's commit/checkin history for the file that contained it. In my experience, chances are good that if you need the same functionality again later, the context will have changed sufficiently that the original code would not work as written, anyway.
I'd also argue that leaving commented code around "just in case it's useful" could contribute to perpetuating a culture of copy-and-paste programming, which I feel most of us can agree is not a culture conducive to producing maintainable code.
2 hours ago, JoeJ said:
And finally, code is not good because it looks beautiful or modern. It is good if it works, is efficient and the number of bugs is small enough. That's the case here. It's also readable.
"Good code" has come to mean all of what you've said plus "code that can be maintained long term without your successors wanting to shoot you in the face." Does the Quake engine meet that specification?
If your code works, is efficient, and bug-free, but difficult to navigate and modify without breaking stuff, you have not written "good" code. You have at best written "throwaway code that gets the job done." In a world where the industry is moving towards "games as a service" and where at the very least, most major games get patched, extra content, or even outright sequels using the same codebase, I would say that throwaway code of any sort is no longer acceptable. I confess I've been forced to write throwaway code to solve a problem at the last minute like I imagine most programmers have, but I felt really dirty doing it and was constantly aware of how terrible it was that I was doing it.
edit: I was going to say something on the subject of global state and how it can hurt you in an increasingly parallel world, but others have made that argument in the past and I don't think we need to rehash it for the nth time. Having maintained large C++ codebases where global state was the standard way of doing things, even in a single threaded environment they've caused me more pain than I felt they were worth.