Asylum loader question

[sezero]

As reported by Thomas Neumann (@neumatho):

I noticed that in ConvertNote() in Asylum loader, you have a local
variable called lastnote. It seems like it should be a memory of the
last note found and is used as such. The problem is, that when used,
it will always be zero. Should this variable be global instead?
I mean, did I found a bug or is it by purpose the way its doing now?

https://github.com/sezero/mikmod/blob/master/libmikmod/loaders/load_asy.c#L147

Indeed, lastnote assignment doesn't seem to matter in present code, it is never used after being assigned.

Further note by Thomas Neumann (@neumatho):

I found one module, where it go into that code path where there is no note,
but an effect. The module is "Crusader - No Remorse - Misc2" and its on
position 3-6 in channel 6. I tried to load the module into OpenMPT to check
what it does. It do not apply any notes, but the effect and instrument.
See screenshot.

I tried to listen to this channel and the same with the MikMod player, and
it sound identical. So I think, that the loader does the right thing by not
applying any notes. Therefore it could be a good idea to make some cleanup
and remove the lastnote variable :-)

[sezero]

Comments? (@AliceLR?)

[sagamusix]

It's bogus.

[sezero]

It's bogus.

Then I suggest the following patch:

diff --git a/libmikmod/loaders/load_asy.c b/libmikmod/loaders/load_asy.c
index 8dbd9d2..33f31bd 100644
--- a/libmikmod/loaders/load_asy.c
+++ b/libmikmod/loaders/load_asy.c
@@ -146,11 +146,10 @@ static void ASY_Cleanup(void)

 static int ConvertNote(MODNOTE *n)
 {
    UBYTE instrument, effect, effdat, note;
    UWORD period;
-   UBYTE lastnote = 0;

    instrument = n->b&0x1f;
    effect = n->c;
    effdat = n->d;

@@ -191,27 +190,23 @@ static int ConvertNote(MODNOTE *n)
                    /* ...unless an effect was specified,
                     * which forces a new note to be
                     * played */
                    if (effect || effdat) {
                        UniInstrument(instrument - 1);
-                       note = lastnote;
                    } else
                        UniPTEffect(0xc,
                            mh->samples[instrument -
                            1].volume & 0x7f);
                }
            } else {
                /* Fasttracker handling */
                UniInstrument(instrument - 1);
-               if (!note)
-                   note = lastnote;
            }
        }
    }
    if (note) {
        UniNote(note + 2 * OCTAVE - 1);
-       lastnote = note;
    }

    /* Convert pattern jump from Dec to Hex */
    if (effect == 0xd)
        effdat = (((effdat & 0xf0) >> 4) * 10) + (effdat & 0xf);

Comments?

[sagamusix]

Looks sensible to me.

[sezero]

Patch applied as 3c248bac7285b7d4bd93bc39369f8c7bb7257e7a