Document dumb_*_mod* restrict_ parameter more thoroughly and avoid magic numbers

[Rondom]

The restrict_ parameter could be documented better and one should probably avoid magic numbers and instead either define an enum or preprocessor-constants for the two values.

/* dumb_*_mod*: restrict_ |= 1-Don't read 15 sample files / 2-Use old pattern counting method */
DUH *dumb_load_mod(const char *filename, int restrict_);
DUH *dumb_read_mod(DUMBFILE *f, int restrict_);
DUH *dumb_load_mod_quick(const char *filename, int restrict_);
DUH *dumb_read_mod_quick(DUMBFILE *f, int restrict_);

DUH *dumb_read_any_quick(DUMBFILE *f, int restrict_, int subsong);
DUH *dumb_read_any(DUMBFILE *f, int restrict_, int subsong);
DUH *dumb_load_any_quick(const char *filename, int restrict_, int subsong);
DUH *dumb_load_any(const char *filename, int restrict_, int subsong);

[kode54]

I've added enum parameters for this.

[SimonN]

What should we pass as restrict_ when we have no idea what specific .mod file we are going to get? I've experimented with a handful of .mod files, and passing 0 seemed best to me.

The old pattern counting looks like backwards compatibility for uncommon .mod files that DUMB can't detect on its own.

[kode54]

0 is fine as long as you're okay with possibly invalid data being treated as a NST module, which has no identifying signature. I tend to write frontend software so it only tries MOD without that flag if the file has one of the known MOD filename extensions.

[SimonN]

Thanks!

Allegro 5 has always dispatched by filename extension even before calling DUMB. That's because Allegro 5 must decide which library to call in the first place. I've keep this behavior of calling different dumb_*() in Allegro 5 depending on extension, and call dumb_*_mod*() only for .mod, and always with restrict_ = 0. Is that reasonable or should I prefer restrict_ = DUMB_MOD_RESTRICT_NO_15_SAMPLE here?

Do NST files always have .nst as an extension and never .mod? I'm then considering to let Allegro 5 dispatch .nst files to DUMB too, and call dumb_*_mod*() with restrict_ = 0.

[kode54]

NST have unknown extensions, sometimes .MOD. Feel free to use restrict_ = 0 in any case.

I only do not, since I have encountered renamed modules before, and in one case, a module that was packed in an LHA archive.

[SimonN]

Okay, I'll use 0 then for both .mod and .nst.

Interesting that the naming didn't standardize. Then yeah, you have to offer this choice to guarantee optimal playback, because not even a default can catch all cases. (On first sight, it looked like restrict_ asked its caller to make a decision that DUMB should make itself.)

Thanks for the insight!

[kode54]

Note that the any reader passes anything that fails the other reader checks into the MOD reader. Just in case something proves to be a false positive for the other signature checks. Too many possible signatures to check for MOD files.

[Rondom]

I suggest to leave this open until we have some of the content of those comments here as documentation in the code as well.

[SimonN]

Yes, this restrict_ needs public documentation: Explain exactly why we should choose 0 as a default. Also we need a minimal example of DUMB-using code. The example programs are nice, but they already do too much (parsing arguments etc.) to be minimal.

Maybe even define default arguments in DUMB 2.1? Ideally, the lib decides everything for you unless you explicitly want more control.