Fix C4709 Warnings In modern VS by TheForce172 · Pull Request #7576 · scp-fs2open/fs2open.github.com

TheForce172 · 2026-07-07T19:15:53Z

Fixes https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4709?view=msvc-170 errors

TheForce172 · 2026-07-07T22:04:03Z

Drafted pending testing and further discussion

Goober5000

One change, plus two missed sites:
fred2/fredrender.cpp:1429
fred2/fredview.cpp:1466

Plus, if we're going to convert everything, we might as well entirely remove the operator overload in flagset.h, or change it to operator|.

EDIT: Or, as Claude suggested, adding an any_of function.

BMagnu · 2026-07-08T02:13:20Z

I generally agree that replacing the operator with | is probably safer

Goober5000 · 2026-07-08T03:58:14Z

Suggested functions, written by Claude:

// Returns true if at least one of the given flags is set.
// Replacement for the flag_combinator subscript idiom (flags[A, B]),
// which trips MSVC warning C4709.
template<typename... Ts>
bool any_of(T first, Ts... rest) const
{
    static_assert((std::is_same<Ts, T>::value && ...),
        "All arguments to any_of() must be flags of the same enum type");
    return (*this)[first] || ((*this)[rest] || ...);
}

// Returns true if none of the given flags are set.
template<typename... Ts>
bool none_of(T first, Ts... rest) const
{
    return !any_of(first, rest...);
}

Goober5000

Miscellaneous items. Also, test_flagset.cpp (part of the test project, not always built) still uses the comma.

Goober5000 · 2026-07-09T02:41:12Z

 				// Cyborg -- Unfortunately Ai info is not reliable and should just not be accessed here.
 				// It will have no real effect on gameplay, since the server decides when turrets fire
-				if (!MULTIPLAYER_CLIENT && (aip->ai_flags[AI::AI_Flags::Being_repaired, AI::AI_Flags::Awaiting_repair]))
+				if (!MULTIPLAYER_CLIENT && // NOLINT(readability-simplify-boolean-expr)


This probably doesn't need NOLINT now

Goober5000 · 2026-07-09T02:41:15Z

 	}

-	if (aip->ai_flags[AI::AI_Flags::No_dynamic, AI::AI_Flags::Kamikaze]) {	//	If not allowed to pursue dynamic objectives, don't evade.  Dumb?  Maybe change. -- MK, 3/15/98
+	if (aip->ai_flags.any_of(AI::AI_Flags::No_dynamic,AI::AI_Flags::Kamikaze)) { //	If not allowed to pursue dynamic objectives, don't evade.  Dumb?


The comment got split over two lines in this change

Goober5000 · 2026-07-09T02:42:24Z

 	aip->last_hit_time = Missiontime;

-	if (aip->ai_flags[AI::AI_Flags::No_dynamic, AI::AI_Flags::Kamikaze])	//	If not allowed to pursue dynamic objectives, don't evade.  Dumb?  Maybe change. -- MK, 3/15/98
+	if (aip->ai_flags.any_of(AI::AI_Flags::No_dynamic,AI::AI_Flags::Kamikaze)) //	If not allowed to pursue dynamic objectives, don't evade.  Dumb?


This comment got split too

Goober5000 · 2026-07-09T02:46:32Z

 	bool is_arriving(ship::warpstage stage = ship::warpstage::BOTH, bool dock_leader_or_single = false) const;
-	inline bool is_departing() const { return flags[Ship::Ship_Flags::Depart_warp, Ship::Ship_Flags::Depart_dockbay]; }
-	inline bool cannot_warp_flags() const { return flags[Ship::Ship_Flags::Warp_broken, Ship::Ship_Flags::Warp_never, Ship::Ship_Flags::Disabled, Ship::Ship_Flags::No_subspace_drive]; }
+	inline bool is_departing() const


Would be nice to keep this all on one line like the others

Goober5000 · 2026-07-09T02:47:03Z

+    inline bool is_small_ship() const
+	{
+		return flags[Ship::Info_Flags::Fighter] || flags[Ship::Info_Flags::Bomber] || flags[Ship::Info_Flags::Support]
+			   || flags[Ship::Info_Flags::Escapepod];
+	}
+	inline bool is_big_ship() const
+	{
+		return flags[Ship::Info_Flags::Cruiser] || flags[Ship::Info_Flags::Freighter] ||
+			   flags[Ship::Info_Flags::Transport] ||
+			   flags[Ship::Info_Flags::Corvette] ||
+			   flags[Ship::Info_Flags::Gas_miner] || flags[Ship::Info_Flags::Awacs];
+	}
+	inline bool is_huge_ship() const
+	{
+		return flags[Ship::Info_Flags::Capital] || flags[Ship::Info_Flags::Supercap] ||
+			   flags[Ship::Info_Flags::Drydock] || flags[Ship::Info_Flags::Knossos_device];
+	}
+	inline bool is_flyable() const
+	{
+		return !(
+			flags[Ship::Info_Flags::Cargo] || flags[Ship::Info_Flags::Navbuoy] || flags[Ship::Info_Flags::Sentrygun]);
+	} // AL 11-24-97: this useful to know for targeting reasons
+	  // note: code that previously used is_harmless() / SIF_HARMLESS now uses several flags defined in objecttypes.tbl
 //	inline bool is_harmless() const { return flags[Ship::Info_Flags::Cargo, Ship::Info_Flags::Navbuoy, Ship::Info_Flags::Escapepod]; }		// AL 12-3-97: ships that are not a threat
-    inline bool is_fighter_bomber() const { return flags[Ship::Info_Flags::Fighter, Ship::Info_Flags::Bomber]; }
+	inline bool is_fighter_bomber() const
+	{
+		return flags[Ship::Info_Flags::Fighter] || flags[Ship::Info_Flags::Bomber];
+	}


Similarly it would be nice to keep these all on one line each. These still use the || operators so they could be condensed with the new functions.

Goober5000 · 2026-07-09T02:47:37Z

-
+    inline bool is_homing() const
+	{
+		return wi_flags.any_of(Weapon::Info_Flags::Homing_heat,Weapon::Info_Flags::Homing_aspect,Weapon::Info_Flags::Homing_javelin);
+	}
+	inline bool is_locked_homing() const
+	{
+		return wi_flags.any_of(Weapon::Info_Flags::Homing_aspect,Weapon::Info_Flags::Homing_javelin);
+	}
+	inline bool hurts_big_ships() const
+	{
+		return wi_flags.any_of(Weapon::Info_Flags::Bomb,Weapon::Info_Flags::Beam,Weapon::Info_Flags::Huge,Weapon::Info_Flags::Big_only);
+	}
+	inline bool is_interceptable() const
+	{
+		return wi_flags.any_of(Weapon::Info_Flags::Fighter_Interceptable,Weapon::Info_Flags::Turret_Interceptable);
+	}
+	inline bool is_mine()               const { return wi_flags[Weapon::Info_Flags::Mine]; }
+


Again with the one-line functions

Goober5000 · 2026-07-09T02:57:19Z

+		return (*this)[first] || ((*this)[rest] || ...);
+	}

+	// Returns true if none of the given flags are set.


none_of has a comment but any_of doesn't

TheForce172 added 3 commits July 7, 2026 19:59

Fix C4709 Warnings on latest VS

e94fc70

more

a77eb99

suppress false positive

4608ac9

TheForce172 marked this pull request as draft July 7, 2026 22:03

Goober5000 requested changes Jul 8, 2026

View reviewed changes

Comment thread code/particle/volumes/ModelSurfaceVolume.cpp

Switch to using any/none_of

c08d4fd

Goober5000 reviewed Jul 9, 2026

View reviewed changes

Address Feedback

61cf6b1

TheForce172 marked this pull request as ready for review July 14, 2026 11:43

Uh oh!

Conversation

TheForce172 commented Jul 7, 2026

Uh oh!

TheForce172 commented Jul 7, 2026

Uh oh!

Goober5000 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

BMagnu commented Jul 8, 2026

Uh oh!

Goober5000 commented Jul 8, 2026

Uh oh!

Goober5000 left a comment

Choose a reason for hiding this comment

Uh oh!

Goober5000 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Goober5000 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Goober5000 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Goober5000 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Goober5000 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Goober5000 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Goober5000 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Goober5000 left a comment •

edited

Loading