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

UPDATES!! #8

Closed
wants to merge 8 commits into from
Closed

UPDATES!! #8

wants to merge 8 commits into from

Conversation

AKASGaming
Copy link

@AKASGaming AKASGaming commented Jun 15, 2022

• Updated from .NET 4.5 to .NET 4.8

• Updated other Packages

• Supports adding multiple files at once to Trailers and Prerolls

• Fixed Time and Date

• Moved Logs for bigger screens

• Added Placeholder options for Randomized Prerolls and Specified amount of Prerolls

(Someone else needs to implement these features 😜)

• Movies/Feature Supports more formats

• Proper Folder Picker

Copy link
Owner

@wmandra wmandra left a comment

Choose a reason for hiding this comment

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

Commit will not be accepted even if fully implemented. Randomization of preroll is not a desired feature.

Copy link
Owner

@wmandra wmandra left a comment

Choose a reason for hiding this comment

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

Changes to GeneralSettings.xaml should be removed.

New icons should be in Resources\Icons path not Windows.
Several of the icons added already exist in the project.

@AKASGaming
Copy link
Author

AKASGaming commented Jun 15, 2022

  1. What if other users would like this feature? I'm already working on the randomizer part right now
  2. I have no clue why VS moved/duplicated those

Copy link
Owner

@wmandra wmandra left a comment

Choose a reason for hiding this comment

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

Need to separate version update changes from feature changes with separate commits.

Changes to DigitalHomeCinemaManager/Controls/Settings/GeneralSettings.xaml are unnecessary.

Changes to DigitalHomeCinemaManager/Controls/Settings/GeneralSettings.xaml.cs are generally ok but should continue to use Environment.SpecicalFolder.MyVideos for exceptions. Again, randomize function should be removed.

Movie filter should remain .mkv only.

Changes of date from lowercase to uppercase will not be merged for style reasons. CA1308 was explicitly disabled for this reason.

No reason for clockTimer to have 1s resolution. This change unnecessarily increases calls to OnTimerElapsed handler.

PlaylistSelectionWindow should not support multi-select by default. Instead a Multiselect property should be added to the PlaylistSelectionWindow class. Multiselect ok for Preroll, Trailers, and Commercials but not Feature.

Margin change to Dolby logo unnecessary.

Combination of time hour/minute ok, but time display should remain hh:mm only.

I don't understand the rationale for the log output move.

@wmandra
Copy link
Owner

wmandra commented Jun 15, 2022

  1. What if other users would like this feature? I'm already working on the randomizer part right now
  2. I have no clue why VS moved/duplicated those

It doesn't really fit the use case at least not for Preroll or Commercials.

I do think randomization or number limit might fit for Trailers, but even then I'm not convinced of the necessity.

@AKASGaming
Copy link
Author

AKASGaming commented Jun 15, 2022

I do think randomization or number limit might fit for Trailers, but even then I'm not convinced of the necessity.

Yeah, that's a typo on my end, it's meant for the trailers.

It doesn't really fit the use case

This would be used when your playlist contains multiple trailers, so it'll automatically shuffle them when added.

Margin change to Dolby logo unnecessary

That was an accident and I forgot to move it back

log output move

It doesn't fit on small screens, but on large screens is easy to see

Multiselect ok for Preroll, Trailers, and Commercials but not Feature

Did you actually test this feature yourself? It only does this on Trailers and Prerolls, not Commercials or the Feature

Changes to DigitalHomeCinemaManager/Controls/Settings/GeneralSettings.xaml.cs are generally ok but should continue to use Environment.SpecicalFolder.MyVideos for exceptions

I tried this, but it ended up making the folder dialog buggy

Movie filter should remain .mkv only

Why? Most of the movies I have are .mp4 and .mkv so only a select few would actually be playable

Changes to DigitalHomeCinemaManager/Controls/Settings/GeneralSettings.xaml are unnecessary

Would clearing the playlist be a bad thing? What if a user wants a completely different playlist? Then they'd have to manually remove everything they have on it already. And this also includes tooltips to help better understand the buttons

No reason for clockTimer to have 1s resolution. This change unnecessarily increases calls to OnTimerElapsed handler.

This is for the clock display

Fixed/Added Audio Monitoring
Adding future support for API
Added Randomizer for Trailers
Fixed Multiselect for Trailers & Prerolls (Only Trailers support it now)
Added Tooltips to more buttons
@AKASGaming
Copy link
Author

You're not gonna accept the changes I've made anyways ¯_(ツ)_/¯

@AKASGaming AKASGaming closed this Jun 27, 2022
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.

None yet

2 participants