Cryengine Source Code • Hacker News Board

Re: Cryengine Source Code

#111

Post by wesmac » Sun, Jun 28, 2020, 3:45 AM UTC

Nican wrote: post #13

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 (?)

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.

Re: Cryengine Source Code

#112

Post by Jasper_ » Sun, Jun 28, 2020, 3:46 AM UTC

jfkebwjsbx wrote: post #106

Jasper_ wrote: post #45

Earlier quoted context omitted.

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 a…

> 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 r…

> 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 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!

Re: Cryengine Source Code

#113

Post by Remnant44 » Sun, Jun 28, 2020, 4:50 AM UTC

Jasper_ wrote: post #112

jfkebwjsbx wrote: post #106

Earlier quoted context omitted.

> 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 r…

> 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 elsewh…

If you enable /Ox, the codegen basically drops to what you would expect: the vector version drops down essentially identical code to the new/delete (modulo a memset to enforce the clear to zero condition)

It is a good illustration of why debug stl builds are such hot garbage though...

Re: Cryengine Source Code

#114

Post by isatty » Sun, Jun 28, 2020, 5:04 AM UTC

bufferoverflow wrote: post #94

Nican wrote: post #13

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 (?)

And * magic numbers m_iSimClass = 3; return 1E10; m_timeSmooth*(1/0.3f) sqr(0.0001f) m_pos.len2()>1E18 helper.pos.len2() * 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, wh…

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

Re: Cryengine Source Code

#115

Post by athriren » Sun, Jun 28, 2020, 5:13 AM UTC

Jasper_ wrote: post #108

gentleman11 wrote: post #98

Earlier quoted context omitted.

Is there an industry-wide set of concepts and best practices that is applicable for large scale games? What sorts of things do hiring managers worry about at night?

Preface: I'm not a gameplay programmer. I'm not even a dynamics or physics specialist. And I don't hire for them. But as a core engine programmer: The power of an engine is that it is a series of closely intertwined parts. Often times, the graphics code and collision code are linked; here's an example. Shoot a bullet in any FPS; the resulting decal is likely created from the collision data, since it's much faster to…

Just want to chime in to say I have learned a lot from your participation in this thread, thanks for being part of what makes HN great.

Re: Cryengine Source Code

#116

Post by gmueckl » Sun, Jun 28, 2020, 5:44 AM UTC

Remnant44 wrote: post #113

Jasper_ wrote: post #112

Earlier quoted context omitted.

> 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 elsewh…

If you enable /Ox, the codegen basically drops to what you would expect: the vector version drops down essentially identical code to the new/delete (modulo a memset to enforce the clear to zero condition) It is a good illustration of why debug stl builds are such hot garbage though...

It is a good illustration for why using the STL is not always a good idea: you can't blindly take the perf hit from that kind of overhead in a debuggable build of a game that you still want to run at reasonably interactive frame rates.

Re: Cryengine Source Code

#117

Post by gmueckl » Sun, Jun 28, 2020, 6:09 AM UTC

Jasper_ wrote: post #54

atombender wrote: post #43

Earlier quoted context omitted.

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" function…

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 h…

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 ;).

Re: Cryengine Source Code

#118

Post by flipchart » Sun, Jun 28, 2020, 6:14 AM UTC

Jasper_ wrote: post #112

jfkebwjsbx wrote: post #106

Earlier quoted context omitted.

> 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 r…

> 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 elsewh…

I'm still getting up to speed on C++ (& ASM) so forgive the ignorance - are the differences you're referring to all the cleanup code that the vector does in the destructor?

As a side note, you're not calling delete[] on the array code, but I suppose it makes little difference if you take the vector cleanup into account.

Re: Cryengine Source Code

#119

Post by numpad0 » Sun, Jun 28, 2020, 6:22 AM UTC

Nextgrid wrote: post #78

user5994461 wrote: post #70

Earlier quoted context omitted.

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…

Was ammo in Crysis controlled client-side? I've always assumed such counters were stored server-side and thus a server won't apply a "fire" event if the player's ammo counter is at zero until it receives a "reload" event to reset the counter.

One explanation I’ve seen about weak server-side verification is online multiplayer is a cost center so developers wants to offload much as they could. At least before microtransactions I guess.

Re: Cryengine Source Code

#120

Post by bitcharmer » Sun, Jun 28, 2020, 6:54 AM UTC

userbinator wrote: post #80

Nican wrote: post #13

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 (?)

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.

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?