I am adding music formats to my audio player. This time I am adding support for the ancient Amiga module format that was used in a lot of music trackers under lots of various names. While I have not added every effect used by the "format" yet, it is playing my old music surprisingly well.
My question is about my structs.
I have organised the module (the song) in structs in a similar way to this:
typedef struct AmigaModule {
uint8_t *ModuleName;
uint8_t SampleCount;
ModuleSample *SampleList;
uint8_t SongPositions;
uint8_t RestartPosition;
uint8_t PatternTable[128];
uint8_t MaxPattern;
uint8_t ChannelCount;
ModuleChannel *ChannelList;
} AmigaModule;
Where ModuleSample is a struct for each sample, and ModuleChannel is a struct for each channel in the song, with more structs to describe the pattern data for each channel, but this is not really interesting for my question.
ModuleChannel that has grown massively big with lots of arbitrary variables for each effect that tracks state of the song while it is playing, and I have made an effort in refactoring my code to a better structure. Now I have removed all the state variables in its own struct to track states.
It looks something like this:
typedef enum ModuleChannelEffect {
EFFECT_NONE,
EFFECT_ARPEGGIO,
EFFECT_PORTAMENTO,
EFFECT_VIBRATO,
EFFECT_TREMOLO,
EFFECT_VOLUME_SLIDE,
EFFECT_RETRIGGER_SAMPLE
} ModuleChannelEffect;
typedef enum ModuleChannelEffectSpecifier {
EFFECT_SPECIFIER_NONE,
EFFECT_SPECIFIER_PORTAMENTO_TO_NOTE,
EFFECT_SPECIFIER_PORTAMENTO_UP,
EFFECT_SPECIFIER_PORTAMENTO_DOWN,
EFFECT_SPECIFIER_VOLUME_SLIDE_UP,
EFFECT_SPECIFIER_VOLUME_SLIDE_DOWN
//And more coming later...
} ModuleChannelEffectSpecifier;
typedef struct ModuleEffect {
ModuleChannelEffect primaryEffect;
ModuleChannelEffectSpecifier primaryEffectSpecifier;
uint16_t parameter1;
uint16_t parameter2;
uint16_t parameter3;
} ModuleEffect;
And here is my issue:
See the three lines 'parameter1', 'parameter2', 'parameter3'?
In code, it looks like this:
currentChannel->effect.parameter1 = periodsTable[translatedPeriod];
currentChannel->effect.parameter2 = periodsTable[arpNoteIndex1];
currentChannel->effect.parameter3 = periodsTable[arpNoteIndex2];
And when I use those values:
currentChannel->period = currentChannel->effect.parameter1;
I like two things:
1. Code that is self-explanatory.
2. Generalisation, as long as it is not over-generalisation.
My problem is that 'parameter1' doesn't tell me anything, really. In the portamento effect, is 'parameter1' the speed or the note I am sliding towards? I could name the values for what they are, but then I wouldn't solve the problem with too many variables inside the struct again, because I would need to make lots of variables I don't always need to use.
Then it came to me that I could do something like this:
#define ArpeggioValue1 parameter1
#define ArpeggioValue2 parameter2
#define ArpeggioValue3 parameter3
Now I can reference the variable names in a more sensible and descriptive way:
currentChannel->effect.ArpeggioValue1 = periodsTable[translatedPeriod];
currentChannel->effect.ArpeggioValue2 = periodsTable[arpNoteIndex1];
currentChannel->effect.ArpeggioValue3 = periodsTable[arpNoteIndex2];
currentChannel->period = currentChannel->effect.ArpeggioValue1;
My question is whether this is an acceptable coding practise, is there a better way of doing this? Do you have any other suggestion? The point here is really that I want to learn to get better. What I am doing now is working, but how would you go about it? Make massive objects to handle this? Some other thing? What is the "best" way?