I used an else here and it seems okay but this thread suggests I’m maybe overlooking a bad side effect.
Is that true? It all seems to work fine…
void UGrabber::Release()
{
UPhysicsHandleComponent* PhysicsHandle = GetPhysicsHandle();
if (PhysicsHandle == nullptr)
{
return;
}
else
{
PhysicsHandle->ReleaseComponent();
}
}
I’d just write it like this:
void UGrabber::Release()
{
UPhysicsHandleComponent* PhysicsHandle = GetPhysicsHandle();
if (PhysicsHandle)
{
PhysicsHandle->ReleaseComponent();
}
}
This form is much more concise.
Since it no longer explicitly calls it out, would this still protect me from the situation where I forget to put a physics handle on my player? It seems safe right now, but if I added more lines to that release function, then we might get into trouble. That’s true, right?
This code protects against crashing inside UGrabber::Release if GetPhysicsHandle returns a nullptr. It doesn’t ‘care’ what happens inside GetPhysicsHandle, it only deals with the return value.
I haven’t seen the code for this part of the lecture yet, so I’m assuming based on context that GetPhysicsHandle returns a PHC that is attached to the player (PC, Pawn, Character, etc.). So in that scenario, UGrabber::Release would return gracefully if the PHC on the player was nullptr.
In any case, I’m not sure what you mean by ‘forget to put a physics handle on the player’, I’d need to see the code.
Thanks for the clarification.
I just expect this is how I’d most likely end up with a nullptr here, but my take away from this section was that testing before use is best practice to prevent an engine crash from this.
In general, yes, it is usually best practice to guard against dereferencing nullptrs (google ‘defensive programming’ to read up on other common best practices).
I say ‘usually’ here because sometimes during the development cycle we actually want the program to crash if the code tries to access an invalid pointer. The reason for this is that in certain situations we assume that the pointer being dereferenced should always point to a valid object. The problem with checking for nullptr in these situations is that we might end up with undefined behavior at runtime if execution is allowed to continue, which would be the case if we protected against accessing an invalid pointer.
Keep in mind that the intent to crash on an invalid pointer may not always be clear to other team members (or even to yourself as time passes). This is where the ensure and check macros come in handy because they more clearly express the original programmer’s intent. For example:
// Use the ensure macro to show a warning that SomeObjectPtr should
// be valid at this point, but we're allowing execution to continue anyway.
// ensure() will also break into the debugger on first failure if one is attached.
if (!ensure(SomeObjectPtr != nullptr)) { /* warn and continue */ }
// Use the check macro to halt program execution completely.
// check() will also break into the debugger if one is attached.
check(SomeObjectPtr != nullptr); // fatal error if SomeObjectPtr is null.
// or
// Check if SomeObjectPtr is not null and points to a live object that is not pending kill
check(IsValid(SomeObjectPtr));
Lots more info on UE’s built-in assertion macros here.
Lastly, we don’t want the program to crash on the user’s machine, so in general it’s good thoroughly exercise code that assumes pointer validity.
I moved on and it turns out he comes back around to this later in the “Boolean Logical Operators” lecture. Instead of using if/else he has us use the feature of && to evaluate the pointer before using it.
I still find this was an educational diversion and on-point to notice the inefficiency.
You don’t need to use the logical AND operator (&&) to evaluate a pointer before using it, although I
imagine he might have done something like:
if (SomeObjectPtr && SomeObjectPtr->SomeFunction()) ...
In C++, boolean expressions inside if statements will evaluate from left to right, and the statement will ‘bail early’ if the result of an evaluation would otherwise make the other expressions meaningless. So in this example, if SomeObjectPtr is nullptr, the code for SomeObjectPtr->SomeFunction() will never be reached, so you don’t have to worry about the function being called on a null object pointer.
Can you post the code you’re referring to?
Absolutely, but your guess is correct. My code from the top turns into:
void UGrabber::Release()
{
UPhysicsHandleComponent* PhysicsHandle = GetPhysicsHandle();
if (PhysicsHandle && PhysicsHandle->GetGrabbedComponent())
{
PhysicsHandle->ReleaseComponent();
}
}
Oh ok, that makes sense. This is essentially the equivalent of:
// Assuming GetGrabbedComponent returns a pointer to an object
if ( (PhysicsHandle != nullptr) AND (PhysicsHandle->GetGrabbedComponent() != nullptr) )
{ ... }
and is actually a keyword in C++ and is an alternative token for &&.
I used the word AND in my response for illustrative purposes, not meant to be taken literally.
Also, C++ is case-sensitive so AND and and are not the same thing. Additionally, and is a macro defined in iso646.h that must be included, otherwise a compiler error is generated (at least in MSVC it is not considered a ‘native’ keyword):
#include <iso646.h>
if (true and true) return; // OK with header included (at least for MSVC)
if (true AND true) return; // Compiler error. 'AND' must be defined first.
I’m aware to all points, I just wanted to point out you could have just used and.
Only on older versions where MSVC wasn’t fully conforming to the C++11 standard. It compiles on newer versions with /permissive- which is for strict conformance (it’s also applied if /std:c++20 is used and also enabled by default for new Visual Studio projects).
Demo: Compiler Explorer
Yep, makes sense. When I was typing my response I was trying to put the emphasis on ‘AND’ because I had referred to it earlier as the ‘logical AND operator’. I suppose a better form would have been:
if ( handle is not equal to null and grabbed component is not equal to null ) then do work…
Hmm, I’m using Rider with MSVC backend but it didn’t recognize and as a native keyword. I’ll have to look into the project settings. Thanks for the suggestion.
That would be an issue with whatever front end is used. Unreal adds /permissive- to the compiler options so it should still compile
Hmm, that doesn’t seem to be the case unless I’m overlooking something obvious. I created a test project with UE5’s launcher and then tried compiling this line with both Rider and VS 2022:
if (true and true) { /* ... */ }
Both failed with the same error message:
Rider: TestProjectGameModeBase.cpp(10): [C2065] 'and': undeclared identifier
VS 2022: TestProjectGameModeBase.cpp(10): error C2065: 'and': undeclared identifier
Interestingly, I updated the Additional Options for Intellisense to use C++20:
This appeared to allow Intellisense to ‘recognize’ and although it’s not colored like a normal native keyword. The build still failed though since this appears to be just an Intellisense option. TBH I’m not actually sure where to view/edit the compiler options that Unreal generates.
Update: I found the code you posted in VCToolChain.cs. I had to search using Agent Ransack because neither VAX or Rider could find it (some source files appear to live outside of the main project hierarchy). Still find it odd that I’m getting the compiler errors if those switches are supposed to enable use of and as a keyword.
I could have sworn I wrote code that required /permissive- in Unreal before but I guess I’m mistaken as you need to opt into stricter conformance in the BuildConfiguration.xml e.g.
<?xml version="1.0" encoding="utf-8"?>
<Configuration xmlns="https://www.unrealengine.com/BuildConfiguration">
<WindowsPlatform>
<bStrictConformanceMode>true</bStrictConformanceMode>
</WindowsPlatform>
</Configuration>
This topic was automatically closed 20 days after the last reply. New replies are no longer allowed.