Skip to content

bugfix: Modules now cease updating when disabled by non-whitelisted disabled types #2458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Stubbjax wants to merge 1 commit into
base: main
Choose a base branch
from
Open

bugfix: Modules now cease updating when disabled by non-whitelisted disabled types #2458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions Generals/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3188,7 +3188,15 @@ void GameLogic::update()
{
UpdateModulePtr u = *it;
DisabledMaskType dis = u->friend_getObject()->getDisabledFlags();
#if RETAIL_COMPATIBLE_CRC
if (!dis.any() || dis.anyIntersectionWith(u->getDisabledTypesToProcess()))
#else
// TheSuperHackers @bugfix Stubbjax 15/03/2026 The disabled-types-to-process mask is now exclusive.
// Previously, if the disabled mask had any bits in common with the disabled-types-to-process mask,
// the update would be processed. Now, if any *other* bits are set in the disabled mask, the update
// is no longer processed.
if (!dis.any() || u->getDisabledTypesToProcess().testForAll(dis))
Copy link
Copy Markdown

@xezon xezon Mar 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this certainly correct?

I would have expected:

const Bool isDisabled = dis.any() && dis.testForAny(u->getDisabledTypesToProcess());
if (!isDisabled)
...

Copy link
Copy Markdown
Author

@Stubbjax Stubbjax Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The getDisabledTypesToProcess function returns the disabled statuses that do not block the module's update.

For example, BaseRegenerateUpdate::getDisabledTypesToProcess returns DISABLED_UNDERPOWERED. This means the module is allowed to continue performing its repair update even if the player has low power.

Similarly, PoisonedBehaviour::getDisabledTypesToProcess returns DISABLEDMASK_ALL because nothing should disable the poisoned update. This is highlighted by the accompanying comment "we should still poison disabled things".

The function serves as a whitelist of disabled types that do not affect the update of the respective module. Perhaps a better name would be getAllowedDisabledTypes or getIgnoredDisabledTypes.

Copy link
Copy Markdown

@xezon xezon Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok got it. But why is this condition still correct then?

So what we need to test is: (dis & ~allowedDisabledTypes) == 0

Is u->getDisabledTypesToProcess().testForAll(dis) equivalent to that? It is so complicated to read. Can we write that easier?

Copy link
Copy Markdown

@xezon xezon Mar 21, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So after cleaning up BitFlags function and getting a better understanding of what each of these functions do, I think

if (!dis.any() || u->getDisabledTypesToProcess().testForAll(dis))
{
  // do the update ...
}

should be

const DisabledMaskType disallowedDisabledTypes = u->getDisabledTypesToProcess().flip();
if (dis.testForNone(disallowedDisabledTypes))
{
  // do the update ...
}

Can you double check?

Copy link
Copy Markdown
Author

@Stubbjax Stubbjax Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't that the same thing but in reverse? What u->getDisabledTypesToProcess().testForAll(dis)) does is essentially check that all bits set in dis (the current status) are also set in the allowed mask. If dis has any bits that are not in the allowed mask, then testForAll returns false and the update is skipped.

Copy link
Copy Markdown

@xezon xezon Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right. But then you can omit !dis.any() as well. It is implicitly true when dis is empty. The first version is somehow much more complicated to understand for me in this context, because it uses the test-against flags as our primary, which requires mental gymnastics.

#endif
{
USE_PERF_TIMER(GameLogic_update_normal)

Expand Down Expand Up @@ -3228,7 +3236,15 @@ void GameLogic::update()
UpdateSleepTime sleepLen = UPDATE_SLEEP_NONE; // default, if it is disabled.

DisabledMaskType dis = u->friend_getObject()->getDisabledFlags();
#if RETAIL_COMPATIBLE_CRC
if (!dis.any() || dis.anyIntersectionWith(u->getDisabledTypesToProcess()))
#else
// TheSuperHackers @bugfix Stubbjax 15/03/2026 The disabled-types-to-process mask is now exclusive.
// Previously, if the disabled mask had any bits in common with the disabled-types-to-process mask,
// the update would be processed. Now, if any *other* bits are set in the disabled mask, the update
// is no longer processed.
if (!dis.any() || u->getDisabledTypesToProcess().testForAll(dis))
#endif
{
USE_PERF_TIMER(GameLogic_update_sleepy)

Expand Down
16 changes: 16 additions & 0 deletions GeneralsMD/Code/GameEngine/Source/GameLogic/System/GameLogic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3716,7 +3716,15 @@ void GameLogic::update()
{
UpdateModulePtr u = *it;
DisabledMaskType dis = u->friend_getObject()->getDisabledFlags();
#if RETAIL_COMPATIBLE_CRC
if (!dis.any() || dis.anyIntersectionWith(u->getDisabledTypesToProcess()))
#else
// TheSuperHackers @bugfix Stubbjax 15/03/2026 The disabled-types-to-process mask is now exclusive.
// Previously, if the disabled mask had any bits in common with the disabled-types-to-process mask,
// the update would be processed. Now, if any *other* bits are set in the disabled mask, the update
// is no longer processed.
if (!dis.any() || u->getDisabledTypesToProcess().testForAll(dis))
#endif
{
USE_PERF_TIMER(GameLogic_update_normal)

Expand Down Expand Up @@ -3756,7 +3764,15 @@ void GameLogic::update()
UpdateSleepTime sleepLen = UPDATE_SLEEP_NONE; // default, if it is disabled.

DisabledMaskType dis = u->friend_getObject()->getDisabledFlags();
#if RETAIL_COMPATIBLE_CRC
if (!dis.any() || dis.anyIntersectionWith(u->getDisabledTypesToProcess()))
#else
// TheSuperHackers @bugfix Stubbjax 15/03/2026 The disabled-types-to-process mask is now exclusive.
// Previously, if the disabled mask had any bits in common with the disabled-types-to-process mask,
// the update would be processed. Now, if any *other* bits are set in the disabled mask, the update
// is no longer processed.
if (!dis.any() || u->getDisabledTypesToProcess().testForAll(dis))
#endif
{
USE_PERF_TIMER(GameLogic_update_sleepy)

Expand Down
Loading