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

Two more minor things

Goober5000 · 2026-07-18T20:08:38Z

 	}

-	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? //Maybe change. -- MK, 3/15/98


Slashes got added to the middle of the comment at the end of the line. The comment ought not to have any changes so the diff will be clean between revisions.

Goober5000 · 2026-07-18T20:09:11Z

 	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? //Maybe change. -- MK, 3/15/98


Same here with the slashes

Goober5000

In addition to the line endings, the flag_combinator items in flagset.h can now be removed.

Goober5000 · 2026-07-19T00:47:53Z

+		ASSERT_TRUE((flags.any_of(TestFlags::First, TestFlags::Second)));
+		ASSERT_TRUE((flags.any_of(TestFlags::First, TestFlags::Another)));
+		ASSERT_FALSE((flags.any_of(TestFlags::AndAnother, TestFlags::Another)));


This file shows whitespace changes affecting every line, when only the flag test lines actually changed. I'm guessing this is a line-ending mismatch. Please fix the file line endings so that only the meaningful lines show as changed.

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

TheForce172 requested a review from Goober5000 July 18, 2026 15:37

Goober5000 requested changes Jul 18, 2026

View reviewed changes

Goober5000 requested changes Jul 19, 2026

View reviewed changes

TheForce172 added 2 commits July 19, 2026 11:06

Fix Line endings?

efe86ec

Fix commits and strip out flag_combinator

646f71f

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Goober5000 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Goober5000 left a comment

Choose a reason for hiding this comment

Uh oh!

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