Comparing Quake3 Original and Kenny Edition Codebases

Quake III Arena is a multiplayer-focused first-person shooter video game released in December 1999. The game was developed by id Software. Quake III was a very popular game and even now it’s still popular and many gamers enjoy with it.

The original source code of Quake3 can be found here .

Artem Kharytoniuk decide to create it’s own version  and his goal is described in it’s repo:

This repository contains updated version of the original Q3 codebase with reorganized code structure, compatibility fixes, build setup for the latest Visual Studio and modifications that update the core tech but preserve original gameplay, look and feel.

Let’s compare the source code of the two projects  and discover what improvements were down in the Kenny edition version.

The whole picture

Here’s a summary of some metrics of the original code

quake4

And here’s the summary of the modified version

quake1

Here are some remarks after comparing their summaries:

  • Less code in the modified version.
  • Less types, methods and variables.
  • Rating and debt very similar for the whole solution.
  • Similar method complexity.

Let’s discover these metrics for each project:

The original code

quake2

And for the modified code

quake3

Let’s go further and compare deeply the differences between the two versions.

Modularity

Modularity is a software design technique that increases the extent to which software is composed from separate parts , you can manage and maintain modular code easily.

For procedural language like C  where no logical artifacts like namespace, component or class does not exist, we can modularize by using directories and files.

In the modified version, there are some refactoring concerning the folders structure. here’s an example of the refactoring where the folders related to the engine are isolated in the engine directory.

 

quake5

Encapsulation

Encapsulation  is the hiding of functions and data which are internal to an implementation.  In C, encapsulation is performed by using the keyword static . These entities are called file-scope functions and variables.

Let’s search for all static functions for the original code:

quake6

Many methods are declared as static to enforce the encapsulation. However there are many other ones not declared as static.

We can use the Metric view to have a good idea how many functions are static. In the Metric View, the code base is represented through a Treemap. Treemapping is a method for displaying tree-structured data by using nested rectangles. The tree structure used in a CppDepend treemap is the usual code hierarchy:

  • Projects contains directories.
  • Directories contains files.
  • Files contains struects, functions and variables.

The treemap view provides a useful way to represent the result of a CQLinq request, so we can visually see the types concerned by the request.

quake7

As we can observe almost all the functions from renderer and cgame projects are declared as static, which is not the case for the other projects where only few functions are declared as static.

Did the modified version enforce the encapsulation?

Here’s the CQLinq result of the static functions query:

quake8

There are less static functions than the original code, mainly because many functions were removed due to the refactoring, and this new version does not enforce more than the original one the encapsulation.

Usage of structs to store the data model

In C programing the functions uses variables to acheive their treatments, theses variables could be:

  • Static variables.
  • Global variables.
  • Local variables
  • Variables from structs.

Each project has it’s data model which could be used by many source files, using global variables is a solution but not the good one, using structs to group data is more recommended.

Let’s search for global variables with a primitive type:

quake11

Only 166 variables from 9084 fields could be refactored to make them const, static or embed them in a struct.

And here’s the same quey for the modified code

quake12

few new global variables candidate to be refactored are added to the new code. For that we can search for the primitive global variables , not static , not const and not changed in the source code:

quake22

some of them like  multi_texture_add_frag_spv_size or multi_texture_clipping_plane_vert_spv_size  added in the code could be declared as const.

Code Smells in two projects

Types with too many fields

Let’s search for structs with more than 30 fields:

quake13

And here’s the result for the modified version

quake14

Some big structs were refactored, for example the number of fields of cgMedia_t passed 258 from to 199 fields.

Too big functions

Here’s from the linux coding style web page, an advice about the length of functions:

Functions should be short and sweet, and do just one thing.  They should
fit on one or two screenfuls of text (the ISO/ANSI screen size is 80x24,
as we all know), and do one thing and do that well.

The maximum length of a function is inversely proportional to the
complexity and indentation level of that function.  So, if you have a
conceptually simple function that is just one long (but simple)
case-statement, where you have to do lots of small things for a lot of
different cases, it's OK to have a longer function.

Let’s search for functions where the number of lines of code is more than 50 for the original code:

quake23

And here’s the same result for the modified version:

quake16

Many big functions are gone, some of them are completly removed and the other ones are refactored.

Functions with too many parameters

Functions where NbParameters > 8 might be painful to call  and might degrade performance. Another alternative is to provide  a structure dedicated to handle arguments passing.

Here are the functions concerned for the original code

quake18

And here’s the result for the modified version:

quake17

only very few functions has more than 8 parameters in both versions.

Functions with too many  local variables

Methods where NbVariables is higher than 8 are hard to understand and maintain. Methods where NbVariables is higher than 15 are extremely complex and should be split in smaller methods (except if they are automatically generated by a tool)

Here’s the result for the original code:

quake20

And the result for the modified one

quake21

Some added functions in the new modified version have more thann 15 variables, vk_initialize() is an added function with 73 ariables.

Functions too complex

Many metrics exist to detect complex functions, NBLinesOfCode,Number of parameters and number of local variables are the basic ones.

There are other interesting metrics to detect complex functions:

  • Cyclomatic complexity is a popular procedural software metric equal to the number of decisions that can be taken in a procedure.
  • Nesting Depth is a metric defined on methods that is relative to the maximum depth of the more nested scope in a method body.
  • Max Nested loop is equals the maximum level of loop nesting in a function.

The max value tolerated for these metrics depends more on the team choices, there’s no standard values.

Let’s search for functions candidate to be refactored for the original code:

quake24

And the modified one:

quake25

 

We can observe that many complex functions are removed or refactored.

 Conclusion

The original source code of Quake III is well implemented, few code smells are detected. However a big clean was introduced in the modified version where many functions and variables were removed, some structs are refactored and the physical structure a little changed.