# Amnesia: The Dark Descent or how to forget to fix copy paste Just before the release of the "Amnesia: Rebirth" game, the vendor "Fractional Games" opened the source code of the legendary "Amnesia: The Dark Descent" and its sequel "Amnesia: A Machine For Pigs"\. Why not use the static analysis tool to see what dreadful mistakes are hidden in the inside of these cult horror games? ![0769_Amnesia_The_Dark_Descent/image1.png](https://import.viva64.com/docx/blog/0769_Amnesia_The_Dark_Descent/image1.png) After seeing the news on Reddit that the source code of the games "[Amnesia: The Dark Descent](https://github.com/FrictionalGames/AmnesiaTheDarkDescent)" and "[Amnesia: A Machine for Pigs](https://github.com/FrictionalGames/AmnesiaAMachineForPigs)" was released, I couldn't pass by and not check this code using PVS\-Studio, and at the same time write an article about it\. Especially since the new part of this game\-series \- "[Amnesia: Rebirth](https://store.steampowered.com/app/999220/Amnesia_Rebirth/)" is released on the 20th of October \(and at the moment of posting this article the game is already released\)\. "Amnesia: The Dark Descent" was released in 2010 and became a cult game in the survival horror genre\. Frankly speaking, I have never been able to play through it, even a bit\. The reason is that in horror games I play by one algorithm: install, run for five minutes, exit by "alt\+f4" at the first creepy moment and delete the game\. But I liked watching this game run through videos on YouTube\. In case someone is not familiar with PVS\-Studio yet, this is a static analyzer that looks for errors and suspicious places in source code of programs\. [![getTrialImageLink](https://wcdn.pvs-studio.com/media/get_trial_insert.png)](https://pvs-studio.com/en/pvs-studio-download/) I especially like delving into source code of games\. So, if you are interested in what errors are made in games, you can read my previous articles\. Also check out the [articles by my colleagues](https://pvs-studio.com/en/blog/?tag=gamedev) about checking source code of games\. After checking, it turned out that a large amount of code overlaps between "The Dark Descent" and "A Machine For Pigs", and the reports for these two projects were very similar\. So almost all the errors that I will cite further take place in both projects\. The better half of errors found by the analyzer in these projects was copy\-paste errors\. This explains the title of the article\. The main reason for these errors is the "[last line effect](https://pvs-studio.com/en/blog/posts/csharp/0708/)"\. Let's get right to it\. ## Copy\-paste errors There were a lot of suspicious places that looked like inattentive copying\. Some cases may be due to the internal logic of the game itself\. Once they confused both the analyzer and me, at least a comment could be of use\. After all, other developers may be as clueless as I am\. **Fragment 1\.** Let's start with an example where the entire function consists of comparing method results and fields values of two objects _aObjectDataA_ and _aObjectDataB_\. I will cite the entire function for clarity\. Try to see for yourself where the error was made in the function: ```cpp static bool SortStaticSubMeshesForBodies(const ....& aObjectDataA, const ....& aObjectDataB) { //Is shadow caster check if( aObjectDataA.mpObject->GetRenderFlagBit(....) != aObjectDataB.mpObject->GetRenderFlagBit(....)) { return aObjectDataA.mpObject->GetRenderFlagBit(....) < aObjectDataB.mpObject->GetRenderFlagBit(....); } //Material check if( aObjectDataA.mpPhysicsMaterial != aObjectDataB.mpPhysicsMaterial) { return aObjectDataA.mpPhysicsMaterial < aObjectDataB.mpPhysicsMaterial; } //Char collider or not if( aObjectDataA.mbCharCollider != aObjectDataB.mbCharCollider) { return aObjectDataA.mbCharCollider < aObjectDataB.mbCharCollider; } return aObjectDataA.mpObject->GetVertexBuffer() < aObjectDataA.mpObject->GetVertexBuffer(); } ``` Here's a pic for you to avoid accidentally spying on the answer: ![0769_Amnesia_The_Dark_Descent/image2.png](https://import.viva64.com/docx/blog/0769_Amnesia_The_Dark_Descent/image2.png) Did you find a bug? So, in the last _return_, there is a comparison using _aObjectDataA_ on both sides\. Note that all the expressions in the original code were written in a line\. Here I broke lines so that everything fit exactly in the width of the line\. Just imagine how hard it will be to look for such a flaw at the end of the working day\. Whereas the analyzer will find it immediately after building the project and running incremental analysis\. [V501](https://pvs-studio.com/en/docs/warnings/v501/) There are identical sub\-expressions 'aObjectDataA\.mpObject\-\>GetVertexBuffer\(\)' to the left and to the right of the '<' operator\. WorldLoaderHplMap\.cpp 1123 As a result, such an error will be found almost at the time of writing the code, instead of hiding in the depths of the code from several stages of Q&A, making your search much more difficult\. > Note by my colleague Andrey Karpov\\\. Yes, this is a classic "last line effect" error\\\. Moreover, this is also a classic pattern of the error related to comparing two objects\\\. See the article "\[The Evil within the Comparison Functions\]\(https://pvs\-studio\.com/en/blog/posts/cpp/0509/\)"\\\. **Fragment 2\.** Let's take a quick look at the code that triggered the warning: ![0769_Amnesia_The_Dark_Descent/image4.png](https://import.viva64.com/docx/blog/0769_Amnesia_The_Dark_Descent/image4.png) Here is a screenshot of the code for clarity\. This is how the warning looks like: [V501](https://pvs-studio.com/en/docs/warnings/v501/) There are identical sub\-expressions 'lType \=\= eLuxJournalState\_OpenNote' to the left and to the right of the '\|\|' operator\. LuxJournal\.cpp 2262 The analyzer found that there is an error in the check of the _lType_ variable value\. The equality with the same element of the _eLuxJournalState\_OpenNote_ enumerator is checked twice\. First, I wish this condition would be written in a table\-like format for better readability\. See the chapter 13 from the mini\-book "[The ultimate question of programming, refactoring, and everything](https://pvs-studio.com/en/blog/posts/cpp/0391/)" for more details\. ```cpp if(!( lType == eLuxJournalState_OpenNote || lType == eLuxJournalState_OpenDiary || lType == eLuxJournalState_OpenNote || lType == eLuxJournalState_OpenNarratedDiary)) return false; ``` In this form, it becomes much easier to notice the error even without the analyzer\. Anyway, here comes a question \- does such an erroneous check lead to logic distortion of the program? After all, one may need to check some other _lType_ value, but the check was missed due to a copy\-paste error\. So, let's take a look at the enumeration itself: ```cpp enum eLuxJournalState { eLuxJournalState_Main, eLuxJournalState_Notes, eLuxJournalState_Diaries, eLuxJournalState_QuestLog, eLuxJournalState_OpenNote, eLuxJournalState_OpenDiary, eLuxJournalState_OpenNarratedDiary, eLuxJournalState_LastEnum, }; ``` There are only three values with the word "Open" in their name\. All three are present in the check\. Most likely, there is no logic distortion here, but we can hardly know for sure\. So, the analyzer either found a logical error that the game developer could fix, or found an "ugly" written snippet that would be worth rewriting for better elegancy\. **Fragment 3\.** The following case is generally the most obvious example of a copy\-paste error\. [V778](https://pvs-studio.com/en/docs/warnings/v778/) Two similar code fragments were found\. Perhaps, this is a typo and 'mvSearcherIDs' variable should be used instead of 'mvAttackerIDs'\. LuxSavedGameTypes\.cpp 615 ```cpp void cLuxMusicHandler_SaveData::ToMusicHandler(....) { .... // Enemies //Attackers for(size_t i=0; iGetEntityByID(mvAttackerIDs[i]); if(....) { .... } else { Warning("....", mvAttackerIDs[i]); } } //Searchers for(size_t i=0; iGetEntityByID(mvSearcherIDs[i]); if(....) { .... } else { Warning("....", mvAttackerIDs[i]); } } } ``` In the first loop, the _pEntity_ pointer \(obtained via _mvAttackerIDs_\) is handled\. If the condition is not met, a debug message is issued for the same _mvAttackerIDs_\. However, in the next loop, which is formed the same as the previous code section, _pEntity_ is obtained using _mvSearcherIDs_\. While the warning is still issued with the mention of _mvAttackerIDs_\. Most likely, the code block with the note "Searchers" was copied from the "Attackers" block, _mvAttackerIDs_ was replaced with _mvSearcherIDs_, but the _else_ block was not changed\. As a result, the error message uses an element of the wrong array\. This error does not affect the logic of the game, but this way you can play a dirty trick on a person who will have to debug this place, and waste time working with the wrong _mvSearcherIDs_ element\. ![0769_Amnesia_The_Dark_Descent/image5.png](https://import.viva64.com/docx/blog/0769_Amnesia_The_Dark_Descent/image5.png) **Fragment 4\.** The analyzer indicated the next suspicious fragment with as many as three warnings: * [V547](https://pvs-studio.com/en/docs/warnings/v547/) Expression 'pEntity \=\= 0' is always false\. LuxScriptHandler\.cpp 2444 * [V649](https://pvs-studio.com/en/docs/warnings/v649/) There are two 'if' statements with identical conditional expressions\. The first 'if' statement contains function return\. This means that the second 'if' statement is senseless\. Check lines: 2433, 2444\. LuxScriptHandler\.cpp 2444 * [V1051](https://pvs-studio.com/en/docs/warnings/v1051/) Consider checking for misprints\. It's possible that the 'pTargetEntity' should be checked here\. LuxScriptHandler\.cpp 2444 Take a look at the code: ```cpp void __stdcall cLuxScriptHandler::PlaceEntityAtEntity(....) { cLuxMap *pMap = gpBase->mpMapHandler->GetCurrentMap(); iLuxEntity *pEntity = GetEntity(....); if(pEntity == NULL) return; if(pEntity->GetBodyNum() == 0) { .... } iPhysicsBody *pBody = GetBodyInEntity(....); if(pBody == NULL) return; iLuxEntity *pTargetEntity = GetEntity(....); if(pEntity == NULL) return; // <= iPhysicsBody *pTargetBody = GetBodyInEntity(....); if(pTargetBody == NULL) return; .... } ``` The [V547](https://pvs-studio.com/en/docs/warnings/v547/) warning was issued for the second _pEntity \=\= NULL_ check\. For the analyzer, this check will always be _false_, since if this condition were _true_, the function would exit earlier due to a previous similar check\. The next warning \([V649](https://pvs-studio.com/en/docs/warnings/v649/)\) was issued just for the fact that we have two identical checks\. Usually this case may not be a mistake\. Who knows, may be one part of the code implements the same logic, and another part of the code must do something else based on the same check\. But in this case, the body of the first check consists of _return_, so it won't even get to the second check if the condition is _true_\. By tracking this logic, the analyzer reduces the number of false messages for suspicious code and outputs them only for very strange logic\. The error indicated by the last warning is very similar in nature to the previous example\. Most likely, all checks were duplicated from the first _if\(pEntity \=\= NULL\)_ check, and then the object being checked was replaced with the required one\. In the case of the _pBody_ and _pTargetBody_ objects, the replacement was made, but the _pTargetEntity_ object was forgotten\. As a result, this object is not checked\. If you dig a bit deeper into the code of the example we are considering, it turns out that such an error will not affect the program performance\. The _pTargetBody_ pointer gets its value from the _GetBodyInEntity_ function: ```cpp iPhysicsBody *pTargetBody = GetBodyInEntity(pTargetEntity, asTargetBodyName); ``` The first passed argument here is an unchecked pointer that is not used anywhere else\. Fortunately, inside this function there is a check of the first argument for _NULL_: ```cpp iPhysicsBody* ....::GetBodyInEntity(iLuxEntity* apEntity, ....) { if(apEntity == NULL){ return NULL; } .... } ``` As a result, this code works correctly in the end, although it contains an error\. **Fragment 5\.** Another suspicious place with copy\-paste\! ![0769_Amnesia_The_Dark_Descent/image6.png](https://import.viva64.com/docx/blog/0769_Amnesia_The_Dark_Descent/image6.png) In this method, the fields of the _cLuxPlayer_ class object are zeroed\. ```cpp void cLuxPlayer::Reset() { .... mfRoll=0; mfRollGoal=0; mfRollSpeedMul=0; // <= mfRollMaxSpeed=0; // <= mfLeanRoll=0; mfLeanRollGoal=0; mfLeanRollSpeedMul=0; mfLeanRollMaxSpeed=0; mvCamAnimPos =0; mvCamAnimPosGoal=0; mfRollSpeedMul=0; // <= mfRollMaxSpeed=0; // <= .... } ``` But for some reason, the two variables _mfRollSpeedMul_ and _mfRollMaxSpeed_ are zeroed twice: * [V519](https://pvs-studio.com/en/docs/warnings/v519/) The 'mfRollSpeedMul' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 298, 308\. LuxPlayer\.cpp 308 * [V519](https://pvs-studio.com/en/docs/warnings/v519/) The 'mfRollMaxSpeed' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 299, 309\. LuxPlayer\.cpp 309 Let's look at the class itself and at its fields: ```cpp class cLuxPlayer : .... { .... private: .... float mfRoll; float mfRollGoal; float mfRollSpeedMul; float mfRollMaxSpeed; float mfLeanRoll; float mfLeanRollGoal; float mfLeanRollSpeedMul; float mfLeanRollMaxSpeed; cVector3f mvCamAnimPos; cVector3f mvCamAnimPosGoal; float mfCamAnimPosSpeedMul; float mfCamAnimPosMaxSpeed; .... } ``` Interestingly, there are three similar variable blocks with related names: _mfRoll_, _mfLeanRoll_, and _mvCamAnimPos_\. In _Reset_, these three blocks are reset to zero, except for the last two variables from the third block, _mfCamAnimPosSpeedMul_ and _mfCamAnimPosMaxSpeed_\. Just in place of these two variables, duplicated assignments are found\. Most likely, all these assignments were copied from the first assignment block, and then the variable names were replaced with the necessary ones\. It may be that the two missing variables should not have been reset, but the opposite is also very likely\. In any case, repeated assignments will not be of great help in supporting this code\. As you can see, in a long set of identical actions, you may not notice such an error, and the analyzer helps you out here\. **Fragment 5\.5\.** The code is very similar to the previous one\. Let me give you a code snippet and a warning from the analyzer for it right away\. [V519](https://pvs-studio.com/en/docs/warnings/v519/) The 'mfTimePos' variable is assigned values twice successively\. Perhaps this is a mistake\. Check lines: 49, 53\. AnimationState\.cpp 53 ```cpp cAnimationState::cAnimationState(....) { .... mfTimePos = 0; mfWeight = 1; mfSpeed = 1.0f; mfBaseSpeed = 1.0f; mfTimePos = 0; mfPrevTimePos=0; .... } ``` The _mfTimePos_ variable was set to 0 twice\. As in the previous example, let's get into in the declaration of this field: ```cpp class cAnimationState { .... private: .... //Properties of the animation float mfLength; float mfWeight; float mfSpeed; float mfTimePos; float mfPrevTimePos; .... } ``` You may notice that this declarations block also corresponds to the assignment order in the erroneous code snippet, as in the previous example\. Here in the assignment, _mfTimePos_ gets the value instead of the _mfLength_ variable\. Except for in this case, the error can't be explained by copying the block and the "last line effect"\. _mfLength_ may not need to be assigned a new value, but this piece of code is still dubious\. **Fragment 6\.** This part of code from "Amnesia: A Machine For Pigs" sparked off the analyzer to issue a truckload of warnings\. I will give only a part of the code that triggered errors of the same kind: ```cpp void cLuxEnemyMover::UpdateMoveAnimation(float afTimeStep) { .... if(prevMoveState != mMoveState) { .... //Backward if(mMoveState == eLuxEnemyMoveState_Backward) { .... } .... //Walking else if(mMoveState == eLuxEnemyMoveState_Walking) { bool bSync = prevMoveState == eLuxEnemyMoveState_Running || eLuxEnemyMoveState_Jogging ? true : false; .... } .... } } ``` Where is the error here? Here are the analyzer warnings: * [V768](https://pvs-studio.com/en/docs/warnings/v768/) The enumeration constant 'eLuxEnemyMoveState\_Jogging' is used as a variable of a Boolean\-type\. LuxEnemyMover\.cpp 672 * [V768](https://pvs-studio.com/en/docs/warnings/v768/) The enumeration constant 'eLuxEnemyMoveState\_Walking' is used as a variable of a Boolean\-type\. LuxEnemyMover\.cpp 680 * [V768](https://pvs-studio.com/en/docs/warnings/v768/) The enumeration constant 'eLuxEnemyMoveState\_Jogging' is used as a variable of a Boolean\-type\. LuxEnemyMover\.cpp 688 The if\-else\-if sequence in the original code is repeated, and further these warnings were issued for each body of each _else if_\. Let's consider the line that the analyzer points to: ```cpp bool bSync = prevMoveState == eLuxEnemyMoveState_Running || eLuxEnemyMoveState_Jogging ? true : false; ``` No surprise, an error crept into such an expression, originally written in line\. And I'm sure you've already noticed it\. The _eLuxEnemyMoveState\_Jogging_ enumeration element is not compared with anything, but its value is checked\. Most likely, the expression 'prevMoveState \=\= eLuxEnemyMoveState\_Jogging'was meant\. Such a mistake may seem quite harmless\. But in another article about [checking the Bullet Engine](https://pvs-studio.com/en/blog/posts/cpp/0647/), among the commits to the project, I found a [fix for an error](https://github.com/bulletphysics/bullet3/pull/1289) of the same kind, which led to the fact that forces were applied to objects from the wrong side\. As for this case, this error was made several times\. Well, note that the ternary condition is completely meaningless, since it will be applied to the boolean results of logical operators in the last place\. **Fragment 7\.** Finally, the last couple of examples of copy\-paste errors\. This time again in a conditional statement\. The analyzer issued a warning for this piece of code: ```cpp void iParticleEmitter::SetSubDivUV(const cVector2l &avSubDiv) { //Check so that there is any subdivision // and that no sub divison axis is //equal or below zero if( (avSubDiv.x > 1 || avSubDiv.x > 1) && (avSubDiv.x >0 && avSubDiv.y >0)) { .... } .... } ``` I think that in such a separate fragment from the entire code, it is quite easy to notice an awkward place\. Nonetheless, the error found its way to hide from developers of this game\. The analyzer issued the following message: [V501](https://pvs-studio.com/en/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\|\|' operator: avSubDiv\.x \> 1 \|\| avSubDiv\.x \> 1 ParticleEmitter\.cpp 199 The second parenthesis in the condition indicates that both _x_ and _y_ fields are checked\. But in the first parenthesis, for some reason, this point was missed and only the _x_ field is checked\. In addition, judging by the review comment, both fields should have been checked\. So it is not the "last line effect" that has worked here, but rather the "first line effect", since in the first parenthesis the author forgot to replace accessing the _x_ field with accessing the _y_ field\. Obviously, such errors are very insidious, since in this case even the explanatory comment to the condition did not help the developer\. In such cases, I would recommend making it a habit to record related checks in tabular form\. This way, it is easier both to edit, and notice a defect: ```cpp if( (avSubDiv.x > 1 || avSubDiv.x > 1) && (avSubDiv.x > 0 && avSubDiv.y > 0)) ``` **Fragment 7\.5\.** An absolutely similar error was found in a different place: ```cpp static bool EdgeTriEqual(const cTriEdge &edge1, const cTriEdge &edge2) { if(edge1.tri1 == edge2.tri1 && edge1.tri2 == edge2.tri2) return true; if(edge1.tri1 == edge1.tri1 && edge1.tri2 == edge2.tri1) return true; return false; } ``` Did you get a chance to see where it was hiding? It is not for nothing that we've already dealt with so many examples :\) The analyzer has issued a warning: [V501](https://pvs-studio.com/en/docs/warnings/v501/) There are identical sub\-expressions to the left and to the right of the '\=\=' operator: edge1\.tri1 \=\= edge1\.tri1 Math\.cpp 2914 We'll sort this fragment through one part after another\. Obviously, the first check checks the equality of the fields _edge1\.tri1 _and _edge2\.tri2_, and at the same time the equality of _edge1\.tri2_ and _edge2\.tri2_: ```cpp edge1.tri1 -> edge2.tri1 edge1.tri2 -> edge2.tri2 ``` In the second check, judging by the correct part of the check 'edge1\.tri2 \=\= edge2\.tri1', equality of these fields had to be checked in a crosswise way: ![0769_Amnesia_The_Dark_Descent/image7.png](https://import.viva64.com/docx/blog/0769_Amnesia_The_Dark_Descent/image7.png) But instead of checking for _edge1\.tri1 \=\= edge2\.tri2_, there was a pointless check _edge1\.tri1 \=\= edge1\.tri1_\. By the way, all this is in the function, I removed nothing\. Still such an error has wormed into the code\. ## Other errors **Fragment 1\.** Here is the following code snippet with the original indents\. ```cpp void iCharacterBody::CheckMoveCollision(....) { .... ///////////////////////////////////// //Forward velocity reflection //Make sure that new velocity points in the right direction //and that it is not too large! if(mfMoveSpeed[eCharDir_Forward] != 0) { vForwardVel = ....; float fForwardSpeed = vForwardVel.Length(); if(mfMoveSpeed[eCharDir_Forward] > 0) if(mfMoveSpeed[eCharDir_Forward] > fForwardSpeed) mfMoveSpeed[eCharDir_Forward] = fForwardSpeed; else if(mfMoveSpeed[eCharDir_Forward] < fForwardSpeed) mfMoveSpeed[eCharDir_Forward] = -fForwardSpeed; } .... } ``` PVS\-Studio warning: [V563](https://pvs-studio.com/en/docs/warnings/v563/) It is possible that this 'else' branch must apply to the previous 'if' statement\. CharacterBody\.cpp 1591 This example can be confusing\. Why does _else_ have the same indent as the outer one at the _if_ level? Is it implied that _else_ is for the outermost condition? Well, then one has to place braces correctly, otherwise _else_ refers to the right\-front _if_\. ```cpp if(mfMoveSpeed[eCharDir_Forward] > 0) { if(mfMoveSpeed[eCharDir_Forward] > fForwardSpeed) mfMoveSpeed[eCharDir_Forward] = fForwardSpeed; } else if(mfMoveSpeed[eCharDir_Forward] < fForwardSpeed) { mfMoveSpeed[eCharDir_Forward] = -fForwardSpeed; } ``` Or is it not so? When writing this article, I changed my mind several times about which version of the actions sequence for this code was most likely\. If we dig a bit deeper into this code, it turns out that the variable _fForwardSpeed_, which is compared in the lower _if_, can't have a value less than zero, since it gets the value from the _Length_ method: ```cpp inline T Length() const { return sqrt( x * x + y * y + z * z); } ``` Then, most likely, the point of these checks is that we first check whether the _mfMoveSpeed_ element is greater than zero, and then check its value relative to _fForwardSpeed_\. Moreover, the last two _if_ statements correspond to each other in terms of the wording\. In this case, the original code will work as intended\! But it will definitely make the one who comes to edit/refactor it rack their brains\. I thought I'd never come across code that looked like this\. Out of interest, I looked at our [collection of errors](https://pvs-studio.com/en/blog/examples/) found in open\-source projects and described in articles\. Examples of this error were also found in other projects \- you can look at them [yourself](https://pvs-studio.com/en/blog/examples/v563/)\. Please, don't write like this, even if you are clear about it yourself\. Use braces, or correct indentation, or better \- both\. Don't make those who come to understand your code suffer, or yourself in the future ;\) **Fragment 2\.** This error has taken me aback, so it took a while to find logic here\. In the end, it still seems to me that this is most likely a mistake, a quite whopping one\. Take a look at the code: ```cpp bool cBinaryBuffer::DecompressAndAdd(char *apSrcData, size_t alSize) { .... /////////////////////////// // Init decompression int ret = inflateInit(&zipStream); if (ret != Z_OK) return false; /////////////////////////// // Decompress, chunk by chunk do { //Set current output chunk zipStream.avail_out = lMaxChunkSize; .... //Decompress as much as possible to current chunk int ret = inflate(&zipStream, Z_NO_FLUSH); if(ret != Z_OK && ret != Z_STREAM_END) { inflateEnd(&zipStream); return false; } .... } while (zipStream.avail_out == 0 && ret != Z_STREAM_END); .... return true; } ``` [V711](https://pvs-studio.com/en/docs/warnings/v711/) It is dangerous to create a local variable within a loop with a same name as a variable controlling this loop\. BinaryBuffer\.cpp 371 So, we have a _ret_ variable, which controls the exit from the _do\-while_ loop\. But inside this loop, instead of assigning a new value to this external variable, a new variable named _ret_ is declared\. As a result, it overrides the external _ret_ variable, and the variable that is checked in the loop condition will never change\. With mishap coinciding, such a loop could become infinite\. Most likely, in this case, it is an internal condition that saves this code\. It checks the value of the internal _ret_ variable and leads to the exit of the function\. ![0769_Amnesia_The_Dark_Descent/image8.png](https://import.viva64.com/docx/blog/0769_Amnesia_The_Dark_Descent/image8.png) ## Conclusion Very often, developers do not use static analysis regularly, but with long breaks\. Or even run the project through the analyzer only once\. As a result of this approach, the analyzer often does not detect anything serious or finds something like the examples we are considering, which may not particularly affect the game's performance\. One gets the impression that the analyzer is not really useful\. Well, it found such places, but everything still works\. The fact is that there were similar places where a mistake was on the surface, and definitely resulted in a program error\. These fragments have already been refined due to long hours of debugging, tests runs, Q&A department\. As a result, when the analyzer checks the project only once, it shows only those problems that did not manifest themselves in any way\. Sometimes such problems comprise critical issues that actually affected the program but will not likely to follow their scenario\. Therefore, this error was unknown to the developers\. That's why it is extremely important to evaluate the usefulness of static analysis only after its regular use\. Once a one\-time run through PVS\-Studio revealed such suspicious and sloppy fragments in the code of this game, imagine how many obvious errors of this kind had to be localized and fixed in course of development\. Use a static analyzer regularly\!