Menu Item cleanup (#190)

* Move selection options to `Sized`, Toggle class

* change basicTitle to title; place in constructors

* remove soundEditor.currentValue

Finish menu item tidying

* Fix Toggle scroll

* Fix nullptr and signed/unsigned bugs

* Turn Enumeration back to int from size_t

* Boundary check for selection list drawing

* Fix drawItemsForOLED

* Fix last found bugs before continuing refactor

* Move to static_vector

* Compiling with static_vector and templates

* Move Title to deluge::string

* Fix some bugs

* Add fmt as submodule

* Stringify and runtime-formatted titles

* building for both

* tweak static_vector

* Building for both

* Building on both

* Format includes

* Remove fmt submodule

* Working build after merge

* Typed Enumeration and Selection

* Fix lookup for getMax/MinValue in Param

* Run format

* Finish conversion to string

* A little bit of cleanup

* Swap allocator (and new/delete) to delugeAlloc

* Format

* Format

.clang-format

@@ -16,5 +16,7 @@ BreakBeforeBinaryOperators: NonAssignment
 BreakBeforeTernaryOperators: True
 AccessModifierOffset: -4
 AlwaysBreakTemplateDeclarations: Yes
+#IndentRequiresClause: False
+#RequiresClausePosition: OwnLine
 
 SortIncludes: CaseSensitive

CMakeLists.txt

@@ -187,17 +187,6 @@ add_compile_options(
     # ASM stuff
     $<$<COMPILE_LANGUAGE:ASM>:-x>
     $<$<COMPILE_LANGUAGE:ASM>:assembler-with-cpp>
-
-    # Warnings
-    # $<$<CONFIG:DEBUG>:-Wextra> # TODO: Enable when ready
-    # $<$<CONFIG:DEBUG>:-Wall> # TODO: Enable when ready
-    # $<$<CONFIG:DEBUG>:-Wpedantic> # TODO: Enable when ready
-    $<$<CONFIG:DEBUG>:-Wnull-dereference>
-    $<$<CONFIG:DEBUG>:-Wno-psabi>
-
-    # Stack usage
-    $<$<CONFIG:DEBUG>:-Wstack-usage=100>
-    $<$<CONFIG:RELEASE>:-Wstack-usage=1000>
 )
 
 add_link_options(
@@ -224,9 +213,27 @@ if(FORCE_COLORED_OUTPUT)
     endif()
 endif(FORCE_COLORED_OUTPUT)
 
+# Add libraries
+add_subdirectory(lib)
+
 # Add our sources
 add_subdirectory(src)
 
+# Project-only compile options (warnings, etc.)
+target_compile_options(deluge INTERFACE
+
+    # Warnings
+    # $<$<CONFIG:DEBUG>:-Wextra> # TODO: Enable when ready
+    # $<$<CONFIG:DEBUG>:-Wall> # TODO: Enable when ready
+    # $<$<CONFIG:DEBUG>:-Wpedantic> # TODO: Enable when ready
+    $<$<CONFIG:DEBUG>:-Wnull-dereference>
+    $<$<CONFIG:DEBUG>:-Wno-psabi>
+
+    # Stack usage
+    $<$<CONFIG:DEBUG>:-Wstack-usage=100>
+    $<$<CONFIG:RELEASE>:-Wstack-usage=1000>
+)
+
 # 7SEG configuration
 add_firmware(deluge7SEG deluge)
 

SConstruct

@@ -139,6 +139,12 @@ for build_env in env_standalone_builds:
         duplicate=False,
     )
 
+    VariantDir(
+        os.path.relpath(os.path.join(build_env["BUILD_DIR"], 'lib')),
+        "#lib",
+        duplicate=False,
+    )
+
     # If we're in "only prepare mode" don't queue up any of the actual compilation steps.
     # Just bail at this point. There's also no point putting compilation db creation
     # before this as it won't compile the DB without awareness of the build steps.
@@ -151,6 +157,8 @@ for build_env in env_standalone_builds:
     # Compilation DB settings wrapper
     build_env.Tool("compdb")
 
+    build_env.StaticLibrary('fmt', ['lib/fmt/format.cc'])
+
     # Using the specified include dirs rather than walking every path in src was
     # preventing a successful .elf build so generically went with the latter approach.
     # This will serve us better later, buuuuut could also potentially break something.

lib/CMakeLists.txt

@@ -0,0 +1,8 @@
+include(FetchContent)
+
+FetchContent_Declare(
+  fmt
+  GIT_REPOSITORY https://github.com/fmtlib/fmt/
+  GIT_TAG        a0b8a92e3d1532361c2f7feb63babc5c18d00ef2 # release-10.0.0
+)
+FetchContent_MakeAvailable(fmt)

site_scons/build_opts.scons

@@ -27,7 +27,7 @@ env.AppendUnique(
     ASMPATH=[
         "src/NE10/common",
     ],
-    CPPPATH=["src/deluge", "src/NE10/modules", "src/NE10/inc", "src", "src/fatfs"],
+    CPPPATH=["src/deluge", "src/NE10/modules", "src/NE10/inc", "src", "src/fatfs", 'lib/fmt/include'],
 )
 env["ASPATH"] = " {}".format(" ".join(['-I"{}"'.format(inc) for inc in env["ASMPATH"]]))
 

src/NE10/inc/NE10_macros.h

@@ -50,7 +50,7 @@ extern "C" {
 // some external macro definitions to be exposed to the users
 /////////////////////////////////////////////////////////
 
-void* delugeAlloc(int requiredSize);
+void* delugeAlloc(unsigned int requiredSize);
 void delugeDealloc(void* address);
 
 #define NE10_MALLOC delugeAlloc

src/definitions.h

@@ -23,6 +23,8 @@
 #define XTAL_SPEED_MHZ 13225625
 #endif
 
+#define RUNTIME_FEATURE_SETTING_MAX_OPTIONS 8
+
 // UART
 #define MIDI_TX_BUFFER_SIZE 1024
 

src/definitions_cxx.hpp

@@ -19,6 +19,8 @@
 #include "definitions.h"
 #include "util/misc.h"
 #include <algorithm>
+#include <cmath>
+#include <cstddef>
 #include <cstdint>
 
 #define ALPHA_OR_BETA_VERSION 1 // Whether to compile with additional error-checking
@@ -576,6 +578,7 @@ enum class SynthMode {
 	FM,
 	RINGMOD,
 };
+constexpr int kNumSynthModes = util::to_underlying(SynthMode::RINGMOD) + 1;
 
 enum class ModFXType {
 	NONE,

src/deluge/CMakeLists.txt

@@ -21,11 +21,7 @@ target_link_libraries(deluge INTERFACE
     fatfs
     RTT
     NE10
-)
-
-target_compile_options(deluge INTERFACE
-    # Set preinclude
-    $<$<COMPILE_LANGUAGE:CXX>:-includepreinclude_cxx.h>
+    fmt::fmt # fmtlib
 )
 
 target_compile_definitions(deluge INTERFACE

src/deluge/gui/menu_item/arpeggiator/gate.h

@@ -18,11 +18,11 @@
 #include "gui/menu_item/unpatched_param.h"
 #include "gui/ui/sound_editor.h"
 
-namespace menu_item::arpeggiator {
+namespace deluge::gui::menu_item::arpeggiator {
 class Gate final : public UnpatchedParam {
 public:
-	Gate(char const* newName = NULL, int32_t newP = 0) : UnpatchedParam(newName, newP) {}
-	bool isRelevant(Sound* sound, int32_t whichThing) { return !soundEditor.editingCVOrMIDIClip(); }
+	using UnpatchedParam::UnpatchedParam;
+	bool isRelevant(Sound* sound, int32_t whichThing) override { return !soundEditor.editingCVOrMIDIClip(); }
 };
 
-} // namespace menu_item::arpeggiator
+} // namespace deluge::gui::menu_item::arpeggiator

src/deluge/gui/menu_item/arpeggiator/midi_cv/gate.h

@@ -20,20 +20,20 @@
 #include "model/clip/instrument_clip.h"
 #include "model/song/song.h"
 
-namespace menu_item::arpeggiator::midi_cv {
+namespace deluge::gui::menu_item::arpeggiator::midi_cv {
 class Gate final : public Integer {
 public:
 	using Integer::Integer;
-	void readCurrentValue() {
-		InstrumentClip* current_clip = static_cast<InstrumentClip*>(currentSong->currentClip);
+	void readCurrentValue() override {
+		auto* current_clip = static_cast<InstrumentClip*>(currentSong->currentClip);
 		int64_t arp_gate = (int64_t)current_clip->arpeggiatorGate + 2147483648;
-		soundEditor.currentValue = (arp_gate * 50 + 2147483648) >> 32;
+		this->value_ = (arp_gate * 50 + 2147483648) >> 32;
 	}
-	void writeCurrentValue() {
-		((InstrumentClip*)currentSong->currentClip)->arpeggiatorGate =
-		    (uint32_t)soundEditor.currentValue * 85899345 - 2147483648;
+	void writeCurrentValue() override {
+		(static_cast<InstrumentClip*>(currentSong->currentClip))->arpeggiatorGate =
+		    (uint32_t)this->value_ * 85899345 - 2147483648;
 	}
-	int32_t getMaxValue() const { return 50; }
-	bool isRelevant(Sound* sound, int32_t whichThing) { return soundEditor.editingCVOrMIDIClip(); }
+	[[nodiscard]] int32_t getMaxValue() const override { return 50; }
+	bool isRelevant(Sound* sound, int32_t whichThing) override { return soundEditor.editingCVOrMIDIClip(); }
 };
-} // namespace menu_item::arpeggiator::midi_cv
+} // namespace deluge::gui::menu_item::arpeggiator::midi_cv

src/deluge/gui/menu_item/arpeggiator/midi_cv/rate.h

@@ -20,25 +20,26 @@
 #include "model/clip/instrument_clip.h"
 #include "model/song/song.h"
 
-namespace menu_item::arpeggiator::midi_cv {
+namespace deluge::gui::menu_item::arpeggiator::midi_cv {
 class Rate final : public Integer {
 public:
-	Rate(char const* newName = NULL) : Integer(newName) {}
-	void readCurrentValue() {
-		soundEditor.currentValue =
-		    (((int64_t)((InstrumentClip*)currentSong->currentClip)->arpeggiatorRate + 2147483648) * 50 + 2147483648)
+	using Integer::Integer;
+	void readCurrentValue() override {
+		this->value_ =
+		    (((int64_t)(static_cast<InstrumentClip*>(currentSong->currentClip))->arpeggiatorRate + 2147483648) * 50
+		     + 2147483648)
 		    >> 32;
 	}
-	void writeCurrentValue() {
-		if (soundEditor.currentValue == 25) {
-			((InstrumentClip*)currentSong->currentClip)->arpeggiatorRate = 0;
+	void writeCurrentValue() override {
+		if (this->value_ == 25) {
+			(static_cast<InstrumentClip*>(currentSong->currentClip))->arpeggiatorRate = 0;
 		}
 		else {
-			((InstrumentClip*)currentSong->currentClip)->arpeggiatorRate =
-			    (uint32_t)soundEditor.currentValue * 85899345 - 2147483648;
+			(static_cast<InstrumentClip*>(currentSong->currentClip))->arpeggiatorRate =
+			    (uint32_t)this->value_ * 85899345 - 2147483648;
 		}
 	}
-	int32_t getMaxValue() const { return 50; }
-	bool isRelevant(Sound* sound, int32_t whichThing) { return soundEditor.editingCVOrMIDIClip(); }
+	[[nodiscard]] int32_t getMaxValue() const override { return 50; }
+	bool isRelevant(Sound* sound, int32_t whichThing) override { return soundEditor.editingCVOrMIDIClip(); }
 };
-} // namespace menu_item::arpeggiator::midi_cv
+} // namespace deluge::gui::menu_item::arpeggiator::midi_cv

src/deluge/gui/menu_item/arpeggiator/mode.h

@@ -16,7 +16,7 @@
 */
 #pragma once
 #include "definitions_cxx.hpp"
-#include "gui/menu_item/selection.h"
+#include "gui/menu_item/selection/typed_selection.h"
 #include "gui/ui/sound_editor.h"
 #include "model/clip/clip.h"
 #include "model/clip/instrument_clip.h"
@@ -25,22 +25,21 @@
 #include "processing/sound/sound.h"
 #include "util/misc.h"
 
-namespace menu_item::arpeggiator {
-class Mode final : public Selection {
+namespace deluge::gui::menu_item::arpeggiator {
+class Mode final : public TypedSelection<ArpMode, kNumArpModes> {
 public:
-	Mode(char const* newName = NULL) : Selection(newName) {}
-	void readCurrentValue() { soundEditor.currentValue = util::to_underlying(soundEditor.currentArpSettings->mode); }
-	void writeCurrentValue() {
+	using TypedSelection::TypedSelection;
+	void readCurrentValue() override { this->value_ = soundEditor.currentArpSettings->mode; }
+	void writeCurrentValue() override {
 
 		// If was off, or is now becoming off...
-		if (soundEditor.currentArpSettings->mode == ArpMode::OFF
-		    || static_cast<ArpMode>(soundEditor.currentValue) == ArpMode::OFF) {
+		if (soundEditor.currentArpSettings->mode == ArpMode::OFF || this->value_ == ArpMode::OFF) {
 			if (currentSong->currentClip->isActiveOnOutput()) {
 				char modelStackMemory[MODEL_STACK_MAX_SIZE];
 				ModelStackWithThreeMainThings* modelStack = soundEditor.getCurrentModelStack(modelStackMemory);
 
 				if (soundEditor.editingCVOrMIDIClip()) {
-					((InstrumentClip*)currentSong->currentClip)
+					(static_cast<InstrumentClip*>(currentSong->currentClip))
 					    ->stopAllNotesForMIDIOrCV(modelStack->toWithTimelineCounter());
 				}
 				else {
@@ -52,18 +51,13 @@ class Mode final : public Selection {
 				}
 			}
 		}
-		soundEditor.currentArpSettings->mode = static_cast<ArpMode>(soundEditor.currentValue);
+		soundEditor.currentArpSettings->mode = this->value_;
 
 		// Only update the Clip-level arp setting if they hadn't been playing with other synth parameters first (so it's clear that switching the arp on or off was their main intention)
 		if (!soundEditor.editingKit()) {
-			bool arpNow =
-			    (static_cast<ArpMode>(soundEditor.currentValue) != ArpMode::OFF); // Uh.... this does nothing...
+			bool arpNow = (this->value_ != ArpMode::OFF); // Uh.... this does nothing...
 		}
 	}
-	char const** getOptions() {
-		static char const* options[] = {"OFF", "UP", "DOWN", "BOTH", "Random", NULL};
-		return options;
-	}
-	int32_t getNumOptions() { return kNumArpModes; }
+	static_vector<string, capacity()> getOptions() override { return {"OFF", "UP", "DOWN", "BOTH", "Random"}; }
 };
-} // namespace menu_item::arpeggiator
+} // namespace deluge::gui::menu_item::arpeggiator

src/deluge/gui/menu_item/arpeggiator/octaves.h

@@ -18,13 +18,13 @@
 #include "gui/menu_item/integer.h"
 #include "gui/ui/sound_editor.h"
 
-namespace menu_item::arpeggiator {
+namespace deluge::gui::menu_item::arpeggiator {
 class Octaves final : public Integer {
 public:
-	Octaves(char const* newName = NULL) : Integer(newName) {}
-	void readCurrentValue() { soundEditor.currentValue = soundEditor.currentArpSettings->numOctaves; }
-	void writeCurrentValue() { soundEditor.currentArpSettings->numOctaves = soundEditor.currentValue; }
-	int32_t getMinValue() const { return 1; }
-	int32_t getMaxValue() const { return 8; }
+	using Integer::Integer;
+	void readCurrentValue() override { this->value_ = soundEditor.currentArpSettings->numOctaves; }
+	void writeCurrentValue() override { soundEditor.currentArpSettings->numOctaves = this->value_; }
+	[[nodiscard]] int32_t getMinValue() const override { return 1; }
+	[[nodiscard]] int32_t getMaxValue() const override { return 8; }
 };
-} // namespace menu_item::arpeggiator
+} // namespace deluge::gui::menu_item::arpeggiator

src/deluge/gui/menu_item/arpeggiator/rate.h

@@ -18,11 +18,11 @@
 #include "gui/menu_item/patched_param/integer.h"
 #include "gui/ui/sound_editor.h"
 
-namespace menu_item::arpeggiator {
+namespace deluge::gui::menu_item::arpeggiator {
 class Rate final : public patched_param::Integer {
 public:
-	Rate(char const* newName = NULL, int32_t newP = 0) : Integer(newName, newP) {}
-	bool isRelevant(Sound* sound, int32_t whichThing) { return !soundEditor.editingCVOrMIDIClip(); }
+	using patched_param::Integer::Integer;
+	bool isRelevant(Sound* sound, int32_t whichThing) override { return !soundEditor.editingCVOrMIDIClip(); }
 };
 
-} // namespace menu_item::arpeggiator
+} // namespace deluge::gui::menu_item::arpeggiator

src/deluge/gui/menu_item/arpeggiator/sync.h

@@ -18,18 +18,18 @@
 #include "gui/menu_item/sync_level.h"
 #include "gui/ui/sound_editor.h"
 
-namespace menu_item::arpeggiator {
+namespace deluge::gui::menu_item::arpeggiator {
 class Sync final : public SyncLevel {
 public:
-	Sync(char const* newName = NULL) : SyncLevel(newName) {}
+	using SyncLevel::SyncLevel;
 	void readCurrentValue() {
-		soundEditor.currentValue = syncTypeAndLevelToMenuOption(soundEditor.currentArpSettings->syncType,
-		                                                        soundEditor.currentArpSettings->syncLevel);
+		this->value_ = syncTypeAndLevelToMenuOption(soundEditor.currentArpSettings->syncType,
+		                                            soundEditor.currentArpSettings->syncLevel);
 	}
 	void writeCurrentValue() {
-		soundEditor.currentArpSettings->syncType = menuOptionToSyncType(soundEditor.currentValue);
-		soundEditor.currentArpSettings->syncLevel = menuOptionToSyncLevel(soundEditor.currentValue);
+		soundEditor.currentArpSettings->syncType = menuOptionToSyncType(this->value_);
+		soundEditor.currentArpSettings->syncLevel = menuOptionToSyncLevel(this->value_);
 	}
 };
 
-} // namespace menu_item::arpeggiator
+} // namespace deluge::gui::menu_item::arpeggiator

src/deluge/gui/menu_item/audio_clip/attack.h

@@ -20,18 +20,19 @@
 #include "model/clip/audio_clip.h"
 #include "model/song/song.h"
 
-namespace menu_item::audio_clip {
+namespace deluge::gui::menu_item::audio_clip {
 class Attack final : public Integer {
 public:
 	using Integer::Integer;
 
-	void readCurrentValue() {
-		soundEditor.currentValue =
-		    (((int64_t)((AudioClip*)currentSong->currentClip)->attack + 2147483648) * 50 + 2147483648) >> 32;
+	void readCurrentValue() override {
+		this->value_ =
+		    (((int64_t)(static_cast<AudioClip*>(currentSong->currentClip))->attack + 2147483648) * 50 + 2147483648)
+		    >> 32;
 	}
-	void writeCurrentValue() {
-		((AudioClip*)currentSong->currentClip)->attack = (uint32_t)soundEditor.currentValue * 85899345 - 2147483648;
+	void writeCurrentValue() override {
+		(static_cast<AudioClip*>(currentSong->currentClip))->attack = (uint32_t)this->value_ * 85899345 - 2147483648;
 	}
-	int32_t getMaxValue() const { return 50; }
+	[[nodiscard]] int32_t getMaxValue() const override { return 50; }
 };
-} // namespace menu_item::audio_clip
+} // namespace deluge::gui::menu_item::audio_clip