LuisMayo / objection_engine

Library that turns comment chains into ace attorney scenes, used in several bots
MIT License
105 stars 20 forks source link

Alternative music tracks #96

Closed Meorge closed 1 year ago

Meorge commented 1 year ago

As discussed in issue #95, this adds Cross Examination ~ Moderato as a possible song to play in place of Trial, and Phoenix Wright ~ Objection! as a possible song to play in place of Pursuit ~ Cornered. (Previously, Cross Examination ~ Allegro was also an option to replace Trial, but listening to it more I felt like it was a bit too tense.)

https://user-images.githubusercontent.com/9957987/201246854-f08a5561-ae69-4d81-b72d-1551af1caaca.mp4

Note that, for each game folder (pwr, jfa, tat) in assets/music, the MP3 files named cross-moderato.mp3 and objection.mp3 will need to be added. I got the MP3s from here for Phoenix Wright and Justice for All, and here for Trials and Tribulations.

Here's a zip file with just the added tracks: new_music.zip

LuisMayo commented 1 year ago

Hi.

I've been thinking about this PR and I'm afraid it may need changes. The problem I'm trying to help with here is to stop breaking backwards compatibility.

Background: There are some people running the bots using custom sets of music. In fact the reason why the music is read from a directory instead of being hard-coded is precisely because some people asked I actually don't think they actually like update the bot (let alone objection_engine) but as a library we should allow them to do it.

Problem: Right now if we merge this and they update the library their bot will stop working.

Proposed solutions:

  1. Merge it into a v4 objection_engine branch(my bots would probably switch to this branch), since this changes are breaking this couldn't make it to the main branch. However I wouldn't feel comfortable deprecating v3 given there are no other breaking changes.
  2. Change the code so it reads a random number of relaxed and objection tracks for each music set. This would mean changing lines L537 and L538 to build the array by reading the music content directories and sorting music into calm or cornered if it starts by "trial" or "objection" (again for backwards compatibility reasons.
  3. Change the code so if it fails while loading the music it just defaults to trial.mp3 or objection.mp3.

It's your code, your PR and after all your call. Please tell me your thoughts and I'm sorry I didn't think of this problem earlier.

Thanks!

Meorge commented 1 year ago

No worries, and your point about needing backwards compatibility makes a ton of sense!

I was thinking solution 2 or 3 could work well. Perhaps a music folder could (optionally) contain a config.json file that contained lists of tracks to use for the different phases, like so:

{
    "relaxed": ["trial.mp3", "cross-moderato.mp3"],
    "tense": ["objection.mp3", "press.mp3"]
}

If this file is not found in the music folder the user requested, it'd fall back to the old behavior (using hardcoded trial.mp3 and press.mp3filenames). If the JSON file is found, it randomly chooses a track path from each of the lists, and attempts to load that. If that file doesn't exist, it falls back to the old behavior again. How does this approach sound to you?

LuisMayo commented 1 year ago

I think that approach is actually better than any of my ideas. Go ahead!

Thanks.

Meorge commented 1 year ago

Great, I'll see if I can get that done either tonight or tomorrow! 😄

Meorge commented 1 year ago

Changes are pushed! The engine should behave as it has in the past if there's no config.json file inside of the music folder, but if there is one like

{
    "relaxed": ["cross-moderato", "trial"],
    "tense": ["objection", "press"]
}

then it will choose randomly from those lists instead. 🙂

LuisMayo commented 1 year ago

This one is good to do. Thanks a lot!