C++ gameplay coding style suggestion: more checks at the start of functions

Now that I’ve had a chance to work with the UE4 C++ gameplay side for a few years, I feel like the gameplay code would be a lot easier to learn if there were more liberal use of checks, specifically relating to online multiplayer. For example, in our game code if a function is intended to be authority-only, we’ll outright assert it at the top of the function:

check(HasAuthority());

This not only helps to catch a lot of multiplayer bugs, but also helps document the intended purpose of the function, making the gameplay code a lot easier to understand. I was a little surprised by how much of the engine C++ gameplay code relied on header comments or didn’t have any comments at all to indicate the purpose of a function or a variable in relation to online multiplayer. Obviously UPROPERTY and UFUNCTION can help document intended purpose for functions, but there are plenty of raw C++ functions and variables in there with limited documentation. I imagine if someone at Epic went through and concreted some assertions, they’d probably catch a lot of bugs with it.

Cheers :slight_smile:

Good idea, I’ll be adding that check to my project

Yeah, the other one that gets my feathers all ruffled up is whenever a Replicated UPROPERTY from the server is directly set in code that runs on clients as well as the authority. There’s loads of places in the engine gameplay code where that happens, and it really makes things unclear. A replicated UPROPERTY on a client should never be set directly, as it will always have its value replicated over from the server. More often than not it’s a benign issue, as it happens in construction code or somewhere else where it’s not going to cause a problem, but for readability/comprehension it makes it very strange. And coming back to my first point, if the gameplay functions asserted more clearly their intended online context (authority only, simulated/autonomous clients only, any of the above) these kinds of things would be a lot easier to spot and clean up.

check(condition) crashes when the condition isn’t met. A better approach IMHO would be to use ensure(condition):

if(ensure(condition))
{
//logic
}