bugfix: Fix issue where builders could resume completed tasks after being disabled by Stubbjax · Pull Request #2793 · TheSuperHackers/GeneralsGameCode

Stubbjax · 2026-06-14T15:41:05Z

This change fixes an issue introduced by #1870 that allows builders to resume an already completed task after being disabled.

When assigning a new build task to a builder, if the target building is an already-completed building, then the new task is ignored.

Before

BEFORE.mp4

After

AFTER.mp4

greptile-apps · 2026-06-14T15:43:49Z

Greptile Summary

This PR fixes a regression from #1870 where a builder (Dozer/Worker) re-enabled after a temporary disable (EMP, hack, underpowered) could incorrectly resume a build task whose target was already fully constructed. The fix moves the disable/resume logic out of Object::onDisabledEdge and into new onDisabledEdge overrides on DozerAIUpdate and WorkerAIUpdate, adding a guard in resumePreviousTask that skips the newTask call when the target building is no longer OBJECT_STATUS_UNDER_CONSTRUCTION.

Introduces setPreviousTask / clearPreviousTask helpers and a rememberTask parameter on cancelTask; the task is only saved for EMP/hack/subdued/underpowered disables, not for container entry (DISABLED_HELD).
Also fixes a latent xfer bug (currentVersion >= 2 → version >= 2) that would have incorrectly written previous-task data when saving old-format saves in both Generals and GeneralsMD.
Three clearPreviousTask declarations/definitions in the GeneralsMD/ tree use 2-space indentation instead of tabs.

Confidence Score: 5/5

The logic changes are correct and well-scoped: the build-complete guard, the REPAIR/FORTIFY resume branch, the DOZER_TASK_INVALID guard in setPreviousTask, and the xfer variable name fix are all sound. The only findings are minor indentation inconsistencies in the GeneralsMD tree and a date annotation referencing 2025.
All task-type paths in resumePreviousTask are now handled, clearPreviousTask is called unconditionally at the end so m_previousTask is never left stale, and internalCancelTask correctly no longer writes to m_previousTask unconditionally. The xfer variable name fix prevents incorrect save/load behavior on older saves. No functional regressions were identified.
The three clearPreviousTask sites in GeneralsMD/ (both headers and DozerAIUpdate.cpp) have a 2-space vs tab indentation mismatch worth correcting before merge.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp	Implements `setPreviousTask`, `clearPreviousTask`, `onDisabledEdge`; refactors `resumePreviousTask` to skip resumed builds whose target has already completed; fixes `currentVersion` → `version` in `xfer`.
Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/WorkerAIUpdate.cpp	Parallel implementation to DozerAIUpdate.cpp for the Worker unit, including the `xfer` variable name fix.
GeneralsMD/Code/GameEngine/Include/GameLogic/Module/DozerAIUpdate.h	Interface updated to match Generals version; `clearPreviousTask` declaration uses 2-space indentation while surrounding declarations use tabs.
GeneralsMD/Code/GameEngine/Include/GameLogic/Module/WorkerAIUpdate.h	Same as DozerAIUpdate.h — `clearPreviousTask` override declaration uses 2-space indentation instead of tabs.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp	Removes the now-redundant dozerAI-specific block from `onDisabledEdge`; the logic has been moved into the modules themselves and is invoked via the existing behavior-module loop.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp	Matches Generals implementation; `clearPreviousTask` function body uses 2-space indentation instead of tabs, inconsistent with surrounding code. `xfer` variable name fix applied correctly.

Sequence Diagram

sequenceDiagram
    participant Obj as Object
    participant Mod as BehaviorModule loop
    participant DAI as DozerAIUpdate / WorkerAIUpdate

    Note over Obj: Builder gets EMP'd / hacked / underpowered
    Obj->>Mod: onDisabledEdge(true)
    Mod->>DAI: onDisabledEdge(true)
    DAI->>DAI: "isDisabledByType(EMP|HACKED|SUBDUED|UNDERPOWERED)?"
    alt "rememberTask = true"
        DAI->>DAI: "setPreviousTask(currentTask)<br/>copies m_task[task] → m_previousTaskInfo"
        DAI->>DAI: "internalCancelTask(task)<br/>clears m_task[task]"
    else "rememberTask = false (e.g. DISABLED_HELD)"
        DAI->>DAI: "internalCancelTask(task)<br/>no memory saved"
    end

    Note over Obj: Disable expires (re-enabled)
    Obj->>Mod: onDisabledEdge(false)
    Mod->>DAI: onDisabledEdge(false)
    DAI->>DAI: resumePreviousTask()
    alt "previousTask == BUILD && target still UNDER_CONSTRUCTION"
        DAI->>DAI: newTask(BUILD, target) — resume
    else "previousTask == BUILD && target complete"
        DAI->>DAI: clearPreviousTask() — stay idle (bug fix)
    else "previousTask == REPAIR or FORTIFY && target exists"
        DAI->>DAI: "newTask(REPAIR|FORTIFY, target) — resume"
    else target gone or no previousTask
        DAI->>DAI: clearPreviousTask() — stay idle
    end

_{Reviews (10): Last reviewed commit: "bugfix: Cancelling all tasks now clears ..." | Re-trigger Greptile}

xezon · 2026-06-14T15:57:21Z

 		if( task == DOZER_TASK_BUILD )
+		{
+			// TheSuperHackers @bugfix Stubbjax 15/06/2026 Ignore the build task if the building is already complete.
+			if (target->getConstructionPercent() == CONSTRUCTION_COMPLETE)


Should this perhaps go into DozerAIUpdate::resumePreviousTask and here it should be an assert?

Good point. I suppose it could seeing as this is the only way it can happen. Having to resolve the target from the task info does make the code a bit more complex though.

xezon · 2026-06-14T16:32:07Z

+
+			DozerAIInterface* dozerAI = getAI() ? getAI()->getDozerAIInterface() : nullptr;
+			if (dozerAI)
+				dozerAI->setPreviousTask(dozerAI->getCurrentTask());


This looks a bit strange. Wasn't cancelTask supposed to take of this? For example from Object::onDisabledEdge.

Yes, but there was no distinction on how the disabling happened here. A unit entering a transport causes it to become disabled, and thus it would attempt to resume the task after exiting the transport.

setPreviousTask is confusing and can be removed again and

m_previousTask = task; m_previousTaskInfo = m_task[task];

readded to internalCancelTask.

I tested that with the EMP test case shown in the video and it worked.

The problem with this approach is that commanding a currently-constructing Dozer to enter a Chinook will have the Dozer attempt to resume the construction after exiting the Chinook.

Yeah true. Maybe capture should not return to internalCancelTask. Could we move the capture into Object::onDisabledEdge(TRUE) before cancelTask, gated on DISABLED_UNDERPOWERED, DISABLED_EMP, DISABLED_SUBDUED, or DISABLED_HACKED? The disabled mask is set by then, the capture becomes symmetric with resumption, and it is no longer coupled to the audio-deduplication block. Could also use the !RETAIL_COMPATIBLE_CRC guard as the resume path.

Has been a while I looked at this but it would be good to make enter and exit states symmetric and intuitive.

xezon · 2026-06-29T16:21:22Z

@Stubbjax Is this now fixed up?

Stubbjax · 2026-07-01T15:45:28Z

@Stubbjax Is this now fixed up?

It should be.

xezon · 2026-07-04T16:20:06Z

+
+			DozerAIInterface* dozerAI = getAI() ? getAI()->getDozerAIInterface() : nullptr;
+			if (dozerAI)
+				dozerAI->setPreviousTask(dozerAI->getCurrentTask());


setPreviousTask is confusing and can be removed again and

m_previousTask = task; m_previousTaskInfo = m_task[task];

readded to internalCancelTask.

I tested that with the EMP test case shown in the video and it worked.

xezon · 2026-07-21T17:28:17Z

This needs polishing.

bobtista · 2026-07-28T15:54:40Z

+	if (m_previousTask == DOZER_TASK_INVALID)
+		return;
+
+	if (m_previousTask == DOZER_TASK_BUILD)


This changes behavior for DOZER_TASK_REPAIR. In non-retail-compatible builds, main attempts to reissue any saved task, but this implementation only reissues DOZER_TASK_BUILD and then clears the saved repair task. Is this what we want?
If not, could have the OBJECT_STATUS_UNDER_CONSTRUCTION check remain specific to BUILD while preserving existing non-build resumption behavior. DOZER_TASK_FORTIFY follows the same code path, but does anything use it? This applies to the mirrored implementations too.

xezon

Please continue work on this fix.

…eing disabled

…sabled

xezon · 2026-08-24T19:38:11Z

+				Bool attemptToResumeTask = isDisabledByType(DISABLED_EMP) ||
+					isDisabledByType(DISABLED_HACKED) ||
+					isDisabledByType(DISABLED_SUBDUED) ||
+					isDisabledByType(DISABLED_UNDERPOWERED);


Are these 4 complete?

Maybe do the checklist by exclusion instead?

Bool attemptToResumeTask = !isDisabledByType(DISABLED_HELD) && ... ;

Yes, this mirrors the conditions in Object::setDisabledUntil and Object::clearDisabled.

Ok this link is rather unintuitive. Can we perhaps consolidate the conditions across the 3 places so that they are unlikely to go out of sync?

Would it make more sense to do such refactors to unrelated logic in a subsequent change?

Yes if it is not forgotten. It's a side quest spawned from this.

…kerAIUpdate

xezon · 2026-09-01T14:35:03Z

 	virtual void cancelAllTasks() = 0;													///< cancel all tasks from the queue, if it's the current task the dozer will stop working on it
 	virtual void setPreviousTask(DozerTask task) = 0;						///< set the previous task
 	virtual void resumePreviousTask() = 0;									///< resume the previous task if there was one
+	virtual void clearPreviousTask() = 0;										///< clear the previous task


Does it need to be virtual function? If it is only called internally in the implementation, then it can be implementation local. Same for the other Task functions here.

OmarAglan · 2026-09-01T14:51:58Z

@codex

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3886b38525

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

xezon · 2026-09-02T08:06:11Z

 	// task actions
 	virtual void newTask( DozerTask task, Object *target ) = 0;	///< set a desire to do the requrested task
-	virtual void cancelTask( DozerTask task ) = 0;							///< cancel this task from the queue, if it's the current task the dozer will stop working on it
+	virtual void cancelTask( DozerTask task, Bool rememberTask = false ) = 0;							///< cancel this task from the queue, if it's the current task the dozer will stop working on it


The EA comment can be expanded that if rememberTask is set that it will resume the task when enabled again.

Stubbjax self-assigned this Jun 14, 2026

Stubbjax added Bug Something is not working right, typically is user facing Major Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour labels Jun 14, 2026

xezon added the ThisProject The issue was introduced by this project, or this task is specific to this project label Jun 14, 2026

xezon reviewed Jun 14, 2026

View reviewed changes

Stubbjax force-pushed the fix-previous-dozer-task branch from 657688e to a0fbee6 Compare June 14, 2026 15:59

greptile-apps Bot reviewed Jun 14, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp

Stubbjax force-pushed the fix-previous-dozer-task branch from de2f3f1 to bdf1ab1 Compare June 14, 2026 16:17

xezon reviewed Jun 14, 2026

View reviewed changes

Stubbjax force-pushed the fix-previous-dozer-task branch from 74beb35 to a4ffdb5 Compare June 14, 2026 16:52

xezon reviewed Jun 14, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp Outdated

Comment thread Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp Outdated

This comment was marked as outdated.

Sign in to view

xezon reviewed Jul 4, 2026

View reviewed changes

Stubbjax force-pushed the fix-previous-dozer-task branch from ede3cb1 to 2f69024 Compare July 28, 2026 15:02

bobtista reviewed Jul 28, 2026

View reviewed changes

xezon reviewed Aug 23, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp Outdated

Stubbjax added 8 commits August 23, 2026 23:55

bugfix: Fix issue where builders could resume completed tasks after b…

68743ff

…eing disabled

bugfix: Only save the previous task for resumption when explicitly di…

8b4cd2e

…sabled

refactor: Move fix to the task resumption method

0c66471

docs: Remove superfluous comments

6cf851a

refactor: Check construction status bit instead of construction percent

dc11862

tweak: Still clear previous task if the resumed build fails

122edf0

refactor: Optimise target acquisition

04e0119

fix: Reverse condition

cbb639a

Stubbjax added 3 commits August 24, 2026 01:19

bugfix: Apply correct version condition

2145c6a

tweak: Support resumption of repair tasks

cb68740

refactor: Streamline previous task assignment

506f141

Stubbjax force-pushed the fix-previous-dozer-task branch from 2f69024 to 506f141 Compare August 23, 2026 15:32

xezon reviewed Aug 24, 2026

View reviewed changes

Stubbjax added 3 commits September 1, 2026 20:22

refactor: Push down onDisabledEdge behaviour to DozerAIUpdate and Wor…

5f41b3b

…kerAIUpdate

refactor: Remember task as part of cancellation

155c6d4

refactor: Consolidate previous task clearing logic

3886b38

greptile-apps Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp Outdated

xezon reviewed Sep 1, 2026

View reviewed changes

bugfix: Also resume FORTIFY tasks

6bc5dac

chatgpt-codex-connector Bot reviewed Sep 1, 2026

View reviewed changes

Comment thread Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp

bugfix: Cancelling all tasks now clears the previous task

3c36a85

xezon reviewed Sep 2, 2026

View reviewed changes

Uh oh!

Conversation

Stubbjax commented Jun 14, 2026

Before

After

Uh oh!

greptile-apps Bot commented Jun 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Stubbjax Jun 14, 2026 • 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!

Uh oh!

Uh oh!

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!

Uh oh!

This comment was marked as outdated.

xezon commented Jun 29, 2026

Uh oh!

Stubbjax commented Jul 1, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xezon commented Jul 21, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

OmarAglan commented Sep 1, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

greptile-apps Bot commented Jun 14, 2026 •

edited

Loading

Stubbjax Jun 14, 2026 •

edited

Loading