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

Display labels and values to ADSREG #51

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

Conversation

piersroberts
Copy link

Sometimes I want to make a note of exact ADSR values which isn't currently possible so I've added a line that shows them :)

Copy link

@no-trbl-2-u no-trbl-2-u left a comment

Choose a reason for hiding this comment

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

Nice work :) Excited to see more features in an already infinite module 👍

gfxHeader(applet_name());
gfxPos(1, 15);
gfxPrint(labels[edit_stage]);
gfxPrint(45 + 18 - digitCount(adsr[edit_stage]) * 6, 15, adsr[edit_stage]);

Choose a reason for hiding this comment

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

It might be beneficial to the next person to pull the equation of the first arg out into its own variable with a comment. This way there's less logic inline and less to consider while you're reading it.

Comment on lines +282 to +289
int digitCount(int n) {
int count = 0;
while (n != 0) {
n /= 10; // n = n/10
++count;
}
return count;
}

Choose a reason for hiding this comment

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

Also, by no means a blocker, but I'm of the school of thought where we should avoid imperative-style code as well as potentially infinite loops. My brain is a little fried right now because I've been jumping between PRs all morning, but my instincts are telling me there might be a cleaner and safer way to write this method.

I could 100% be wrong and you could have tried a million things, in which case, ignore my dumb butt :p

Copy link
Owner

Choose a reason for hiding this comment

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

That's pretty much the canonical way to count digits in C. No int exists that will cause an infinite loop, and log10() is much slower.

suit4 added a commit to suit4/O_C-HemisphereSuite that referenced this pull request Jun 17, 2022
suit4 added a commit to suit4/O_C-HemisphereSuite that referenced this pull request Jun 17, 2022
suit4 added a commit to suit4/O_C-HemisphereSuite that referenced this pull request Jun 17, 2022
Adding BigScope App – Changes from pull request Chysn#76
Changes from pull request Chysn#51
Changes from pull request Chysn#105
Changes from pull request Logarhythm1#2
suit4 added a commit to suit4/O_C-HemisphereSuite that referenced this pull request Jun 17, 2022
Adding BigScope App – Changes from pull request Chysn#76
Changes from pull request Chysn#51
Changes from pull request Chysn#105
Changes from pull request Logarhythm1#2
qiemem pushed a commit to qiemem/O_C-HemisphereSuite that referenced this pull request Mar 15, 2023
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

3 participants