Write the code as if it would be accompanied by a violent psychopath who knows where you live.
... even if the project is not planned to be developed and you are not going to share the source codes, because in 20 years some maniac will study and modify the machine code of your product, and he may want to find you.
In general, I rarely play computer games. Happened, not played for several years in a row. But sometimes a small reverse engineer wakes up in me who motivates me to get into the machine code of some favorite toy from the past. In the past year, I worked on refining Need For Speed ​​III: Hot Pursuit (1998) . This is my favorite game in the genre, but now I, unfortunately, know how disgusting it is written. A large number of small bugs in the most unexpected places are a direct consequence of the poor quality of the code.
In the executable file, a huge amount of code that was inherited from the previous parts of the game and is not used, that is, the outdated code was not removed by the developers, and I had cases when some outdated code was called, but the results of its work were ignored, because the updated code they were no longer needed. In the game, static fixed-size arrays are used everywhere, in many cases there are no checks for going beyond the array, which leads to crashes when some reserved amount of memory is not enough. The game uses a large number of dirty hacks. For example, the race function for the reflected route variant is implemented not by the reflection of the model of the route when it is loaded, but by flipping each frame when rendering, with the inversion of the left and right buttons and a number of similar substitutions where the developers did not forget to add the corresponding code. Because of this, the inscriptions on the cars (for example, cops) are reflected, and during the races the cops confuse the “right” and “left” in the negotiations on the radio. There are also errors that are clearly related to the use of magic numbers in the code instead of the named constants, when the value of the constant in the development process was changed, but there was code that uses the old value and does not work correctly. But one case turned out to be so funny that I wanted to share it within a small note.
')
In the game there was one minor bug associated with indents in the drop-down menus. For example, if the Hot Pursuit mode was selected, the left indent of all top drop-down lists increased significantly without an obvious need.
I suggested that this is due to an error in the code, which adds space for displaying icons of the passage in the drop-down list of tracks, which should be displayed only in the Hot Pursuit mode.
Regular drop-down lists also had problems with indents depending on the mode, but not in such an explicit form. Since I was just digging along with the corresponding code, I decided to correct this strange behavior at the same time.
It is worth noting that the code of the game does not contain traces of things like inheritance, so most likely it was written in pure C. The game implements code that reads lists of menu items and their properties from external text files, but developers were often lazy to add support for a new property, and wrote additional code, relying simply on the name of the element. For example, the output code of drop-down lists itself checks the name of the current element and some other variables, and depending on this it can apply some additional logic.
So it turned out here. Since the menu uses two different types of drop-down lists (regular and top), the entire code for working with them was completely duplicated twice. In addition, it turned out that the left indent is selected by different code fragments when calculating the left and right borders of the drop-down list. Total - in theory, we should have 4 copies of the same code. No matter how wrong!
Option 1 (when calculating the indent of the left border of the usual dropout):
On C, it looked like this:
Option 2 (when calculating the indent of the right border of a normal dropout):
On C, it looked like this:
Options 3 and 4 (when calculating the indentation of the left and right borders of the dropout under the title):
On C, it looked like this:
All options together (for clarity):
What is the correct option? The answer is: none! Only if we combine all the checks together can we get the only correct version of the code, and it would look like this:
In total, we have 4 variants of the same code, with 3 different errors made in different variants! Just a unique case and a great demonstration of why copy-paste is a bad idea.
How changes are made to the machine code, I wrote earlier . To fix the problem, I wrote one function:
All the above code snippets have been replaced by a call to this function. Now the indents in the lists are selected correctly :) And this is just one of more than 200 changes that were made in the patch . The described change is actually one of the smallest, but the error itself, in my opinion, was interesting (as a demonstration of the harm from the corresponding anti-pattern).
And now think. If an impostor was found here who undertook to fix other people's bugs in the program without source codes without asking, there may be someone who will be so angry that he will have a desire to find a developer or his relatives. And you need it? Write a quality code :)
In general, I rarely play computer games. Happened, not played for several years in a row. But sometimes a small reverse engineer wakes up in me who motivates me to get into the machine code of some favorite toy from the past. In the past year, I worked on refining Need For Speed ​​III: Hot Pursuit (1998) . This is my favorite game in the genre, but now I, unfortunately, know how disgusting it is written. A large number of small bugs in the most unexpected places are a direct consequence of the poor quality of the code.
How bad is it?
In the executable file, a huge amount of code that was inherited from the previous parts of the game and is not used, that is, the outdated code was not removed by the developers, and I had cases when some outdated code was called, but the results of its work were ignored, because the updated code they were no longer needed. In the game, static fixed-size arrays are used everywhere, in many cases there are no checks for going beyond the array, which leads to crashes when some reserved amount of memory is not enough. The game uses a large number of dirty hacks. For example, the race function for the reflected route variant is implemented not by the reflection of the model of the route when it is loaded, but by flipping each frame when rendering, with the inversion of the left and right buttons and a number of similar substitutions where the developers did not forget to add the corresponding code. Because of this, the inscriptions on the cars (for example, cops) are reflected, and during the races the cops confuse the “right” and “left” in the negotiations on the radio. There are also errors that are clearly related to the use of magic numbers in the code instead of the named constants, when the value of the constant in the development process was changed, but there was code that uses the old value and does not work correctly. But one case turned out to be so funny that I wanted to share it within a small note.
')
How many errors can be in 4 variants of the same code?
In the game there was one minor bug associated with indents in the drop-down menus. For example, if the Hot Pursuit mode was selected, the left indent of all top drop-down lists increased significantly without an obvious need.
I suggested that this is due to an error in the code, which adds space for displaying icons of the passage in the drop-down list of tracks, which should be displayed only in the Hot Pursuit mode.
Regular drop-down lists also had problems with indents depending on the mode, but not in such an explicit form. Since I was just digging along with the corresponding code, I decided to correct this strange behavior at the same time.
It is worth noting that the code of the game does not contain traces of things like inheritance, so most likely it was written in pure C. The game implements code that reads lists of menu items and their properties from external text files, but developers were often lazy to add support for a new property, and wrote additional code, relying simply on the name of the element. For example, the output code of drop-down lists itself checks the name of the current element and some other variables, and depending on this it can apply some additional logic.
So it turned out here. Since the menu uses two different types of drop-down lists (regular and top), the entire code for working with them was completely duplicated twice. In addition, it turned out that the left indent is selected by different code fragments when calculating the left and right borders of the drop-down list. Total - in theory, we should have 4 copies of the same code. No matter how wrong!
Option 1 (when calculating the indent of the left border of the usual dropout):
On C, it looked like this:
dw_padding = (stricmp(str_element_name, "tracks") == 0 && dw_cfg_race_type == 3) ? 35 : 15; Option 2 (when calculating the indent of the right border of a normal dropout):
On C, it looked like this:
dw_padding = (stricmp(str_element_name, "tracks") == 0 || stricmp(str_element_name, "rectrk") == 0) ? 35 : 15; Options 3 and 4 (when calculating the indentation of the left and right borders of the dropout under the title):
On C, it looked like this:
dw_padding = (dw_cfg_race_type == 3) ? 35 : 15; All options together (for clarity):
dw_padding = (stricmp(str_element_name, "tracks") == 0 && dw_cfg_race_type == 3) ? 35 : 15; dw_padding = (stricmp(str_element_name, "tracks") == 0 || stricmp(str_element_name, "rectrk") == 0) ? 35 : 15; dw_padding = (dw_cfg_race_type == 3) ? 35 : 15; What is the correct option? The answer is: none! Only if we combine all the checks together can we get the only correct version of the code, and it would look like this:
dw_padding = (dw_cfg_race_type == 3 && (stricmp(str_element_name, "tracks") == 0 || stricmp(str_element_name, "rectrk") == 0)) ? 35 : 15; In total, we have 4 variants of the same code, with 3 different errors made in different variants! Just a unique case and a great demonstration of why copy-paste is a bad idea.
How was this fixed?
How changes are made to the machine code, I wrote earlier . To fix the problem, I wrote one function:
All the above code snippets have been replaced by a call to this function. Now the indents in the lists are selected correctly :) And this is just one of more than 200 changes that were made in the patch . The described change is actually one of the smallest, but the error itself, in my opinion, was interesting (as a demonstration of the harm from the corresponding anti-pattern).
Findings?
And now think. If an impostor was found here who undertook to fix other people's bugs in the program without source codes without asking, there may be someone who will be so angry that he will have a desire to find a developer or his relatives. And you need it? Write a quality code :)
Source: https://habr.com/ru/post/302570/
All Articles