Artefact2 / libxm

A small XM (FastTracker II Extended Module) player library.
Do What The F*ck You Want To Public License
142 stars 14 forks source link

Fix a couple of bugs in effect Rxy (multiretrig) #20

Closed rasky closed 3 years ago

rasky commented 3 years ago

Rxy is possibly the most quirky command in the XM format, and it's hard to get right in all its dark corners. This gets libxm closer to FT2-Clone and XMPlay (even closer than MilkyTracker) but still not quite perfect.

The main bug fixed here is that the volume change of a Rxy command only takes effect if the volume column is empty, and only if there is no volume envelope in the instrument (and in this case, the envelope is not even restarted, contrary to what E9x does).

To fix this, I changed the code to first retrigger the note keeping the volume and the envelope like it is, and then updating it if needed.

It's important to notice that, even the volume part of Rxy doesn't apply, the value is still remembered in case a subsequent R0y is issued; so the part handling Rxy in xm_handle_note_and_instrument isn't affected.

Another bug (more obvious) is that the multi_retrig_add lookup tables is defined in 64th of a volume step, but the value was not scaled by 64, so the volume change was always saturating the volume.

Co-developed with @bryc who also prepared the testcases.

bryc commented 3 years ago

Here are XMs in the wild that do use Rxx for effects (usually fancy percussion): look & zalza - little computer boy - Position 2 - Channel 14 dune & orange - mark a.j. pisses off - Position 12 - Channel 10

Here's the audio comparison of test caes:

te934-zj4v6

Artefact2 commented 3 years ago

Thank you both for fixing this bug! Rxy is fairly rare in modules so I never noticed the volume issues in my casual listening. Merged in ceb1ec9ba30758cce09bb80838040f35c6027f91

rasky commented 3 years ago

Just to log it, there's another bug in Rxy which I'm not planning to fix for now. If a row contains no note but an instrument with the Rxy effect, what happens is that on tick 0, the ghost note of the previous instrument plays (as per standard behavior with rows that contain instruments without notes), but on the first retrigger the instrument is changed to the new one. So actually the instrument changes mid-row.

This looks harder to implement because the logic to change instruments is not part of xm_trigger_note, but I haven't investigated into it. If you want, I can open an issue to log this.

Artefact2 commented 3 years ago

Opening an issue would be great. Thanks!