Skip to content
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.

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

UI: Functionality to Record Video on Game Boot #12321

Closed
wants to merge 3 commits into from

Conversation

ReaperXYZ
Copy link
Contributor

Description of Changes

Added method to record video on game boot

Rationale behind Changes

Easier to record TASes
No need to frame advance to the point where video mode is set.

Suggested Testing Steps

Test recording on startup and then resetting. Make sure recording on startup does not interfere with boot process.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for submitting a contribution to PCSX2

As this is your first pull request, please be aware of the contributing guidelines.

Additionally, as per recent changes in GitHub Actions, your pull request will need to be approved by a maintainer before GitHub Actions can run against it. You can find more information about this change here.

Please be patient until this happens. In the meantime if you'd like to confirm the builds are passing, you have the option of opening a PR on your own fork, just make sure your fork's master branch is up to date!

@Mrlinkwii
Copy link
Contributor

May I ask what are the 3 binary files added in pcsx2/windows folders

@ReaperXYZ
Copy link
Contributor Author

I do not know what files those are. That was a mistake I did not catch. I am sorry about that lack of diligence.

@Mrlinkwii
Copy link
Contributor

Can you please remove them

@Mrlinkwii
Copy link
Contributor

Can you squash your commits for me :) testing can start and /or changes can be requested if needs be

@ReaperXYZ ReaperXYZ changed the title Functionality to record video on game boot UI: Functionality to Record Video on Game Boot Feb 18, 2025
Copy link
Member

@F0bes F0bes left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

It works well, I just found some formatting issues, and provided a suggestion for better UX.

static bool s_record_on_start = false;
static QString s_path_to_recording_for_record_on_start;


Copy link
Member

Choose a reason for hiding this comment

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

Extra line

return;
if (s_record_on_start && !s_path_to_recording_for_record_on_start.isEmpty())
{
QMessageBox msgbox2(this);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a huge fan of using a messagebox for the purpose of notifications that don't require the user to do anything.

I would opt to use our OSD system instead. I experimented and found that the following works well.

Host::AddOSDMessage(tr("Recording will start in a moment").toStdString(), 3.0f);
QTimer::singleShot(2000, []() { g_emu_thread->beginCapture(s_path_to_recording_for_record_on_start); });

@@ -1962,6 +2021,12 @@ void MainWindow::onVMStarted()
updateWindowTitle();
updateStatusBarWidgetVisibility();
updateInputRecordingActions(true);
if (s_record_on_start)
{
m_ui.actionVideoCapture->setEnabled(true);
Copy link
Member

Choose a reason for hiding this comment

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

You've removed the only occurrence of this action being disabled. The setEnabled call can be omitted here, as it will always be enabled.

@F0bes
Copy link
Member

F0bes commented Feb 26, 2025

I'm unable to access your branch and implement my changes. I've committed your changes in #12321 and it will be merged there. I recommend using a different branch than master when PRing, it's just better practice.

@F0bes F0bes closed this Feb 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants