I've found that a lot of more junior engineers just want to rotely pattern match on one-size-fits-all "style" rules in their code review without fully understanding why those rules exist in the first place.

Like all rules, there's always exceptions. In a hot code path like the physics engine logic, you're going for performance optimizations (which often LOOK messy) and that means making slight compromises on readability. It's quite a bit different than the code implementing the business logic in your SaaS company.

> C has no pattern for breaking out of multiple for loops at the same time.

True, but it is cleaner to reorganize the code into several functions and use the return value to propagate across layers if needed. Performance should be the same.

> it's not applicable here

Why? If there is a new/delete pair anywhere, it should have been an object.

> Game developers have a long-seated distrust of std::vector, and for good reasons.

Which reasons? std::vector (and std::unique_ptr) is universally useful (unlike many other std data structures) and codegen should be on par as a new/delete pair.

If the std is completely broken in some of the console platforms they need to support (likely from what I hear here) then it can also be done with a custom heap array wrapper.

So I don’t see the reason why that shouldn’t have been an object.

> C has no pattern for breaking out of multiple for loops at the same time. Other languages like Java and JavaScript introduced "break label;" to handle this edge case. goto is perfectly acceptable to break out of multiple loops.

Yeah, that's not what it does. The code looks like this:

    if (pgeom->Intersect(pentlist[i]->m_parts[j].pPhysGeomProxy->pGeom, gwd,gwd+1, &ip, pcontacts)) {
            got_unproj:
            if (dirUnproj.len2()==0) dirUnproj = pcontacts[0].dir;
            t = pcontacts[0].t;     // lock should be released after reading t
            return t;
    }       
    for(int ipart=1;ipart<m_nParts;ipart++) if (m_parts[ipart].flagsCollider & pentlist[i]->m_parts[j].flags) {
            gwd[2].R = Matrix33(qrot); 
            gwd[2].offset = pos + qrot*m_parts[ipart].pos;
            gwd[2].scale = m_parts[ipart].scale;
            gwd[2].v = -dirUnproj;
            if (m_parts[ipart].pPhysGeomProxy->pGeom->Intersect(pentlist[i]->m_parts[j].pPhysGeomProxy->pGeom, gwd+2,gwd+1, &ip, pcontacts))
                    goto got_unproj;
            }
    }

Notice (a) the if statement on the same line as the for loop, and (b) the fact that the goto jumps out of a conditional inside a for loop inside of a conditional into a conditional just before the for loop.

EDIT: Just realised the poster is referring to a different function.

IMO this one is more horrifying.

I agree with the gist of your post, but one positive about single use functions is that you can be explicitly clear about data visibility.

In this contrived example, you can tell that formatting a title is not affected by user preferences, but formatting the body is. (And additionally that formatting a body has no information about the other fields of an entry)

  def formatRssFeedEntries(userSettings: UserSettings, data: List[Entry]) {
    val titles = data.map(entry => formatTitle(entry.title))
    val bodies = data.map(entry => formatBody(entry.body, userSettings))
  ...
  }

Nested ifs in for loops, with lots of things going on here, I don‘t know but that‘s the definition of hard to maintain and to read code.

Plus the Hungarian notation and whitespaces are not helping either.

When there is a test class to accompany this thing it would maybe help to faster make sense of it. But that‘s still no fun.

Splitting it up and using functions to extract use cases would help tremdendiously.

Not going to lie, playing Crysis was a lot of fun, and I never knew this was the underneath function running it.

I have a feeling you may have been approaching this problem in UE4 the wrong way. Adding a double jump can be done in numerous ways, but one simple way is with Blueprints. See below link where the exact functionality is implemented with a really simple blueprint.

https://m.youtube.com/watch?v=hFAr7gYV1rA

Well duh, this is a physics engine. Implement double jump by placing an invisible floor underneath the player object when you decrement the counter.

Well to be honest UE4 codebase is one of the biggest garbage dumps I've ever had the displeasure to work with. I think the biggest problem is that it's just been accumulating 20 years worth of code and features without anything ever being refactored. And because of the current status you'd never actually want to modify any existing functionality because you're bound to break something. It's also super badly documented and riddled with bugs and really bad and confusing design deductions such as using instances of your UClasses to represent the "types" and and then having to use flags at runtime to see if "this" is archetype or not.

We haven't at place where deciplines interact.

This. The convenience of everything in a single file is underrated. You can do all the modern best practice like splitting into more functions, making functions small, and I wouldn't mind if they are all in the same file.

Being a Java dev (not enterprise any more) I agree that some enterprise software is an abomination for exactly the reason you mention.

However, can you imagine how bad an enterprise C++ project would look like?

Code like this actually always makes me feel better about my own code when I’ve ultimately had to make a trade off between abstraction/organization and performance. I wonder who all these engineers are that see code like this, especially in hot paths, and can’t understand how it came to be and that there was a deliberate choice made.

This is not about race condition. Rather it is something more like why you need volatile keyword.

https://stackoverflow.com/questions/72275/when-should-the-vo...

Not defending the use of magic numbers but some of it is clear for someone who has written physics code before.

I encourage you to try! However, a lot of gameplay and getting movement controls to feel good is difficult and at odds with the goal of "modularized physics", e.g. you might want to apply different amounts of ground friction depending on whether the player is moving, how they're moving, whether they're holding the jump key, and so on.

Ultimately, we're trying to simplify a large simulation of real-world physics and hundreds of controllable muscles and motor responses trained over a lifetime, down to 5 or 6 keyboard inputs. That means you tend to have such inputs doing multiple things, and it's often hard to make separable.

Here's a good article from John Carmack on why that can be a good approach in game development: http://number-none.com/blow/john_carmack_on_inlined_code.htm...

I feel like this Cryengine example may be a bad example of that, though.

Sometimes with inherently complex performance code it gets nickel and dimed over time yet maintains so many cross-cutting concerns and state that there are no clean extraction points.

So function extraction ends up taking a bunch of "unrelated" state with it where, in the end, it feels like you accomplished nothing but split the code into arbitrary concatenation points, not logical units.

Games are certainly the most popular piece of end user software.

Software that a lot of people use a lot of time looks like this because the bugs are found and the bugs get fixed. The code for fixing a bug has to live somewhere.

What about bugs that computers find? Fuzzers are rarely recommending to fix bugs that are affecting human users. Part of that is also that the kinds of bugs that computers can find are not in, literally, "user interfaces," they are in APIs and formats.

Anyway, end user business software also has code that looks like this. It's not just all tools and infrastructure, I mean it certainly feels that way. But there are 800+ line SQL statements. 800+ line transactional method bodies. I don't want to call this "real code" but the surprise comes from... well eventually you have to make something the end user touches. And it's going to be gnarly.

Apart from performance considerations, it's also because in game development, different systems are much more entangled together in the requirements themselves, in many very small and unpredictable ways that still break architectural boundaries you put in.

For a classic example, it's makes almost no sense separate model and view when you're building a real time action game, because your rendering code and your ballistics and physics work on very similar 3d meshes that are animated by the same skeleton animations and tied to the same objects.

I feel like C++ makes it particularly difficult to jump around big projects (mainly headers especially back in the old days pre good-ide).

There is also a lot of fear of performance regressions by breaking up big chunks of code (arguably unfounded with modern compilers).

Why not? Doesn't the indentation tell you perfectly where each functions starts and ends?

If it works, it's an example of ugly code. Plenty of game code is ugly as sin, though.

I would argue that manual memory management should probably be avoided as a general best practice, but game development is a special case where memory management can often be critical to the performance of the final product, and a manual approach is sometimes warranted.

People customizing the UE4 engine professionally or on large projects put a lot of work into the areas they are interested to the point where it's not really UE4 anymore in that area. Gears of War is a good example where areas of the rendering system would be almost unrecognizable as they have put in countless man years of work diverging from the base engine.

It falls more in the category of having a lot bugs which could've been caught if they used static code analysis, code review, etc...

I understand that you might think that messy could mean it's fine tuned for performance. In this case, I highly doubt it and think it's more reasonable to think it's messy because they had deadlines.

The messy part isn't about performance optimizations. It's more about things that got crammed in there and only works for a very specific subset of parameters. And even then you can't be sure it'll work...

I don't blame the programmers, it feels they had deadlines to uphold from managment.

Let’s assume that when code is first written, the cleanliness and performance is somewhat random within a broad range.

If we want the code to be clean or performant, we will likely have to spend time iterating on and pruning the code. Let’s assume that improving performance and improving cleanliness are at best orthogonal, at worst opposing.

The project has a limited amount of time, particularly for games, which often have a relatively low roof for how much maintenance the code will need.

The project has a budget on time to spend between cleanliness and maintenance. Games need high performance and relatively little maintenance, so they are more likely to spend their budget on much more performance than cleanliness.

(Game engines meant for heavy reuse such as Frostbite and Unreal Engine would likely have a much more even split, and similar for games which are likely to receive recurring and invasive updates. I would expect Fortnite’s code to be fairly clean as games go, for example.)

I don't think it's necessary. With modern C++ it's possible to encapsulate high performance code.

The issue with games in particular is probably partly due to the performance optimizations being directed at a moving target (it's not just your supercomputer nodes, it's every computer CPU). C++ doesn't really help you much in that regard (or at least better know but certainly not 10 years ago)

I disagree with this line of reasoning. I have seen good/bad examples on either side. I think it actually comes down to someone on the developer team having a high set of standards that they push everyone to subscribe to.

> things that are not open source are rarely not well written

I think you got lost in your triple negative there

Crytek v. Cig, which the court ruled largely in favor of Cig, and even called out Crytek's behavior (which was an absolute circus). Crytek will prosecute given even the most questionable grounds.

The consequences of taint are a very real risk here.