Implementation of credits skip delay (Issue #221) #235
Conversation
Otherwise, the saved configuration will not be loaded.
ConfusedPolarBear.Plugin.IntroSkipper/Configuration/PluginConfiguration.cs
Outdated
Show resolved
Hide resolved
ConfusedPolarBear.Plugin.IntroSkipper/Configuration/configPage.html
Outdated
Show resolved
Hide resolved
| @@ -156,7 +156,7 @@ public ActionResult<TimeStamps> GetTimestamps([FromRoute] Guid id) | |||
| var segment = new Intro(timestamp); | |||
|
|
|||
| var config = Plugin.Instance.Configuration; | |||
| segment.IntroEnd -= config.SecondsOfIntroToPlay; | |||
| segment.IntroEnd -= mode == AnalysisMode.Introduction ? config.SecondsOfIntroToPlay : config.SecondsOfCreditsToPlay; | |||
There was a problem hiding this comment.
@jumoog may want this to check that it's the credits for adding recap stuff.
|
This isn't directly related to the commit, but I find it odd that |
|
Do y'all need me to make any changes to my commit? Like changing the default value to 0? |
yes it seems strange |
it happend in this commit: |
We need to have an internal discussion about how to deal with the problems that we have found and how to move forward here. |
|
I’m just pointing out that as a user, if I went from skipping the entire credit sequence to suddenly playing part of it, I would assume the credit detection was flawed. As a user impacted by the issue this references, it will likely seem that detection was “fixed” and nothing more. It won’t be clear that this is a new option. My assumption was group A will be larger than group B (who will also likely be following this discussion and know they need to set an option). Do with that what you want. The final outcome won’t impact me much because I was present to know about the new setting. |
That's a really good point, the default value should be 0 seconds to prevent confusion |
|
And to be fair, it's a great first submission. |
|
closing this in favour of #238 .. I added you as Co-author! Thank you for your PR! |
|
Yes, thanks! |
I am very novice at writing code, but was able to implement my suggestion by looking at what was done for the user defined intro playback duration function. I compiled my change and tested it on my server and was able to adjust the delay in skipping credits through the settings UI. I set the delay value to 2, 5, 15, and 20 for testing purposes and all functioned correctly.
I also did not realize that the comment for the commit on my branch would be the commit comment on my merge request so I added a comment to each commit explaining what I did.