Skip to content

bugfix(skirmish): Set consistent seed values for skirmish games to prevent mismatches for restarted skirmish games #2270

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
Caball009 wants to merge 6 commits into
base: main
Choose a base branch
from
+11 −6
Open

bugfix(skirmish): Set consistent seed values for skirmish games to prevent mismatches for restarted skirmish games #2270

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e3f7584
Fixed incorrect seed value for restarted skirmish games (multiplayer …
Caball009 Feb 9, 2026
45fa3ea
Fixed incorrect seed value for newly started skirmish games (singlepl…
Caball009 Feb 9, 2026
6913185
Changed conditional for seed value.
Caball009 Feb 9, 2026
f72bc0e
Added game mode assertion.
Caball009 Feb 9, 2026
4b6d3dc
Expanded assertion.
Caball009 Feb 9, 2026
0737074
Changed assertion text.
Caball009 Feb 9, 2026
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
12 changes: 7 additions & 5 deletions GeneralsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/QuitMenu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,11 @@ static void restartMissionMenu()
Int gameMode = TheGameLogic->getGameMode();
AsciiString mapName = TheGlobalData->m_mapName;

// TheSuperHackers @bugfix Caball009 07/02/2026 Reuse the previous seed value for the new skirmish match to prevent mismatches.
// Campaign, challenge, and skirmish single-player scenarios all use GAME_SINGLE_PLAYER and are expected to use 0 as seed value.
DEBUG_ASSERTCRASH((TheSkirmishGameInfo != nullptr) == (gameMode == GAME_SKIRMISH), ("Unexpected game mode on map / mission restart"));
const UnsignedInt seed = (TheSkirmishGameInfo) ? TheSkirmishGameInfo->getSeed() : 0;
Copy link

@xezon xezon Feb 10, 2026

Choose a reason for hiding this comment

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

Superfluous ()


//
// if the map name was from a save game it will have "Save/" at the front of it,
// we want to go back to the original pristine map string for the map name when restarting
Expand Down Expand Up @@ -238,11 +243,8 @@ static void restartMissionMenu()
TheScriptEngine->getGlobalDifficulty(),
rankPointsStartedWith)
);
//if (TheGlobalData->m_fixedSeed >= 0)
//InitRandom(TheGlobalData->m_fixedSeed);
InitRandom(0);
//else
// InitGameLogicRandom(GameClientRandomValue(0, INT_MAX - 1));

InitRandom(seed);
Copy link

@xezon xezon Feb 9, 2026

Choose a reason for hiding this comment

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

According to your list:

reallyDoStart -> InitGameLogicRandom( TheSkirmishGameInfo->getSeed() )

Wouldn't this mean that this needs to do:

InitGameLogicRandom( TheSkirmishGameInfo->getSeed() ) then?

Copy link
Author

@Caball009 Caball009 Feb 9, 2026

Choose a reason for hiding this comment

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

Isn't that what I'm doing here?

Copy link

@xezon xezon Feb 9, 2026

Choose a reason for hiding this comment

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

Currently:

InitRandom( TheSkirmishGameInfo->getSeed() );

Expected:

InitGameLogicRandom( TheSkirmishGameInfo->getSeed() );

Copy link
Author

@Caball009 Caball009 Feb 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

That's a good point. It's not clear why the former is used and sometimes the latter. The important point, though, is that InitRandom also sets the logical seed values, which are the only seed values that are part of the CRC computation.

I think we should make reallyDoStart use InitRandom, and perhaps check other places if they should also use InitRandom instead of InitGameLogicRandom.

Copy link

@xezon xezon Feb 10, 2026

Choose a reason for hiding this comment

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

Ok maybe make a follow up investigation. Is not directly related to this fix.

}
//TheTransitionHandler->remove("QuitFull"); //KRISMORNESS ADD
//quitMenuLayout = nullptr; //KRISMORNESS ADD
Expand Down
5 changes: 4 additions & 1 deletion ...lsMD/Code/GameEngine/Source/GameClient/GUI/GUICallbacks/Menus/SkirmishGameOptionsMenu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,6 @@ void reallyDoStart( void )

TheWritableGlobalData->m_mapName = TheSkirmishGameInfo->getMap();
TheSkirmishGameInfo->startGame(0);
InitGameLogicRandom(TheSkirmishGameInfo->getSeed());

Bool isSkirmish = TRUE;
const MapMetaData *md = TheMapCache->findMap(TheSkirmishGameInfo->getMap());
Expand All @@ -443,6 +442,8 @@ void reallyDoStart( void )

if (isSkirmish)
{
InitGameLogicRandom(TheSkirmishGameInfo->getSeed());
Copy link

@xezon xezon Feb 9, 2026

Choose a reason for hiding this comment

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

Why do this change? Shouldn't it be responsibility of whoever set seed in TheSkirmishGameInfo? Now this responsibility has shifted to this function instead.

Copy link
Author

@Caball009 Caball009 Feb 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

This line was only changed because of the change below this for single player maps.

With regard to the setting of the seed value, that's already part of the GameInfo constructor.

Copy link
Author

@Caball009 Caball009 Feb 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

The restart function sets 0 as seed value for skirmish single player maps, so this function should as well. The restart function cannot use the seed value for skirmish single player maps because TheSkirmishGameInfo is null.

Copy link

@xezon xezon Feb 9, 2026

Choose a reason for hiding this comment

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

I do not understand. Does this mean, when !isSkirmish, then TheSkirmishGameInfo not null here, but will be null on mission restart?

If yes, does that not indicate a bug elsewhere?

Copy link
Author

@Caball009 Caball009 Feb 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

TheSkirmishGameInfo is never null in reallyDoStart, otherwise it'd have crashed in the original code.

It's null in restartMissionMenu for skirmish single player mission. I agree that's unexpected, but it works without apparent issue.

Copy link

@xezon xezon Feb 10, 2026

Choose a reason for hiding this comment

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

Ok that is very strange. Perhaps that warrants a follow up investigation?


GameMessage *msg = TheMessageStream->appendMessage( GameMessage::MSG_NEW_GAME );
msg->appendIntegerArgument(GAME_SKIRMISH);
msg->appendIntegerArgument(DIFFICULTY_NORMAL); // not really used; just specified so we can add the game speed last
Expand All @@ -451,6 +452,8 @@ void reallyDoStart( void )
}
else
{
InitGameLogicRandom(0);

GameMessage *msg = TheMessageStream->appendMessage( GameMessage::MSG_NEW_GAME );
msg->appendIntegerArgument(GAME_SINGLE_PLAYER);
msg->appendIntegerArgument(DIFFICULTY_NORMAL); // not really used; just specified so we can add the game speed last
Expand Down
Loading