refactor(bitflags): Simplify functions in BitFlags class

[xezon]

• Merge after refactor(bitflags): Simplify conditions with testForAny, testForAll and remove unexpected assert in BitFlags::testForAll #2476

This change simplifies the function implementations in the BitFlags class.

If I did not make a mistake it should behave exactly the same.

TODO

• Replicate in Generals
• Test in Game

[greptile-apps]

Greptile Summary

This PR refactors the BitFlags template class in both the Generals and Zero Hour (GeneralsMD) codebases, replacing verbose temporary-variable patterns with direct, idiomatic std::bitset operations. The two files are kept in sync as mirrors of each other.

Key changes:

• testForAny, testForNone, countIntersection, countInverseIntersection, and anyIntersectionWith are all simplified by computing the result inline using bitwise operators instead of creating a mutable BitFlags tmp copy. The logic is equivalent.
• testForAll is simplified from the "flip-then-AND" pattern to (m_bits & that.m_bits) == that.m_bits, which is logically equivalent — and the DEBUG_ASSERTCRASH for a zero-bit argument was intentionally removed (necessary to allow testSetAndClear to route through testForAll without falsely asserting on a zero mustBeSet mask).
• testSetAndClear now delegates clearly to testForNone(mustBeClear) && testForAll(mustBeSet), eliminating the @todo improve me comment. Short-circuit evaluation preserves the original early-return semantics.
• clearAndSet now delegates to the clear() and set() member overloads; parameters were also renamed to be more descriptive (flagsToClear, flagsToSet).
• operator== and operator!= drop the unnecessary this-> prefix.
• Note: anyIntersectionWith and testForAny now have identical implementations — they were already semantically the same before the refactor; this is a pre-existing design choice left untouched.

Confidence Score: 4/5

• This PR is safe to merge; all logic transformations are correct and the one behavioral change (assertion removal) is intentional and necessary.
• All simplified expressions are mathematically equivalent to the originals. The removal of DEBUG_ASSERTCRASH from testForAll is a deliberate trade-off to make testSetAndClear delegation work safely — the return-value semantics of testForAll are unchanged for zero-flag inputs (still returns true). The score is 4 rather than 5 only because removing an existing debug guard is a mild regression in debug-build safety for direct callers of testForAll.
• No files require special attention; both Generals and GeneralsMD copies are in sync.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Include/Common/BitFlags.h	All refactored operations are logically equivalent to the originals. The intentional removal of DEBUG_ASSERTCRASH from testForAll weakens debug-time detection of zero-flag arguments on direct testForAll call sites (beyond testSetAndClear), but is necessary for the testSetAndClear delegation to work correctly.
GeneralsMD/Code/GameEngine/Include/Common/BitFlags.h	Identical changes mirrored from the Generals copy. Same analysis applies — logic is correct and the assertion removal is intentional.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    TSC["testSetAndClear(mustBeSet, mustBeClear)"]
    TFN["testForNone(mustBeClear)\n(m_bits & that.m_bits).none()"]
    TFA["testForAll(mustBeSet)\n(m_bits & that.m_bits) == that.m_bits"]
    RET_FALSE["return false"]
    RET_TRUE["return true"]

    TSC -->|"short-circuit &&"| TFN
    TFN -->|"false"| RET_FALSE
    TFN -->|"true"| TFA
    TFA -->|"false"| RET_FALSE
    TFA -->|"true"| RET_TRUE

Loading

_{Last reviewed commit: "Remove the assert"}

[Mauller]

Looks okay to me, was slightly confused by some at first when they were saying All must be set.