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

The PR moves builder disable handling into the relevant behavior modules and preserves eligible tasks for resumption while refusing to restart completed construction.

Adds behavior-module notification on disabled-state transitions.
Snapshots active build, repair, or fortify tasks for temporary disables.
Validates construction status before resuming build tasks.
Keeps the Generals and GeneralsMD implementations aligned.
Corrects previous-task save-transfer version checks.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp	Implements guarded task preservation and complete build, repair, and fortify resumption for Generals dozers.
Generals/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/WorkerAIUpdate.cpp	Mirrors the corrected disable and task-resumption lifecycle for Generals workers.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/DozerAIUpdate.cpp	Applies the corrected dozer task lifecycle to the Zero Hour variant.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Update/AIUpdate/WorkerAIUpdate.cpp	Applies the corrected worker task lifecycle to the Zero Hour variant.
Generals/Code/GameEngine/Source/GameLogic/Object/Object.cpp	Dispatches disabled-state edges to behavior modules so builder-specific handling runs in the owning module.
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Object.cpp	Removes direct dozer handling after responsibility moves into builder behavior modules.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Builder disabled-state edge] --> B{Becoming disabled?}
    B -- Yes --> C{Active task?}
    C -- No --> D[No task cancellation]
    C -- Yes --> E{Temporary resumable disable type?}
    E -- Yes --> F[Snapshot task and target]
    E -- No --> G[Cancel without remembering]
    F --> H[Cancel active task]
    G --> H
    B -- No --> I{Remembered task type}
    I -- Build --> J{Target still under construction?}
    J -- Yes --> K[Resume build]
    J -- No --> L[Do not resume completed build]
    I -- Repair or Fortify --> M{Target still exists?}
    M -- Yes --> N[Resume task]
    M -- No --> O[Discard stale task]
    K --> P[Clear remembered task]
    L --> P
    N --> P
    O --> P

_{Reviews (13): Last reviewed commit: "chore: Move previous-task methods from t..." | 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.

Question still stands. Why are they virtual functions?

They are virtual simply because it follows the existing pattern and allows WorkerAIUpdate to override them. It helps convey that it is shared functionality. I can make them local if that's preferred?

So the question that needs to be asked where a third party is allowed to call these functions at any time. When part of DozerAIInterface, then they are public and meant to be to use for anybody. If the function are for internal use, then they need to move out of the interface and into interface implementations, either private or protected, and if they need to be overridable, then they can be virtual too.

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-03T14:15:07Z

 	// task actions
 	virtual void newTask( DozerTask task, Object* target ) override;	///< set a desire to do the requrested task
-	virtual void cancelTask( DozerTask task ) override;						///< 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 ) override;	///< cancel this task from the queue, if it's the current task the dozer will stop working on it. Can remember the cancelled task for resumption (e.g. after the unit is disabled).


The example in parentheses is ambiguous. It can be read as if the task is resumed after a unit is disabled. Better just omit the example.

That's how it was supposed to be read, but okay.

A unit will resume task after it is re-enabled

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

Comment thread Generals/Code/GameEngine/Include/GameLogic/Module/DozerAIUpdate.h Outdated

xezon reviewed Sep 3, 2026

View reviewed changes

docs: Explain new method argument

a87efc3

Stubbjax force-pushed the fix-previous-dozer-task branch from 558b3cf to a87efc3 Compare September 3, 2026 14:34

chore: Move previous-task methods from the interface to the implementers

47ccf55

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

Flowchart

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!

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!

OmarAglan commented Sep 1, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

greptile-apps Bot commented Jun 14, 2026 •

edited

Loading

Stubbjax Jun 14, 2026 •

edited

Loading