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: Create OBS Studio folder for default recording location #10974

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Warchamp7
Copy link
Member

@Warchamp7 Warchamp7 commented Jul 12, 2024

Description

This updates the default directory for recordings to be a newly created OBS Studio folder inside the default directory
%UserProfile%/Videos/OBS Studio on Windows
/home/<User>/OBS Studio on Linux (?)
/Users/<User>/Movies/OBS Studio on macOS (?)

Code for Linux and mac will need review and suggestions because I am completely guessing there and it will almost certainly not compile.

Motivation and Context

Better organization for users

How Has This Been Tested?

Tested on Windows.

Ensured the directory is created during initial startup and properly set for new Profiles.

Types of changes

  • Tweak (non-breaking change to improve existing functionality)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

Comment on lines +1192 to +1193
if (!MakeRecordingDir())
throw "Failed to create recording directory";
Copy link
Member

Choose a reason for hiding this comment

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

This should be unnecessary as the directory is created on demand when a recording is started. OBS should not create an empty folder in the user's video folder just from being run.

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. It does not create the folder on demand in my testing
    image

  2. I disagree and believe the folder should be initialized when OBS first runs like any app other storage location

Copy link
Member

Choose a reason for hiding this comment

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

This isn't an app storage location though, this is the user's data folders. Well-designed apps should respect a user's folder layout and not create empty files and directories.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have some sort of guidance or documentation from Microsoft about that?

I have multiple applications on my system that have created folders for themselves in the Videos library location

Copy link
Member

Choose a reason for hiding this comment

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

It's just good behavior, just because other apps are shitty doesn't mean we should also be shitty. There's plenty of evidence of of users who desire such behavior from their apps.

Copy link
Member

Choose a reason for hiding this comment

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

One's shitty behaviour is another's expected behaviour though. And by that token I'd rather have an application create files in its own managed subdirectory than mess up the root of a data storage location.

I do agree that the directory should be created upon first use though. I don't remember Shadowplay creating directories for recordings or highlights until I actually triggered either.

Copy link
Member

Choose a reason for hiding this comment

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

I concur with first use, as a user who does not enjoy all the random empty folders I have no intention of using.

@WizardCM WizardCM added the Enhancement Improvement to existing functionality label Jul 13, 2024
UI/platform-osx.mm Outdated Show resolved Hide resolved
UI/platform-osx.mm Outdated Show resolved Hide resolved
@Warchamp7 Warchamp7 added the UI/UX Anything to do with changes or additions to UI/UX elements. label Jul 15, 2024
@GeorgesStavracas GeorgesStavracas self-requested a review July 15, 2024 21:34
return getenv("HOME");
if (moviesDirectory) {
const string videoSavePath =
[moviesDirectory URLByAppendingPathComponent:@"OBS-Studio" isDirectory:YES].path.fileSystemRepresentation;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
[moviesDirectory URLByAppendingPathComponent:@"OBS-Studio" isDirectory:YES].path.fileSystemRepresentation;
[moviesDirectory URLByAppendingPathComponent:@"OBS Studio" isDirectory:YES].path.fileSystemRepresentation;

Shouldn't this just be "OBS Studio"? Spaces are fine to pass as a component to this function, and I'm not aware of any issues with spaces in folder names for OBS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement Improvement to existing functionality UI/UX Anything to do with changes or additions to UI/UX elements.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants