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

don't mark software waveforms legacy #11918

Merged

Conversation

Swiftb0y
Copy link
Member

@Swiftb0y Swiftb0y commented Sep 4, 2023

as discussed in #11915

Anything else you'd like to see implemented?

@github-actions github-actions bot added the ui label Sep 4, 2023
@daschuer
Copy link
Member

daschuer commented Sep 4, 2023

Thank you. That's already good. Need to test.
Do you like to join the two () ()?

@Swiftb0y
Copy link
Member Author

Swiftb0y commented Sep 4, 2023

Do you like to join the two () ()?

Yes, I'm on it, currently being held up by a little bigger refactoring to make the change straightforward.

@Swiftb0y Swiftb0y force-pushed the feat/dont-mark-software-waveforms-legacy branch 2 times, most recently from b83d539 to cfdbb34 Compare September 4, 2023 19:29
@Swiftb0y Swiftb0y force-pushed the feat/dont-mark-software-waveforms-legacy branch from cfdbb34 to 626a4ea Compare September 4, 2023 19:36
@Swiftb0y
Copy link
Member Author

Swiftb0y commented Sep 4, 2023

ready for review

@ronso0
Copy link
Member

ronso0 commented Sep 4, 2023

I'd like to take a look but don't have the time to build, @Swiftb0y do you mind posting a screenshot of the new list?

@Swiftb0y
Copy link
Member Author

Swiftb0y commented Sep 4, 2023

Sure, I'm not sure about the ES code as I never had those on my system but I don't want to jump through hoops to test those as the formatting code is simple enough to declare correct by inspection IMO.

Screenshot from 2023-09-04 22-22-32
Screenshot from 2023-09-04 22-22-45

@ronso0
Copy link
Member

ronso0 commented Sep 4, 2023

Thanks, looks like a good step forward!

Though, these are indeed still way too many choices for beginners, and even advanced users I'd say. 🤪
A '?' hyperlink to a manual chapter with some (basic) explanation may be helpful.

@Swiftb0y
Copy link
Member Author

Swiftb0y commented Sep 4, 2023

A '?' hyperlink to a manual chapter with some (basic) explanation may be helpful.

Sure, but thats not my area of expertise. Would you prefer to postpone it or do you want it in this PR? I'd appreciate a PR to my branch then.

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

Thank you for going the extra mile. This looks good for me.

@daschuer
Copy link
Member

daschuer commented Sep 4, 2023

@ronso0, merge?

// 30FPS is the default
frameRateSlider->setValue(30);
// 60FPS is the default
frameRateSlider->setValue(60);
Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Member Author

Choose a reason for hiding this comment

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

75FPS displays get info fashion now, we should probably expand the possible framerate to 120FPS soon, also variable refresh rates are a thing now and might be interesting for us (though probably only relevant for games that have exclusive access to a display device)

@ronso0
Copy link
Member

ronso0 commented Sep 4, 2023

@ronso0, merge?

Sure, the list looks much better now, and I trust both of you wrt the code.

A '?' hyperlink to a manual chapter with some (basic) explanation may be helpful.

Sure, but thats not my area of expertise. Would you prefer to postpone it or do you want it in this PR? I'd appreciate a PR to my branch then.

I'm just sharing my ideas for further improvement, though those should go to #11915

@ronso0 ronso0 merged commit 4c82ffa into mixxxdj:2.4 Sep 4, 2023
12 of 13 checks passed
@Swiftb0y
Copy link
Member Author

Swiftb0y commented Sep 4, 2023

Thanks everybody.

@Swiftb0y Swiftb0y deleted the feat/dont-mark-software-waveforms-legacy branch September 4, 2023 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants