Cryengine Source Code

212 points by TiccyRobby 5 years ago | 143 comments
  • Nican 5 years ago
    This is on the front page again. Please take some time to read this wonderful function: https://github.com/CRYTEK/CRYENGINE/blob/release/Code/CryEng...

    EDIT: That whole function is a minefield. Just taking a quick look:

    * 814 lines of code

    * goto inside 3 nested for-loops

    * macros

    * commented out code

    * new/delete, with no RAII

    * thread specific variables and locks (?)

    • Jasper_ 5 years ago
      I don't think it's the best code I've ever seen, but a lot of these are surface-level complaints.

      > * goto inside 3 nested for-loops.

      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.

      > * new/delete, with no RAII

      They use RAII, but it's not applicable here. In this case, the developers only want to allocate a temporary array when it needs to be resized. Since we don't want a large stack allocation (stack sizes are super small on consoles), using a delete/free to resize an array seems fine to me. Game developers have a long-seated distrust of std::vector, and for good reasons.

      RAII is used for the WriteCondLock which you criticize in the next bullet point, so it's not like they were unaware of it. Just not the right tool for the job.

      > * thread specific variables and locks (?)

      You seem to be upset that they have code that uses locks at all? I don't really know what this bullet-point is saying, other than "I looked for 5 minutes and didn't understand the threading structure".

      If I had to take an issue with this code, it's the lack of enums for e.g. iSimClass, despite it having an enum with definitions. That's the sort of stuff that's difficult to reason about and follow along with without having a mapping in my head. And it has no overhead, so why not do it?

      https://github.com/CRYTEK/CRYENGINE/blob/6c4f4df4a7a092300d6...

      • Hydraulix989 5 years ago
        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.

        • jfkebwjsbx 5 years ago
          > 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.

          • Jasper_ 5 years ago
            > True, but it is cleaner to reorganize the code into several functions and use the return value to propagate across layers if needed.

            I disagree. Having to jump to another function definition which is inline is a bigger mental block than following a goto. The large amount of arguments you'd need to pass might also be a barrier, as is the mental overhead of checking to see if this function might be called from elsewhere. But reasonable people can disagree on this point.

            I suggest you try to clean up the function yourself and see if your function-ed version is in any way improved.

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

            The case we want is that we have a large array of pairwise collisions. This is a temporary array used inside the method. If we ever have more than this, we need a new (contiguous) array. Are you suggesting that the code should have done something like this?

            template<typename T> class TempArray { T* storage; void resize(int n) { delete[] storage; storage = new T[n]; };

            I mean, sure, it's a very minor cleanup. It changes like, two lines though, and makes us have to access the array through an awkward ->storage pointer. Not ideal IMO.

            > codegen should be on par as a new/delete pair.

            Here's modern MSVC. Let's play "spot the difference": https://gcc.godbolt.org/z/KqR-LN

            I'm not even doing anything but allocating the vector / array. It took me 2 minutes to navigate to godbolt and type this in. Don't just say "should be"... test it yourself!

          • messe 5 years ago
            > 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.

            • Jasper_ 5 years ago
              That's in a different function, so I wasn't counting it, but sure. I mean, I would write it differently, but it's fairly readable. It's jumping to a common function epilogue once it finds something it can collide with. Seriously -- try reading through it rather than gawking at the goto.
              • userbinator 5 years ago
                Either that code is misleadingly indented and you copied one more closing brace than necessary, or you omitted the opening brace for the last if's body (which contains a single goto).
            • barrkel 5 years ago
              If the code was separated out into functions that are only ever called once, I'd find it harder to read.

              Analysing code I'm not familiar with often consists of manually tracing through calls, producing documentation that inlines all the single use function calls.

              Ideally function names act as shorthand for the body of the function, but if they only have one caller they have nothing to keep them honest. In older codebases, function names are as misleading as comments; semantic drift, special cases etc. mean you need to drill into them anyway.

              • mopierotti 5 years ago
                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))
                  ...
                  }
                • barrkel 5 years ago
                  I agree. Blocks can somewhat substitute with scoping effects, and that's what I do with my inlined docs - they're nested {} with plain text description of contents and mentions of key variables and functions, scope is useful.
                • myspy 5 years ago
                  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.

                • stephc_int13 5 years ago
                  As a professional game developer I disagree with most of your comments.

                  This code is clearly not perfect, but the from what I've seen, this is something I could work with.

                  - Function names are easy to read and understand. - Indirections are kept to a manageable level.

                  • Nican 5 years ago
                    Not going to lie, playing Crysis was a lot of fun, and I never knew this was the underneath function running it.
                    • user5994461 5 years ago
                      Crysis shipped with a full blow SDK that included most of its source code. You could actually rebuild the game from it, the 50MB dll that controlled the whole game.

                      Old players maybe remember that the crysis multiplayer was the most cheated game in its era. It was totally unplayable due to all the cheating and that killed the game.

                      One way to make cheats. You could load up the SDK in visual studio. Find the code that's removing -1 ammo when shooting and edit it to not do that (most of the physics and game logic was editable that way). Compile the DLL. Replace the original DLL in the game directory.

                  • skohan 5 years ago
                    As a hobbyist who likes to tinker with game and interactive media development, a sentiment I often come across is that in 2020 it makes no sense to implement a game engine, and that I should just use something which already exists to avoid re-inventing the wheel. Code like this is one thing which helps me to calmly ignore than sentiment.

                    I came across the same kind of thing when I was kicking the tires on the Unreal Engine, and I wanted to attempt to add a double jump. I thought surely this should be an easy task, I would just need to find where the jump occurs, add a counter, and remove the restriction which only lets a character jump when touching the ground. What I found was a monstrous tangle of indirection similar to this one.

                    Now that's not to say that these engines are "bad code" - when you look at all the things a modern game engine does, including supporting interactive editing for non-coders, I'm sure there is some explanation for the level of complexity seen in code like this just because of how many systems must be layered on top of each-other. But that is the thing which makes me question whether general-purpose game engines are really a good idea at all.

                    In most other domains of software we've long ago eschewed this type of do-everything monolithic software design in favor of more loosely coupled composible toolsets. I'm not entirely sure why it seems that game development has yet to escape this paradigm.

                    • christoph 5 years ago
                      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

                      • skohan 5 years ago
                        Oh I am certain I was not approaching the problem in the UE4 way. But the issue is that the way UE4 expects me to do things is not the way I would like to approach game development.

                        UE4 has a strong bias about the way things should work. If I am making something which is fairly well aligned to that bias, then it's fairly easy to make it work. But if I want to achieve something which is quite far from what the engine expects, then I have to invest significant effort undoing or circumventing what UE4 already does before adding my own functionality on top. I would greatly prefer to start from a blank slate, and only add precisely the behavior I actually want.

                        So basically this experience with the double jump just gave me a window into the level of complexity I would have to work around in terms of realizing my own goals.

                      • oneshot96 5 years ago
                        Well duh, this is a physics engine. Implement double jump by placing an invisible floor underneath the player object when you decrement the counter.
                        • skohan 5 years ago
                          I cannot tell if you are joking or not, but needing to place an entity into the world for a single frame to accomplish this rather than simply applying an impulse to the player is precisely the kind of over-complication I'm referring to
                        • ensiferum 5 years ago
                          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.
                          • rtx 5 years ago
                            We haven't at place where deciplines interact.
                          • userbinator 5 years ago
                            I'd rather work with this than Enterprise Java.

                            At least the logic is all there and you only need to scroll to see it, instead of jumping around between a dozen or more different files.

                            Math-heavy code tends to look dense to those who are accustomed to more "mundane" LOB type applications.

                            • kccqzy 5 years ago
                              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.
                              • bitcharmer 5 years ago
                                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?

                                • rowanG077 5 years ago
                                  A gameengine is as enterprise as it gets.
                              • user5994461 5 years ago
                                * goto inside 3 nested for-loops

                                It's actually a reasonable practice in C and C++ to use goto to leave nested loops (I am hesitating to say a recommended practice).

                                There is often no sane way to leave the loop otherwise, break/continue statements only have effect in the most inner loop. It's possible to set extra variables with lots of if/break but that gets crazy real quick and much slower (nested loops are often the hot code path).

                                • wesmac 5 years ago
                                  I knew which function this was before even clicking the link. I spent months working on and debugging that exact function and the surrounding CryPhysics code improving network interpolation back in 2014. All the undefined behavior caused us some trouble while porting to the "next-gen" consoles of the time.

                                  This is the worst code to read in the engine by far. At the end of the day though it worked, we shipped it and it performed well enough.

                                  Last I checked Lumberyard still has a somewhat cleaned up version of this function.

                                  • gameswithgo 5 years ago
                                    inside a bunch of nested loops is where you need a goto, if the language doesn’t have labeled breaks (which is just a fancy goto)

                                    most of your points are things people notice any time real actual big software with performance constraints gets posted. so along with questioning the wisdoms in that function, also question your own wisdom. maybe some of what you think you know is wrong.

                                    • qeternity 5 years ago
                                      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.
                                    • MauranKilom 5 years ago
                                      This is what draws my attention more:

                                      > //FIXME: There's a threading issue in CryPhysics with ARM's weak memory ordering.

                                      https://github.com/CRYTEK/CRYENGINE/blob/6c4f4df4a7a092300d6...

                                      Translation: "We have race conditions in our C++, but x86 is lenient enough and current MSVC not aggressive enough to make it crash and burn constantly on our main platform."

                                      Coincidentally, I finished Crysis 1 today. That involved the first four crashes I had with the game, three of which were in the final battle.

                                      • 361994752 5 years ago
                                        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...

                                        • MauranKilom 5 years ago
                                          Can you elaborate? volatile and threading in C++ are orthogonal to each other. I don't understand why you consider a C# volatile discussion relevant when talking about C++ race conditions.
                                          • ensiferum 5 years ago
                                            Volatile in c++ has nothing to do with threading.
                                        • rurban 5 years ago
                                          This is physics, not your average function. Those who've never written physics should not complain. It's actually good code.
                                          • bufferoverflow 5 years ago
                                            And

                                            * magic numbers

                                                m_iSimClass = 3;
                                                return 1E10;
                                                m_timeSmooth*(1/0.3f)
                                                sqr(0.0001f)
                                                m_pos.len2()>1E18
                                                helper.pos.len2()<50.0f
                                                sqr(m_size.x)*g_PI*0.25f && maxdim<m_size.z*1.4f)
                                                *1.05f
                                                *0.4f
                                                *0.2f
                                                <0.087f
                                            
                                            * Very few comments explaining what's going on

                                            When I was getting my CS degree, my professors required, at the very least, to describe what each method does, what each argument is, and a possible range of values of each, and the same for the return value.

                                            I hate this "self-documenting" nonsense, which obviously doesn't work. This piece of code is a good example of that.

                                            • isatty 5 years ago
                                              Not defending the use of magic numbers but some of it is clear for someone who has written physics code before.
                                            • atombender 5 years ago
                                              The code looks bad, but not for those reasons, in my opinion. The logic in this function doesn't look composable. It combines different kinds of mathematical functions to apply inertia, whether you're jumping, etc. into one big ball of spaghetti that would be hard to extend for anyone not deeply familiar with the code. If I were to try to refactor this, I would try to decompose it into standalone "behaviour" functions that could be attached to any entity
                                              • Jasper_ 5 years ago
                                                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.

                                                • gmueckl 5 years ago
                                                  I guess, a sufficiently simple exercise would be to write a controller that handles ground movement, jumping with air movement on horizontal ground and moving platforms. Doesn't have to feel good to play, just be reasonably robust. Either handling of slopes and stairs or collisions with dynamic objects can be added for extra credits ;).
                                              • gentleman11 5 years ago
                                                800 lines long. The movement component class in ue4 is about 10k lines. Why do game engines separate their code so much less than in other software?
                                                • Trasmatta 5 years ago
                                                  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.

                                                  • smaddox 5 years ago
                                                    Not just in game development. If you have a function that is only called once, it shouldn't be a function yet. Make it a function when you have a second or third use for it. Then, and only then, you will know what the parameters should be.
                                                    • 5 years ago
                                                    • hombre_fatal 5 years ago
                                                      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.

                                                      • an_opabinia 5 years ago
                                                        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.

                                                        • golergka 5 years ago
                                                          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.

                                                          • mhh__ 5 years ago
                                                            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).

                                                          • loeg 5 years ago
                                                            • 5 years ago
                                                              • layoutIfNeeded 5 years ago
                                                                "You may not like it, but this is what peak performance looks like."
                                                                • nolaspring 5 years ago
                                                                  Also Requires visual studio
                                                                  • aronpye 5 years ago
                                                                    It’s not just that function, the C file is over 2300 lines long. It’s hard to tell where one function starts and another one ends in that mess
                                                                    • kccqzy 5 years ago
                                                                      Why not? Doesn't the indentation tell you perfectly where each functions starts and ends?
                                                                    • ajconway 5 years ago
                                                                      Is it an example of bad code? Should we avoid using Cryengine?
                                                                    • dmitrygr 5 years ago
                                                                      > new/delete, with no RAII

                                                                      There is nothing wrong with this. Plenty of people have no issues keeping track of memory in their head.

                                                                      • skohan 5 years ago
                                                                        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.
                                                                    • ebg13 5 years ago
                                                                      Many of the filed issues are "X not work" and not very interesting, but this one...this one is a real gem. https://github.com/CRYTEK/CRYENGINE/issues/763
                                                                      • duncan-donuts 5 years ago
                                                                        I’m gonna go with this is a feature not a bug
                                                                      • misnome 5 years ago
                                                                        Would it be an overreaction to not want to touch this licence with a barge pole?

                                                                        Even ignoring the “We may change this licence at any time and it applies to you” parts, there seem to be serious restriction on usage and basically have to ask them to do _anything_ beforehand.

                                                                        Maybe it makes sense if you are already in a project that is using this Licenced? Is this intended as a general engine licence rather than viewing the source code?

                                                                        • _bxg1 5 years ago
                                                                          Unreal shares its source purely for the sake of people who already license it and need to know how something works/fix something/customize something. Wouldn't be surprised if the same is true here.
                                                                        • gentleman11 5 years ago
                                                                          I considered using cryengine recently but there was an almost total lack of learning resources: I could barely find a tutorial that was newer than 5 years, especially one that involved it’s c++ APIs.

                                                                          I suspect that lumberyards greatest advantage over cryengine in the future will simply be usable documentation provided by amazon. Cryengine is simply not usable without better docs or else an incredible amount of time. Crytek is having financial troubles but I bet their engine would have 10x adoption if they hired a team technical writers

                                                                          Unreals docs are fairly bad also, but at least there are some third party resources to turn to

                                                                          • mhh__ 5 years ago
                                                                            Unreal docs are fairly good for making games but if you want to modify the structure of the engine it's quite annoying/nonexistent.

                                                                            Case in point: Vehicle physics is no where near as good as the docs imply (not a toy but still 20 year old vintage), but there is almost no documentation of how PhysX interacts with the Unreal engine proper i.e. you can get the PxRigidWhatever handle but you can't easily replace PhysX with a proper MB package. Epic seem to be transitioning to Chaos but it's not documented yet.

                                                                            If I ever get good Vehicle physics working I'll write it up (it's definitely possible but I'm not sure how ACC does it)

                                                                            • djmips 5 years ago
                                                                              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.
                                                                              • mhh__ 5 years ago
                                                                                I'm sure it's possible I'd just rather not read thousands of lines of code to find the nitty gritty.

                                                                                I want to make the basics of an open source (sim)racing game (as a test bed for writing tyre models), without using the fairly lacklustre offerings included by default (e.g. no sprung mass, no carcass stiffness etc., idealized suspension etc.). I have no need to go into the bowels of the rendering engine but it struck me that the interactions between PhysX and the actual actor model for the vehicle is almost not documented at all. I assume it's possible to do it solely with PhysX (proper suspension) but I cannot find any case studies of people doing it with the possible exception of their drive project which is under $$$ and NDA I'm guessing.

                                                                                I was also slightly surprised that I had to go looking for the option to connect to the PhysX debugger. It wasn't hard to find but I was half expecting it to be included with the engine.

                                                                          • reykjavik 5 years ago
                                                                            Used to work with cryengine some time ago. That is by far the worst c++ codebase i've ever seen.
                                                                            • halotrope 5 years ago
                                                                              While your opinion is appreciated. It would be more helpful, if you could point out sone of the reasons why it was so bad in particular.
                                                                              • Roritharr 5 years ago
                                                                                Interesting sentiment, I sometimes wonder if a "messy" codebase can have advantages for performance.

                                                                                Many very highly performance tuned applications I saw in the wild would fall into the category of "horrible codebase" when looked at through that lens.

                                                                                • nocturnial 5 years ago
                                                                                  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.

                                                                                  • noarchy 5 years ago
                                                                                    >I don't blame the programmers, it feels they had deadlines to uphold from managment.

                                                                                    This is my own experience. The teams that spent the most time on standards usually had the least pressure, in terms of things like deadlines. Once the focus of the team shifts to having to ship things, there is less time to worry about having 100% code coverage (to pull out an arbitrary number), and so forth. Code review can slip into flagging only things that really matter, and leaving nitpicks for another day.

                                                                                    • gentleman11 5 years ago
                                                                                      What sorts of static code analysis tools do people here use in their game projects? I know carmack is a big fan of them
                                                                                    • wwright 5 years ago
                                                                                      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.)

                                                                                      • mhh__ 5 years ago
                                                                                        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)

                                                                                      • 5 years ago
                                                                                        • jokoon 5 years ago
                                                                                          Generally, things that are not open source are rarely not well written, since there are less programmers who will read your code, and all questions on the code can be done internally, so developers only write code so it works.

                                                                                          Open source generally leads to better quality code, since it's the best way to attract other developers to contribute to it.

                                                                                          So I'm rarely interested by any accomplished project that opens its code.

                                                                                          For example, if microsoft opened its OS, I doubt developers would really try to do things with it. The windows kernel would obviously be high quality code, but a lot of the rest is probably short lived garbage.

                                                                                          • dman 5 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.
                                                                                            • gentleman11 5 years ago
                                                                                              All the open source projects I have personally seen were ones meant to live a long time. When there were code issues, there were always awkward discussions on github about “there should be unit tests here” or “this code makes no sense,” and weeks later the developer announcing a cleanup or some sort. Anecdotal but public scrutiny and pressure is a real thing.

                                                                                              Just as an example, this is why Bitwarden started getting some automated testing - lots of propelled bumping github issues about it in order to get it more visibility

                                                                                              • mhh__ 5 years ago
                                                                                                It's only tangentially related to code quality but I do think open/free source is the only way to write sustainable software if your aim is to change the world rather than ones bank account (so to speak).

                                                                                                There's terrible code all over the place, although it is definitely true that no one's going to clean up - even source available - proprietary code out of kindness of their heart.

                                                                                              • remram 5 years ago
                                                                                                > things that are not open source are rarely not well written

                                                                                                I think you got lost in your triple negative there

                                                                                                • jokoon 5 years ago
                                                                                                  yes, english is not my main language, thanks for the correction
                                                                                            • zamalek 5 years ago
                                                                                              That license looks like a minefield. Any lawyers able to chime in on the reality of becoming tainted? Given their recent history, this is one codebase you really wouldn't want to risk becoming tainted by.
                                                                                              • gear54rus 5 years ago
                                                                                                What is their recent history?
                                                                                                • zamalek 5 years ago
                                                                                                  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.

                                                                                              • dang 5 years ago
                                                                                                • rurban 5 years ago
                                                                                                  The real issue is clear from their announcement post.

                                                                                                  master (now main) was not always stable (of course, stable code are in the stable and release branches) so silly people complained, and the silly PM reacted by closing down pushes to main, and hereby closing down issues and PR's. He clearly has no idea how open source code development works. Now they have to maintain two repos, the internal one and thd public one, and get no feedback from outside. Well, feedback on one year old code.