Skip to content

check header in OpenReplay #2807

Merged
salix5 merged 4 commits into
Fluorohydride:masterfrom
salix5:patch-check
May 30, 2025
Merged

check header in OpenReplay #2807
salix5 merged 4 commits into
Fluorohydride:masterfrom
salix5:patch-check

Conversation

@salix5

@salix5 salix5 commented May 22, 2025

Copy link
Copy Markdown
Collaborator
  • check header in OpenReplay

  • remove Replay::CheckReplay
    CheckReplay cannot check the detail of yrp file.
    Now we can check the yrp file by OpenReplay.

  • does not read yrp file in RefreshReplay
    Opening and reading every files takes too much time and has very limited effects.
    OpenReplay will check the file anyway.

  • add version_info for old replay
    yrp file of version before 0x1353 has many problems in PRNG, and these file need additional attention.

@mercury233
@Wind2009-Louse

@salix5 salix5 requested a review from Copilot May 23, 2025 19:55

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors replay file handling by removing the redundant CheckReplay function and enhancing header validation in OpenReplay while also improving version information reporting for old replays.

  • Removed Replay::CheckReplay functionality
  • Added additional header and version flag checks in OpenReplay
  • Updated replay file filtering and version info logging in menu handling and refresh logic

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
gframe/replay.h Removed declaration of Replay::CheckReplay to simplify header checking.
gframe/replay.cpp Added header validations in OpenReplay and removed CheckReplay implementation.
gframe/menu_handler.cpp Updated replay selection logic and added version info logging.
gframe/game.cpp Removed explicit CheckReplay call in RefreshReplay to rely on OpenReplay.
Comments suppressed due to low confidence (2)

gframe/menu_handler.cpp:545

  • Verify that myswprintf is safe from potential buffer overflow issues when formatting version information; consider using a formatting function that guarantees buffer safety if available.
myswprintf(version_info, L"version 0x%X", temp_replay.pheader.version);

gframe/game.cpp:1259

  • Without using Replay::CheckReplay, ensure that OpenReplay robustly validates replay files, so that invalid or corrupted files are properly handled later in the process.
if (!isdir && IsExtension(name, L".yrp"))

Comment thread gframe/replay.cpp Outdated
@purerosefallen

Copy link
Copy Markdown
Collaborator

lgtm

@purerosefallen purerosefallen self-requested a review May 26, 2025 07:34
@salix5 salix5 requested a review from Copilot May 30, 2025 07:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors replay validation by removing the old CheckReplay helper, consolidating header checks into OpenReplay, skips individual file reads in RefreshReplay, and surfaces version details for legacy replays.

  • Remove CheckReplay declaration and implementation
  • Add inline header validation in OpenReplay and show version info for old replays
  • Update replay listing to rely solely on extension check and defer validation to OpenReplay

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
gframe/replay.h Drop CheckReplay declaration
gframe/replay.cpp Inline header checks in OpenReplay, remove CheckReplay implementation
gframe/menu_handler.cpp Use selected filename variable, append version info for old replay
gframe/game.cpp Simplify RefreshReplay to only filter by extension
Comments suppressed due to low confidence (1)

gframe/menu_handler.cpp:528

  • [nitpick] Consider renaming filename to something more descriptive like selectedReplayName to clarify its purpose and improve readability.
auto filename = mainGame->lstReplayList->getListItem(sel);

Comment thread gframe/replay.cpp
Comment thread gframe/game.cpp
@salix5 salix5 merged commit 5a37286 into Fluorohydride:master May 30, 2025
8 checks passed
@salix5 salix5 deleted the patch-check branch May 30, 2025 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants