openAVproductions / openAV-Luppp

Luppp is a live performance tool, created by OpenAV productions.
http://openavproductions.com/luppp
GNU General Public License v3.0
258 stars 44 forks source link

Sanity check for bpm input #209

Closed georgkrause closed 6 years ago

georgkrause commented 6 years ago

Alright, i thought about a little and this is my solution.

There are two "features" included here:

The third commit is needed because if you enter values <60 or >220, it just dont work without any user feedback. I dont know if this string-stuff is "nice" coded, but its working. if there are any complains about (eg speed or something) please let me know. (Sadly i dont have internet access and cant get into the topic at the moment). Maybe it would be an good idea to tell the user if the input is "wrong", but i didnt wanted to let the popup pop up again and as far as i can see there is no implemented way to tell the user that something gone wrong, is it?

Happy if this could be merged or for any feedback.

harryhaaren commented 6 years ago

Two small comments, but with those fixed I'm happy to merge.

[Update: one warning when compiling, comment on line of code where it occurs]

georgkrause commented 6 years ago

Harry, thanks for your feedback! I am happy there are no major complains on my implementation (did it right, yeah!). The warning you mentioned didnt come up on my system, but as I am not totally happy with the solution at all. Maybe there is another way without all the casts and transformations (fromt int to string to c_string)? I tried on this commit a lot and was happy i found a way which works at all. But it feels dirty.

harryhaaren commented 6 years ago

Implementation seems fine - there is always a better / cleaner / more-abstract method, but its about getting things done too (caveat: gotta make sure stability is ok, which it is in this case).

The warning/error I got is for this reason: fl_input() (see link below) takes two or more arguments, and only one is being provided, fl_input("format", default_string, varargs, ... ); while it was being called fl_input(actual_string). This warning often only shows up on ubuntu machines with fortified headers enabled. Don't worry about it - I'll fixup and push.

http://www.fltk.org/doc-1.3/group__group__comdlg.html#ga4daf4c0bc85342a5794b9c2ad9e20b99

georgkrause commented 6 years ago

I put a commit here with the cleanup (just to learn what to do ;).

harryhaaren commented 6 years ago

Hey @georgkrause , thanks for applying fix on this branch - will merge this now (instead of duplicate that I created), so Closes #210.