[FEATURE] Added a speed modifier#23
Conversation
Added custom speed option for animations in README.
Janneske2001
changed the title
| } | ||
|
|
||
| void Keyboard::SetSpeedFactor(float factor) | ||
| { |
There was a problem hiding this comment.
Add some checks to ensure that factor isn't an unreasonably high or unnecessarily low number. (Lets say >= 1 && <= 250)
There was a problem hiding this comment.
That's a reasonable thing to add. Do you think 250 is a good upper limit? I think there is a limit anyways with how fast it can even switch colour. And I personally think that people would rarely have it play even faster than the animations are right now. For now I think I will add 250 as limit, as you suggested, but it can always be changed.
There was a problem hiding this comment.
Also, when opened from a non-elevated terminal, it will reopen in an elevated separate terminal window. But when an error would happen, it would instantly close. I added a little Press Enter to exit... thing too. It gives users the ability to read the errors in this specific edge case.
| { | ||
| m_ptrKbComms->SetKBColour(frame->zone, frame->colour); | ||
| // MODIFIED: apply speed factor to the sleep duration | ||
| uint32_t sleepMs = static_cast<uint32_t>(frame->ms_time / m_speedFactor); |
There was a problem hiding this comment.
Move the sleepMs variable outside of the loop to avoid re-calculating on each frame.
|
Other than a few minor comments, the changes look good to me. Once those are sorted, I'm happy to approve and merge this change 👍 |
| if (const auto frame = animation.GetFrame(i)) | ||
| { | ||
| m_ptrKbComms->SetKBColour(frame->zone, frame->colour); | ||
| // MODIFIED: apply speed factor to the sleep duration |
There was a problem hiding this comment.
Remove the "MODIFIED" from this comment. Just make it Apply speed factor to the sleep duration.
(And move this comment to outside of the loop like the sleepMs variable)
|
I pushed changes to my repo, but it also included this reworked error handling I talked about. Should I remove that, and then push again and do another merge request? |
Hi @Janneske2001, It's fine, keep the changes. I didn't see that you had already done the changes in GitHub. |
No problem, Do I need to do a new Pull Request? |
|
@Janneske2001 - How big is the change? |
There are 8 changed files in total. Each file doesn't have many changes. However, there was one file that I basically didn't "change", but it seemed "corrupted" on my laptop. I copy-pasted the original content back in it and it worked for me, but it gets treated as a modified file (stdafx.cpp). |
|
@Janneske2001 - OK, so just to understand, what is it that you would need a "New Pull Request" for? That looks like the current change? or am I mistaken in someway? |
|
@DeviceIoControl Honestly, I don't really know how Github works with pull requests and all. Sorry for that. Basically I finished my modifications, so it's ready from my side. |
|
@Janneske2001 - Thanks for clarifying, I'm going to do a bit of testing on my system, just to ensure everything works and I will approve this PR once it is done. Thanks! |
|
@DeviceIoControl Alright nice! I couldn't test with a 3-zone keyboard, but all the single-zone themes work (At least on my device 😅) You're welcome, and I'm glad I could help! |
2 participants
I looked into the code and together with AI I got speed modification to work with only small changes.
Speed can be changed by adding
-speed [number], where the number stands for the percentage of speed.So
-speed 10will set the speed to 10%,-speed 250will set the speed to 250%.