refactor(loadscreen): Refactor the single player load screen to use TheDisplay for intro video playback by Mauller · Pull Request #2273 · TheSuperHackers/GeneralsGameCode

Mauller · 2026-02-08T12:53:45Z

Merge with rebase

This breaks out changes from #2053 that involve the refactoring of the video playback.

I split this into two commits, one with the changes to TheDisplay for adding extra functionality, the second is the load screen refactor changes.

The other changes will come along after for video scaling options and for disabling the custom overlay during video playback.

This also fixes how the video playback is displayed in the progress bar.
Now the video progress filles the entire bar before resetting the bar for the normal data loading portion of the intro screen.

Replicate in generals

…lay for video playback (#)

Mauller · 2026-02-08T12:55:43Z

GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/GUI/W3DGameWindow.cpp

 	}

+	// if the display has a movie playing then we need to draw the displays video buffer
+	if (TheDisplay->isMoviePlaying())


This is added in addition to the original drawing functionality as some screens still use the original method, such as the challenge load screen.

The challenge load screen does something more complex with it's video handling and needs looking at seperate from this change.

greptile-apps · 2026-02-08T12:57:09Z

Greptile Overview

Greptile Summary

Refactored single player load screen to use TheDisplay for video playback instead of managing video buffers locally. This centralizes video playback logic and simplifies the load screen implementation.

Key Changes:

Moved video buffer and stream management from SinglePlayerLoadScreen to Display base class
Added getMovieProgress() method to track video playback progress
Added parameter-less overloads for drawVideoBuffer() and drawScaledVideoBuffer() that use internal buffers
Simplified load screen video loop to call TheDisplay->playMovie() and poll isMoviePlaying()
Progress bar now updates based on actual video progress rather than frame count calculations
W3DGameWindow automatically renders video buffer when movie is playing

Issues Found:

One critical null pointer dereference issue in getMovieProgress() that needs fixing before merge

Confidence Score: 3/5

This PR has a critical null pointer dereference bug that must be fixed before merging
The refactoring is well-structured and simplifies the codebase by centralizing video playback logic. However, getMovieProgress() in Display.cpp:367 dereferences m_videoStream without null checks, which will cause a crash if called when no movie is playing. While the current usage in LoadScreen.cpp protects against this by checking isMoviePlaying() first, the method itself is unsafe and could be misused in future code.
GeneralsMD/Code/GameEngine/Source/GameClient/Display.cpp requires immediate attention to fix the null pointer dereference

Important Files Changed

Filename	Overview
GeneralsMD/Code/GameEngine/Source/GameClient/Display.cpp	Implemented `getMovieProgress()` to calculate video progress percentage; potential null pointer dereference if called outside of movie playback
GeneralsMD/Code/GameEngine/Source/GameClient/GUI/LoadScreen.cpp	Refactored video playback to use TheDisplay's internal buffers; simplified loop logic; progress bar now updates based on movie progress; background window handling improved
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/GUI/W3DGameWindow.cpp	Added automatic rendering of TheDisplay's internal video buffer when a movie is playing

Sequence Diagram

sequenceDiagram
    participant LoadScreen as SinglePlayerLoadScreen
    participant Display as TheDisplay
    participant VideoPlayer as TheVideoPlayer
    participant WindowMgr as TheWindowManager
    participant GameWindow as W3DGameWindow

    LoadScreen->>LoadScreen: Hide background window
    LoadScreen->>LoadScreen: Enable progress bar
    LoadScreen->>Display: playMovie(movieLabel)
    Display->>VideoPlayer: open(movieLabel)
    VideoPlayer-->>Display: videoStream
    Display->>Display: createVideoBuffer()
    Display->>Display: Set m_videoStream & m_videoBuffer
    
    loop while isMoviePlaying()
        LoadScreen->>Display: getMovieProgress()
        Display-->>LoadScreen: progress percentage
        LoadScreen->>LoadScreen: Update progress bar
        LoadScreen->>WindowMgr: update()
        LoadScreen->>Display: update()
        LoadScreen->>Display: draw()
        Display->>GameWindow: drawVideoBuffer()
        GameWindow->>Display: drawVideoBuffer(coords)
        Display->>Display: Render video frame
    end
    
    LoadScreen->>Display: stopMovie()
    Display->>Display: Clean up m_videoStream & m_videoBuffer
    LoadScreen->>LoadScreen: Re-enable background window

_{Last reviewed commit: a71ec34}

greptile-apps

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-08T12:57:12Z

GeneralsMD/Code/GameEngine/Source/GameClient/GUI/LoadScreen.cpp

@@ -548,51 +515,23 @@ void SinglePlayerLoadScreen::init( GameInfo *game )

 			TheGameEngine->serviceWindowsOS();

-			if(!m_videoStream->isFrameReady())
-			{
-				Sleep(1);
-				continue;
-			}
-
-			m_videoStream->frameDecompress();
-			m_videoStream->frameRender(m_videoBuffer);
-
-// PULLED FROM THE MISSION DISK
-//			moveWindows( m_videoStream->frameIndex());
-
-			m_videoStream->frameNext();
-
-			if(m_videoBuffer)
-				m_loadScreen->winGetInstanceData()->setVideoBuffer(m_videoBuffer);
-			if(m_videoStream->frameIndex() % progressUpdateCount == 0)
-			{
-				shiftedPercent++;
-				if(shiftedPercent >0)
-					shiftedPercent = 0;
-				Int percent = (shiftedPercent + FRAME_FUDGE_ADD)/1.3;
-				UnicodeString per;
-				per.format(L"%d%%",percent);
-				TheMouse->setCursorTooltip(UnicodeString::TheEmptyString);
-				GadgetProgressBarSetProgress(m_progressBar, percent);
-				GadgetStaticTextSetText(m_percent, per);
-
-			}
-			TheWindowManager->update();
-
 			// redraw all views, update the GUI
+			TheWindowManager->update();
+			TheDisplay->update();
 			TheDisplay->draw();
 		}


Progress bar update logic was removed during video playback. The old implementation updated the progress bar based on video frame count, but now the loop only checks isMoviePlaying() without updating any visual progress indicator. Check whether this removal was intentional.

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngine/Source/GameClient/GUI/LoadScreen.cpp Line: 502:522 Comment: Progress bar update logic was removed during video playback. The old implementation updated the progress bar based on video frame count, but now the loop only checks `isMoviePlaying()` without updating any visual progress indicator. Check whether this removal was intentional. How can I resolve this? If you propose a fix, please make it concise.

The removal was intentional as the progress bar handling was already considered a hack and added nothing to the video playback

It actually does show a loading bar at the bottom. Seen in USA 01 Mission Load Screen:

It actually does show a loading bar at the bottom. Seen in USA 01 Mission Load Screen:

i know, i removed the loading bar since it doesn't really add anything and the code behind it functioning for showing the video playback is hacky. The portion of the bar that progresses while the video plays back is a small 1/20 th of the bars overall length. So seems better to just use it to report loading progress.

The window it is part of also has to be hidden when rescaled videos are played otherwise it shows behind the video.

I think the purpose of the loading bar is to show how long the video will play? Would be good to keep it.

I think the purpose of the loading bar is to show how long the video will play? Would be good to keep it.

I can check it again but i am sure the video doesn't fill the bar during playback, it only fills a tiny portion at the beginning and the rest is filled when the rest of the game data is loading.

If that is the case, maybe we fix the loading bar. It should be possible to calculate the loading progress because the video length is known.

Managed to implement the video bar, while making it progress with the video playback, in an easier way than i initially expected.

xezon

Does the loading bar in videos still work?

Mauller · 2026-02-13T10:31:27Z

Does the loading bar in videos still work?

No i removed the video playback being part of the loading bar since the code behind it is hacky and it didn't add anything to the playback.
The window the bar is part of also needs hiding when rescaled videos are played otherwise the stretched background image shows behind the video.

…yer intro videos

Mauller · 2026-02-14T16:26:03Z

Tweaked to now show and properly update the progress bar during the video playback.

The bar now correctly displays where the video is during its playback, the bar then gets reset for the normal loading portion of the intro screen.

greptile-apps

_{8 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-14T16:28:47Z

GeneralsMD/Code/GameEngine/Source/GameClient/Display.cpp

+Real Display::getMovieProgress()
+{
+	return (Real)m_videoStream->frameIndex() / (Real)m_videoStream->frameCount() * 100.0f;
+}


Potential null pointer dereference if m_videoStream is null. Should check m_videoStream before dereferencing.

Suggested change

Real Display::getMovieProgress()

{

return (Real)m_videoStream->frameIndex() / (Real)m_videoStream->frameCount() * 100.0f;

}

Real Display::getMovieProgress()

{

if (m_videoStream == nullptr)

return 0.0f;

return (Real)m_videoStream->frameIndex() / (Real)m_videoStream->frameCount() * 100.0f;

}

Prompt To Fix With AI

This is a comment left during a code review. Path: GeneralsMD/Code/GameEngine/Source/GameClient/Display.cpp Line: 367:370 Comment: Potential null pointer dereference if `m_videoStream` is null. Should check `m_videoStream` before dereferencing. ```suggestion Real Display::getMovieProgress() { if (m_videoStream == nullptr) return 0.0f; return (Real)m_videoStream->frameIndex() / (Real)m_videoStream->frameCount() * 100.0f; } ``` How can I resolve this? If you propose a fix, please make it concise.

refactor(display): Add the ability to use internal buffers in TheDisp…

df5756a

…lay for video playback (#)

Mauller self-assigned this Feb 8, 2026

Mauller added 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 8, 2026

Mauller commented Feb 8, 2026

View reviewed changes

greptile-apps bot reviewed Feb 8, 2026

View reviewed changes

xezon reviewed Feb 10, 2026

View reviewed changes

refactor(loadscreen): Use TheDisplay for video playback of single pla…

a71ec34

…yer intro videos

Mauller force-pushed the Mauller/refactor-loadscreen-video-playback branch from 54be1bc to a71ec34 Compare February 14, 2026 16:25

greptile-apps bot reviewed Feb 14, 2026

View reviewed changes

Conversation

Mauller commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mauller Feb 8, 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!

greptile-apps bot commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Overview

Greptile Summary

Confidence Score: 3/5

Important Files Changed

Sequence Diagram

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Mauller Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

xezon Feb 10, 2026

Choose a reason for hiding this comment

Uh oh!

Mauller Feb 13, 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!

xezon Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Mauller Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

xezon Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Mauller Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Mauller commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mauller commented Feb 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mauller commented Feb 8, 2026 •

edited

Loading

Mauller Feb 8, 2026 •

edited

Loading

greptile-apps bot commented Feb 8, 2026 •

edited

Loading

Mauller Feb 13, 2026 •

edited

Loading

Mauller commented Feb 13, 2026 •

edited

Loading

Mauller commented Feb 14, 2026 •

edited

Loading