Add nodiscard to core math classes to catch c++ errors.

A common source of errors is to call functions (such as round()) expecting them to work in place, but them actually being designed only to return the processed value. Not using the return value in this case in indicative of a bug, and can be flagged as a warning by using the [[nodiscard]] attribute.

core/math/aabb.h

@@ -41,7 +41,7 @@
  */
 class Variant;
 
-class AABB {
+class _NO_DISCARD_ AABB {
 public:
 	Vector3 position;
 	Vector3 size;

core/math/basis.h

@@ -34,7 +34,7 @@
 #include "core/math/quaternion.h"
 #include "core/math/vector3.h"
 
-class Basis {
+class _NO_DISCARD_ Basis {
 private:
 	void _set_diagonal(const Vector3 &p_diag);
 

core/math/color.h

@@ -34,7 +34,7 @@
 #include "core/math/math_funcs.h"
 #include "core/string/ustring.h"
 
-struct Color {
+struct _NO_DISCARD_ Color {
 	union {
 		struct {
 			float r;

core/math/face3.h

@@ -36,7 +36,7 @@
 #include "core/math/transform_3d.h"
 #include "core/math/vector3.h"
 
-class Face3 {
+class _NO_DISCARD_ Face3 {
 public:
 	enum Side {
 		SIDE_OVER,

core/math/plane.h

@@ -35,7 +35,7 @@
 
 class Variant;
 
-class Plane {
+class _NO_DISCARD_ Plane {
 public:
 	Vector3 normal;
 	real_t d = 0;

core/math/quaternion.h

@@ -36,7 +36,7 @@
 #include "core/math/vector3.h"
 #include "core/string/ustring.h"
 
-class Quaternion {
+class _NO_DISCARD_ Quaternion {
 public:
 	union {
 		struct {

core/math/rect2.h

@@ -35,7 +35,7 @@
 
 struct Transform2D;
 
-struct Rect2 {
+struct _NO_DISCARD_ Rect2 {
 	Point2 position;
 	Size2 size;
 
@@ -363,7 +363,7 @@ struct Rect2 {
 	}
 };
 
-struct Rect2i {
+struct _NO_DISCARD_ Rect2i {
 	Point2i position;
 	Size2i size;
 

core/math/transform_2d.h

@@ -33,7 +33,7 @@
 
 #include "core/math/rect2.h" // also includes vector2, math_funcs, and ustring
 
-struct Transform2D {
+struct _NO_DISCARD_ Transform2D {
 	// Warning #1: basis of Transform2D is stored differently from Basis. In terms of elements array, the basis matrix looks like "on paper":
 	// M = (elements[0][0] elements[1][0])
 	//     (elements[0][1] elements[1][1])

core/math/transform_3d.h

@@ -35,7 +35,7 @@
 #include "core/math/basis.h"
 #include "core/math/plane.h"
 
-class Transform3D {
+class _NO_DISCARD_ Transform3D {
 public:
 	Basis basis;
 	Vector3 origin;

core/math/vector2.h

@@ -36,7 +36,7 @@
 
 struct Vector2i;
 
-struct Vector2 {
+struct _NO_DISCARD_ Vector2 {
 	static const int AXIS_COUNT = 2;
 
 	enum Axis {
@@ -284,7 +284,7 @@ typedef Vector2 Point2;
 
 /* INTEGER STUFF */
 
-struct Vector2i {
+struct _NO_DISCARD_ Vector2i {
 	enum Axis {
 		AXIS_X,
 		AXIS_Y,

core/math/vector3.h

@@ -37,7 +37,7 @@
 #include "core/string/ustring.h"
 class Basis;
 
-struct Vector3 {
+struct _NO_DISCARD_ Vector3 {
 	static const int AXIS_COUNT = 3;
 
 	enum Axis {

core/math/vector3i.h

@@ -35,7 +35,7 @@
 #include "core/string/ustring.h"
 #include "core/typedefs.h"
 
-struct Vector3i {
+struct _NO_DISCARD_ Vector3i {
 	enum Axis {
 		AXIS_X,
 		AXIS_Y,

core/typedefs.h

@@ -71,6 +71,17 @@
 #endif
 #endif
 
+// No discard allows the compiler to flag warnings if we don't use the return value of functions / classes
+#ifndef _NO_DISCARD_
+#define _NO_DISCARD_ [[nodiscard]]
+#endif
+
+// In some cases _NO_DISCARD_ will get false positives,
+// we can prevent the warning in specific cases by preceding the call with a cast.
+#ifndef _ALLOW_DISCARD_
+#define _ALLOW_DISCARD_ (void)
+#endif
+
 // Windows badly defines a lot of stuff we'll never use. Undefine it.
 #ifdef _WIN32
 #undef min // override standard definition

editor/import/resource_importer_scene.cpp

@@ -1776,7 +1776,8 @@ void ResourceImporterScene::_optimize_track_usage(AnimationPlayer *p_player, Ani
 						if (bone_idx == -1) {
 							continue;
 						}
-						skel->get_bone_pose(bone_idx);
+						// Note that this is using get_bone_pose to update the bone pose cache.
+						_ALLOW_DISCARD_ skel->get_bone_pose(bone_idx);
 						loc = skel->get_bone_pose_position(bone_idx);
 						rot = skel->get_bone_pose_rotation(bone_idx);
 						scale = skel->get_bone_pose_scale(bone_idx);

platform/osx/display_server_osx.mm

@@ -316,7 +316,7 @@ static NSCursor *_cursorFromSelector(SEL selector, SEL fallback = nil) {
 		CGPoint lMouseWarpPos = { pointOnScreen.x, CGDisplayBounds(CGMainDisplayID()).size.height - pointOnScreen.y };
 		CGWarpMouseCursorPosition(lMouseWarpPos);
 	} else {
-		_get_mouse_pos(wd, [wd.window_object mouseLocationOutsideOfEventStream]);
+		_ALLOW_DISCARD_ _get_mouse_pos(wd, [wd.window_object mouseLocationOutsideOfEventStream]);
 		Input::get_singleton()->set_mouse_position(wd.mouse_pos);
 	}
 
@@ -1391,7 +1391,7 @@ inline void sendPanEvent(DisplayServer::WindowID window_id, double dx, double dy
 
 	double deltaX, deltaY;
 
-	_get_mouse_pos(wd, [event locationInWindow]);
+	_ALLOW_DISCARD_ _get_mouse_pos(wd, [event locationInWindow]);
 
 	deltaX = [event scrollingDeltaX];
 	deltaY = [event scrollingDeltaY];
@@ -2463,7 +2463,7 @@ void DisplayServerOSX::window_set_position(const Point2i &p_position, WindowID p
 	[wd.window_object setFrameTopLeftPoint:NSMakePoint(position.x - offset.x, position.y - offset.y)];
 
 	_update_window(wd);
-	_get_mouse_pos(wd, [wd.window_object mouseLocationOutsideOfEventStream]);
+	_ALLOW_DISCARD_ _get_mouse_pos(wd, [wd.window_object mouseLocationOutsideOfEventStream]);
 }
 
 void DisplayServerOSX::window_set_max_size(const Size2i p_size, WindowID p_window) {

scene/main/canvas_item.cpp

@@ -1017,8 +1017,8 @@ void CanvasItem::set_notify_transform(bool p_enable) {
 	notify_transform = p_enable;
 
 	if (notify_transform && is_inside_tree()) {
-		//this ensures that invalid globals get resolved, so notifications can be received
-		get_global_transform();
+		// This ensures that invalid globals get resolved, so notifications can be received.
+		_ALLOW_DISCARD_ get_global_transform();
 	}
 }
 

scene/resources/immediate_mesh.cpp

@@ -382,7 +382,7 @@ AABB ImmediateMesh::get_aabb() const {
 		if (i == 0) {
 			aabb = surfaces[i].aabb;
 		} else {
-			aabb.merge(surfaces[i].aabb);
+			aabb = aabb.merge(surfaces[i].aabb);
 		}
 	}
 	return aabb;

servers/rendering/renderer_rd/renderer_scene_render_rd.cpp

@@ -514,7 +514,7 @@ Ref<Image> RendererSceneRenderRD::environment_bake_panorama(RID p_env, bool p_ba
 		ambient_color_sky_mix = env->ambient_sky_contribution;
 		const float ambient_energy = env->ambient_light_energy;
 		ambient_color = env->ambient_light;
-		ambient_color.to_linear();
+		ambient_color = ambient_color.to_linear();
 		ambient_color.r *= ambient_energy;
 		ambient_color.g *= ambient_energy;
 		ambient_color.b *= ambient_energy;
@@ -533,7 +533,7 @@ Ref<Image> RendererSceneRenderRD::environment_bake_panorama(RID p_env, bool p_ba
 	} else {
 		const float bg_energy = env->bg_energy;
 		Color panorama_color = ((environment_background == RS::ENV_BG_CLEAR_COLOR) ? storage->get_default_clear_color() : env->bg_color);
-		panorama_color.to_linear();
+		panorama_color = panorama_color.to_linear();
 		panorama_color.r *= bg_energy;
 		panorama_color.g *= bg_energy;
 		panorama_color.b *= bg_energy;