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

Custom theme pressed color variable fade #809

Merged
merged 6 commits into from
Jan 20, 2024

Conversation

jmrochuk
Copy link
Contributor

@jmrochuk jmrochuk commented Jan 19, 2024

The Custom theme now supports blending the pressed color back into the regular color.
https://discord.com/channels/1049366310389289001/1049370387563163731/1197730363015962844

Use parameter up/down (S1+S1+(R1/R2)) to increase or decrease the fade. Defaults to 0ms blend which is the current behaviour.

Copy link
Contributor

@arntsonl arntsonl left a comment

Choose a reason for hiding this comment

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

A few nitpicks and code changes but looking good

lib/AnimationStation/src/Effects/CustomTheme.cpp Outdated Show resolved Hide resolved
}

#define PRESS_COOLDOWN_INCREMENT 500
Copy link
Contributor

Choose a reason for hiding this comment

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

stick these at the top of the C++ file as well, makes it much easier for someone else to come in and find them


absolute_time_t lastUpdateTime = nil_time;

uint32_t updateTimeInms = 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

This is super nit picky but updateTimeInms -> updateTimeInMs

@@ -398,6 +398,7 @@ namespace ConfigLegacy
uint32_t customThemeR3Pressed;
uint32_t customThemeA1Pressed;
uint32_t customThemeA2Pressed;
uint32_t customThemeCooldownTimeInMs;
Copy link
Contributor

Choose a reason for hiding this comment

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

We also don't touch anything in config_legacy.cpp, this is kind of an internal knowledge thing so super easy to miss. You can remove any changes to config_legacy.cpp



Copy link
Contributor

Choose a reason for hiding this comment

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

Remove extra return here

Copy link
Contributor

@arntsonl arntsonl left a comment

Choose a reason for hiding this comment

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

I'll see if I can do a PR on your PR you can merge in that will give us web config options. Everything else is looking good, thank you for the quick turnaround!

@@ -397,7 +397,7 @@ namespace ConfigLegacy
uint32_t customThemeL3Pressed;
uint32_t customThemeR3Pressed;
uint32_t customThemeA1Pressed;
uint32_t customThemeA2Pressed;
uint32_t customThemeA2Pressed;
Copy link
Contributor

Choose a reason for hiding this comment

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

If you don't use it, TortoiseGit will give you a visual DIFF between your check-ins which can help this kind of stuff.

You can also just revert to a previous history version so you don't end up having extra spaces or tabs.

Copy link
Contributor

@arntsonl arntsonl left a comment

Choose a reason for hiding this comment

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

Looks good, tested and working correctly

@arntsonl arntsonl merged commit fb5015e into OpenStickCommunity:main Jan 20, 2024
30 checks passed
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