bugfix(victory): Save victory status to prevent early exits from resulting in defeat in network matches by Stubbjax · Pull Request #2292 · TheSuperHackers/GeneralsGameCode

Stubbjax · 2026-02-11T05:55:23Z

This change fixes an issue where a player's victory is not reliably recorded at the end of a network match.

If a player quits the game at any point before the score screen, and it results in the destruction of their assets (i.e. no allies exist), then a loss is recorded, regardless of the actual outcome. This is because the victory condition is processed at the time the game ends and the score screen is shown. To resolve this, the player's victory is now saved as soon as one team remains and the match is considered over, and so any subsequent defeat + asset destruction has no effect on the recorded outcome.

The change includes a minor refactor by abstracting some of the update logic into a multipleAlliancesExist function, and consolidating some stat conditions. There are further refactors that can be done in follow-up changes, such as reading m_isDefeated instead of repeatedly calling hasSinglePlayerBeenDefeated in multiple places, and splitting several blocks in the update method into functions.

Tests

Below is a series of network match test results to verify the fix.

Test 1

Knife surrenders at the start of a 1v1 match. Fork then quits the game.

Player	Team	Old Outcome	New Outcome
Knife	-	Loss	Loss
Fork	-	Loss	Win*

Test 2

Knife surrenders at the start of a 1v1v1 match. Fork then surrenders. Spoon then quits the game.

Player	Team	Old Outcome	New Outcome
Knife	-	Loss	Loss
Fork	-	Loss	Loss
Spoon	-	Loss	Win*

Test 3

Knife surrenders at the start of a 2v1 match. Fork then surrenders. Spoon then quits the game.

Player	Team	Old Outcome	New Outcome
Knife	1	Loss	Win*
Fork	-	Loss	Loss
Spoon	1	Loss	Win*

Test 4

Fork surrenders at the start of a 2v1 match. Spoon then quits the game. Knife then quits the game.

Player	Team	Old Outcome	New Outcome
Knife	1	Loss	Win*
Fork	-	Loss	Loss
Spoon	1	Win	Win

Test 5

Knife surrenders at the start of a 2v1 match. Spoon then surrenders. Fork then quits the game.

Player	Team	Old Outcome	New Outcome
Knife	1	Loss	Loss
Fork	-	Loss	Win*
Spoon	1	Loss	Loss

greptile-apps · 2026-02-11T05:58:20Z

Greptile Summary

This PR fixes a bug where players who quit before the score screen would have their victory incorrectly recorded as a defeat. The fix caches victory status early when only one alliance remains, before players can quit and destroy their assets.

Key Changes:

Added m_isVictorious array to track victorious players persistently
Refactored alliance checking into multipleAlliancesExist() helper function
Created markAllianceVictorious() to mark winning alliance when game ends
Modified hasAchievedVictory() and hasBeenDefeated() to use cached status instead of recalculating
Consolidated duplicate stat tracking logic in ScoreScreen.cpp

Critical Issue:
The multipleAlliancesExist() function cannot distinguish between "exactly one alliance alive" and "nobody alive". When all players are simultaneously defeated (rare edge case), the code will set m_singleAllianceRemaining = true but won't mark anyone victorious because findFirstUndefeatedPlayer() returns nullptr. This leaves match outcome inconsistent.

Confidence Score: 2/5

Critical edge case bug prevents correct handling when all players are simultaneously defeated
The ScoreScreen.cpp refactoring is safe, but VictoryConditions.cpp has a critical logic bug when no undefeated players exist (edge case scenario)
Pay close attention to VictoryConditions.cpp in both Generals and GeneralsMD directories - the edge case handling needs to be fixed before merge

Important Files Changed

Filename	Overview
Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp	Added `m_isVictorious` array and `markAllianceVictorious()` to save victory status early; refactored alliance checking into `multipleAlliancesExist()`; critical edge case when all players are defeated simultaneously
GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp	Identical changes to Generals version; same critical edge case exists

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[VictoryConditions::update called] --> B{Is multiplayer?}
    B -->|No| Z[Return]
    B -->|Yes| C{m_singleAllianceRemaining?}
    C -->|Yes| D[Check player eliminations]
    C -->|No| E[Call multipleAlliancesExist]
    E --> F{Multiple alliances?}
    F -->|Yes| D
    F -->|No| G[Set m_singleAllianceRemaining = true]
    G --> H[Set m_endFrame]
    H --> I[Call findFirstUndefeatedPlayer]
    I --> J{Player found?}
    J -->|Yes| K[Call markAllianceVictorious]
    J -->|No| L[BUG: No one marked victorious]
    K --> D
    L --> D
    D --> M[Loop through all players]
    M --> N{Player defeated?}
    N -->|Yes| O[Set m_isDefeated and notify]
    N -->|No| P[Continue]
    O --> P

_{Last reviewed commit: 36f8103}

greptile-apps

_{2 files reviewed, 3 comments}

_{Edit Code Review Agent Settings | Greptile}

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp

greptile-apps · 2026-02-11T05:58:32Z

Additional Comments (2)

Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp
logic issue: losing players who quit early without asset destruction won't be marked as defeated

The old logic returned true for any player when m_singleAllianceRemaining && !hasAchievedVictory(player). The new logic only checks m_isDefeated[i], which is only set when hasSinglePlayerBeenDefeated() returns true (assets destroyed). If a losing player quits early and their assets are maintained by allies, they won't be marked defeated.

Add fallback logic:

Bool VictoryConditions::hasBeenDefeated(Player *player)
{
	if (!player)
		return false;

	if (!m_singleAllianceRemaining)
		return false;

	for (Int i = 0; i < MAX_PLAYER_COUNT; ++i)
	{
		if (player == m_players[i])
		{
			if (m_isDefeated[i])
				return true;
			if (!m_isVictorious[i])
				return true;
			return false;
		}
	}

	return false;
}

Prompt To Fix With AI

This is a comment left during a code review.
Path: Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp
Line: 287:302

Comment:
logic issue: losing players who quit early without asset destruction won't be marked as defeated

The old logic returned true for any player when `m_singleAllianceRemaining && !hasAchievedVictory(player)`. The new logic only checks `m_isDefeated[i]`, which is only set when `hasSinglePlayerBeenDefeated()` returns true (assets destroyed). If a losing player quits early and their assets are maintained by allies, they won't be marked defeated.

Add fallback logic:

```cpp
Bool VictoryConditions::hasBeenDefeated(Player *player)
{
	if (!player)
		return false;

	if (!m_singleAllianceRemaining)
		return false;

	for (Int i = 0; i < MAX_PLAYER_COUNT; ++i)
	{
		if (player == m_players[i])
		{
			if (m_isDefeated[i])
				return true;
			if (!m_isVictorious[i])
				return true;
			return false;
		}
	}

	return false;
}
```

How can I resolve this? If you propose a fix, please make it concise.

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp
logic issue: losing players who quit early without asset destruction won't be marked as defeated

The old logic returned true for any player when m_singleAllianceRemaining && !hasAchievedVictory(player). The new logic only checks m_isDefeated[i], which is only set when hasSinglePlayerBeenDefeated() returns true (assets destroyed). If a losing player quits early and their assets are maintained by allies, they won't be marked defeated.

Add fallback logic:

Bool VictoryConditions::hasBeenDefeated(Player *player)
{
	if (!player)
		return false;

	if (!m_singleAllianceRemaining)
		return false;

	for (Int i = 0; i < MAX_PLAYER_COUNT; ++i)
	{
		if (player == m_players[i])
		{
			if (m_isDefeated[i])
				return true;
			if (!m_isVictorious[i])
				return true;
			return false;
		}
	}

	return false;
}

Prompt To Fix With AI

This is a comment left during a code review.
Path: GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp
Line: 289:304

Comment:
logic issue: losing players who quit early without asset destruction won't be marked as defeated

The old logic returned true for any player when `m_singleAllianceRemaining && !hasAchievedVictory(player)`. The new logic only checks `m_isDefeated[i]`, which is only set when `hasSinglePlayerBeenDefeated()` returns true (assets destroyed). If a losing player quits early and their assets are maintained by allies, they won't be marked defeated.

Add fallback logic:

```cpp
Bool VictoryConditions::hasBeenDefeated(Player *player)
{
	if (!player)
		return false;

	if (!m_singleAllianceRemaining)
		return false;

	for (Int i = 0; i < MAX_PLAYER_COUNT; ++i)
	{
		if (player == m_players[i])
		{
			if (m_isDefeated[i])
				return true;
			if (!m_isVictorious[i])
				return true;
			return false;
		}
	}

	return false;
}
```

How can I resolve this? If you propose a fix, please make it concise.

…defeat in network matches

greptile-apps

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp

Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp

xezon · 2026-02-18T19:11:18Z

Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp

 }

+//-------------------------------------------------------------------------------------------------
+Player* VictoryConditions::findFirstVictoriousPlayer()


Conceptually, bool multipleAlliancesExist + Player* findFirstNotDefeatedPlayer + markAllianceVictorious(Player*) is a bit cumbersome logic.

A better logic would be Player* getSingleAllianceSurvived + markAllianceVictorious(Player*)
.

Checking for a single surviving alliance does not conceptually work either - we still want the condition to be satisfied if there are no alliances remaining.

greptile-apps

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp

greptile-apps

_{4 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp

greptile-apps

_{4 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp

…lting in defeat in network matches (TheSuperHackers#2292)

refactor: Abstract multiple alliance logic into separate function

bec5f12

Stubbjax self-assigned this Feb 11, 2026

Stubbjax added Bug Something is not working right, typically is user facing Minor Severity: Minor < Major < Critical < Blocker Gen Relates to Generals ZH Relates to Zero Hour Refactor Edits the code with insignificant behavior changes, is never user facing labels Feb 11, 2026

greptile-apps bot reviewed Feb 11, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp Outdated Show resolved Hide resolved

bugfix: Save victory status to prevent early exits from resulting in …

87a75a4

…defeat in network matches

Stubbjax force-pushed the fix-victory-conditions branch from e688101 to 87a75a4 Compare February 11, 2026 06:52

Stubbjax added 2 commits February 12, 2026 01:08

tweak: Standardise win and loss conditions with streak conditions

6dead6c

refactor: Consolidate duplicate stat conditions

aa5f363

greptile-apps bot reviewed Feb 11, 2026

View reviewed changes

Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp Show resolved Hide resolved

xezon reviewed Feb 16, 2026

View reviewed changes

Stubbjax added 3 commits February 17, 2026 10:51

perf: Minor optimisation

5745099

refactor: Split some update functionality into functions

f378df7

docs: Add comments

e297db3

xezon reviewed Feb 18, 2026

View reviewed changes

docs: Consolidate comments

cb93902

greptile-apps bot reviewed Feb 19, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp Outdated Show resolved Hide resolved

Stubbjax force-pushed the fix-victory-conditions branch from db4f4c7 to 06feb14 Compare February 20, 2026 00:11

greptile-apps bot reviewed Feb 20, 2026

View reviewed changes

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp Outdated Show resolved Hide resolved

refactor: Rename function for better clarity

36f8103

Stubbjax force-pushed the fix-victory-conditions branch from 06feb14 to 36f8103 Compare February 20, 2026 00:14

greptile-apps bot reviewed Feb 20, 2026

View reviewed changes

Generals/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp Show resolved Hide resolved

GeneralsMD/Code/GameEngine/Source/GameLogic/ScriptEngine/VictoryConditions.cpp Show resolved Hide resolved

xezon changed the title ~~bugfix: Save victory status to prevent early exits from resulting in defeat in network matches~~ bugfix(victory): Save victory status to prevent early exits from resulting in defeat in network matches Feb 20, 2026

xezon approved these changes Feb 20, 2026

View reviewed changes

xezon merged commit 312ebd8 into TheSuperHackers:main Feb 21, 2026
25 checks passed

Stubbjax deleted the fix-victory-conditions branch February 22, 2026 00:05

CookieLandProjects pushed a commit to CookieLandProjects/CLP_AI that referenced this pull request Feb 28, 2026

bugfix(victory): Save victory status to prevent early exits from resu…

cafea71

…lting in defeat in network matches (TheSuperHackers#2292)

Conversation

Uh oh!

Tests

Test 1

Test 2

Test 3

Test 4

Test 5

Uh oh!

Uh oh!

Greptile Summary

Confidence Score: 2/5

Important Files Changed

Flowchart

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants