Using WildMidi always fails due to API confusion in WildMIDIDevice::LoadInstruments()

Trying to use WildMidi as the music option in GZDoom always results in an error message that says that GZDoom was unable to create the WildMidi device (and then it falls back to FluidSynth, which does work).

Note: In order to use WildMidi, you need to have a copy of the GUS patch files, and you need to give GZDoom the location of a wildmidi.cfg file that's set up to use them in gzdoom.ini. However I have done this and made sure I did it correctly, and it still doesn't work. The patch files I used are from here:

https://www.doomworld.com/idgames/music/dgguspat

I found that this is due to a bug in WildMIDIDevice::LoadInstruments() in music_wildmidi_mididevice.cpp.

The code says this:

            bool success = wildMidiConfig.instruments->LoadConfig(wildMidiConfig.readerName.c_str());
            wildMidiConfig.reader = nullptr;

            if (!success)
            {
                    wildMidiConfig.instruments.reset();
                    wildMidiConfig.loadedConfig = "";
                    throw std::runtime_error("Unable to initialize instruments for WildMidi device");
            }

The implication is that Instruments::LoadConfig() returns a bool, where TRUE indicates success and FALSE indicates failure.

However, if we actually look at the Instruments::LoadConfig() function in thirdparty/wildmidi/wildmidi_lib.cpp, it's not defined to return a bool: it returns an int, where 0 means success and -1 means failure.

When Instruments::LoadConfig() correctly loads the GUS patch info, it returns 0, which is interpreted as FALSE, indicating a failure, so WildMIDIDevice::LoadInstruments() always throws an exception, even though there wasn't actually any reason to do that.

I think the correct code should be:

            int success = wildMidiConfig.instruments->LoadConfig(wildMidiConfig.readerName.c_str());
            wildMidiConfig.reader = nullptr;

            if (success != 0)
            {
                    wildMidiConfig.instruments.reset();
                    wildMidiConfig.loadedConfig = "";
                    throw std::runtime_error("Unable to initialize instruments for WildMidi device");
            }

If I make this change and recompile the ZMusic library, I can select WildMidi as the music device in GZDoom and it works correctly now(again, with the GUS patch files and wildmidi.cfg file present).

For reference, my environment is:

CPU arch: x86-64 OS: FreeBSD 12.3-RELEASE Compiler: Clang/LLVM 10.0.1

It seems to me though that this bug should occur on every platform. In fact I can't see how it could have ever possibly worked.

Oh, and before you ask: no, I'm not submitting a patch. It should be trivial to fix this directly.

ZDoom / ZMusic

Using WildMidi always fails due to API confusion in WildMIDIDevice::LoadInstruments() #33