New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: How to handle Ineluki's MP3 patch #939

Closed
carstene1ns opened this Issue Jul 18, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@carstene1ns
Member

carstene1ns commented Jul 18, 2016

Back in the days when RPG Maker 2000 did not support mp3 music natively, there was a patch by Ineluki that used a modified harmony.dll to accomplish that.
It used *.link.wav files in the Music folder that contained a relative path in them (mostly to a separate mp3 folder in the game directory) and sometimes also commands like loop in a second line.
While these games could have been updated in the meantime to more recent RPG_RT versions that do not need the patch, there are a lot unmaintained ones.

While we could certainly do a string comparison to check for them (like #731) and display a warning, this will lead to a problem: Even if the mp3 file is copied over to the music folder retaining the wav filename, the .link still is a part of the filename to match the database entry. So we will show the warning again, even if it is bogus. And we cannot wait until the files get rejected by the audio backend, because Player will show an unrelated SDL_mixer warning then.

A thing that came to mind is just stripping off the .link part of the filename, if there is one in the database and so trying to play the right file (residing in the same folder copied over by the user without the extension). This way it will not block the old RPG_RT/harmony.dll way.

Edit: btw. Calm Falls (#940) uses this patch.

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Jul 18, 2016

Member

I think the easiest solution is to really parse the files when it ends on ".link" but this needs some adjustments to the read filename to make it FileFinder-friendly.

I would suggest to only read the first line and to ignore the rest.

..\mp3\01-Theme_Of_Laura.mp3

..\ -> Strip this part, we are in Music

FileFinder::FindDefault("mp3\01-Theme_Of_Laura.mp3")

Maybe the filefinder should do the path normalising???

and ..\..\ should throw a warning because of out of project-dir read

Member

Ghabry commented Jul 18, 2016

I think the easiest solution is to really parse the files when it ends on ".link" but this needs some adjustments to the read filename to make it FileFinder-friendly.

I would suggest to only read the first line and to ignore the rest.

..\mp3\01-Theme_Of_Laura.mp3

..\ -> Strip this part, we are in Music

FileFinder::FindDefault("mp3\01-Theme_Of_Laura.mp3")

Maybe the filefinder should do the path normalising???

and ..\..\ should throw a warning because of out of project-dir read

Ghabry added a commit to Ghabry/easyrpg-player that referenced this issue Jul 28, 2016

Make the Ineluki MP3 Path canonical and change logic of the default f…
…ile finder search to detect directories. Fixes #939
@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Jul 28, 2016

Member

I would like to propose a patch for this:
Ghabry@e305582

Ghabry@46bb84e

already see some bugs:
Missing return in the !stream-case
bgm_pending = false; not set for ineluki case
messed up the whitespacing

There are also other commits in this branch because it is based on my Audio branch because I needed a way to pass paths directly to the audiointerface instead of depending on them calling FindMusic.

Member

Ghabry commented Jul 28, 2016

I would like to propose a patch for this:
Ghabry@e305582

Ghabry@46bb84e

already see some bugs:
Missing return in the !stream-case
bgm_pending = false; not set for ineluki case
messed up the whitespacing

There are also other commits in this branch because it is based on my Audio branch because I needed a way to pass paths directly to the audiointerface instead of depending on them calling FindMusic.

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Jul 29, 2016

Member

Are there other commands available besides loop? I can't find the documentation.

Member

Ghabry commented Jul 29, 2016

Are there other commands available besides loop? I can't find the documentation.

@carstene1ns

This comment has been minimized.

Show comment
Hide comment
@carstene1ns

carstene1ns Jul 29, 2016

Member

It seems loop is the only command, at least it is the only available option in the provided link maker program.
screenshot

Member

carstene1ns commented Jul 29, 2016

It seems loop is the only command, at least it is the only available option in the provided link maker program.
screenshot

@Ghabry

This comment has been minimized.

Show comment
Hide comment
@Ghabry

Ghabry Jul 29, 2016

Member

Okay then I will just ignore the 2nd line. Doesn't make a big difference if you loop a long bgm or not.

Member

Ghabry commented Jul 29, 2016

Okay then I will just ignore the 2nd line. Doesn't make a big difference if you loop a long bgm or not.

@carstene1ns carstene1ns added this to the 0.5.0 milestone Jul 30, 2016

Ghabry added a commit to Ghabry/easyrpg-player that referenced this issue Aug 2, 2016

Make the Ineluki MP3 Path canonical and change logic of the default f…
…ile finder search to detect directories. Fixes #939

Ghabry added a commit to Ghabry/easyrpg-player that referenced this issue Aug 2, 2016

Make the Ineluki MP3 Path canonical and change logic of the default f…
…ile finder search to detect directories. Fixes #939

Ghabry added a commit to Ghabry/easyrpg-player that referenced this issue Aug 2, 2016

Make the Ineluki MP3 Path canonical and change logic of the default f…
…ile finder search to detect directories. Fixes #939

Ghabry added a commit to Ghabry/easyrpg-player that referenced this issue Aug 2, 2016

Make the Ineluki MP3 Path canonical and change logic of the default f…
…ile finder search to detect directories. Fixes #939
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment