pret / pokecrystal

Disassembly of Pokémon Crystal
https://pret.github.io/pokecrystal/
2.1k stars 803 forks source link

Provide macros/script/audio.asm for integration with old projects #759

Closed mid-kid closed 2 years ago

mid-kid commented 4 years ago

PR https://github.com/pret/pokecrystal/pull/650 overhauled the way music macros work for new projects, while providing backwards-compatibility with older macros.

However, music writers have expressed woes about music written for new repos not being backwards-compatible with older repos, which are in use by a big number of active projects.

To alleviate this issue, it'd be nice to make a drop-in replacement for macros/script/audio.asm that can be used with the old repositories, to add the new macros while keeping compatibility with the old ones. This would be provided on the wiki, provide installation instructions, and be possible to be linked along with songs that use the new macro format.

This would involve taking the new file, adding the legacy compatibility macros, making either a new, local const macro or using enum again, and making sure a repo version right before this PR still matches with this, and a version from december 2017 if possible, taking into account the RGBDS version used at the time.

Bonus points for this file still being compatible with current repos.

...this, or adding instructions explaining how to build an audio file as its own object while INCLUDEing a different set or macros.

Rangi42 commented 4 years ago

Basically, you're suggesting a way to easily port the new music macros to old repos, without having to actually merge pokecrystal PRs and update lots of files unrelated to music macros.

They could be written like the opposite of legacy macros. For example, this legacy macro turns the old pitchoffset into the new transpose:

pitchoffset: MACRO
    transpose \1, \2 - 1
ENDM

So the wiki could provide a way to turn new song files' transpose into the old pitchoffset:

transpose: MACRO
    pitchoffset \1, \2 + 1
ENDM

A few like channel might get complicated, but I don't think they'd need to reimplement enum or overwrite any macros, so it could just be provided as its own macros/scripts/newaudio.asm file.

vulcandth commented 2 years ago

Couldn't we also just provide a script/tool or regex that can convert modern music to old music formatting? Possible in a wiki page? Not sure how involved this process actually is.

The main issue I see with this.. is how many different "old" music styles do we need to support? Did we do a single audio PR that changed how everything works? Or was it across several commits.

Rangi42 commented 2 years ago

It's easier to rely on rgbasm's own parsing of .asm files, with some new transitional macros included, than it is to try and auto-edit the asm with regexes or a more complex custom script. (Plus that would mean either requiring the user to set up Python, or writing such a tool in C.)

All we need is the "inverse" of the macros in legacy.asm, defining the new command names from #650 in terms of the old ones.

rawr51919 commented 2 years ago

Not sure how I'd go about this since all the audio defines are already present in legacy.asm. Would it be best to create a modern.asm that can be INCLUDEd in older fan hacks or similar?

I mean, I already tried once yet I know I'll need to approach this a bit differently before I can call this one a wrap

Rangi42 commented 2 years ago

Would it be best to create a modern.asm that can be INCLUDEd in older fan hacks or similar?

That was the goal, yes.

rawr51919 commented 2 years ago

Would it be best to create a modern.asm that can be INCLUDEd in older fan hacks or similar?

That was the goal, yes.

Where would that go, you think? perhaps in macros/scripts/modern.asm just unINCLUDEd somehow? or would it be best to put it in docs/modern.asm and reference it in a legacy_music_commands.md? legacy_music_commands.md could be an almost complete copypasta for legacy compat with modern pokecrystal and tweaked significantly for documentation relating to modern audio commands post #650 for legacy pokecrystal At least that's how I understand it

mid-kid commented 2 years ago

One requirement I'd add is that it has to be drop-in compatible with the RGBDS version that was being used at the time as well.

Whether it ends up on the repository or the wiki doesn't really matter to me, but if put in the repo, I'd put it somewhere out of the way of the files that are actually used in the disassembly, like in docs/, to avoid cluttering the actually relevant files for anyone hacking current pokecrystal.

Rangi42 commented 2 years ago

I'd like it to be in macros/ since that's literally what they are, and its role is similar to legacy.asm (supporting code from a different time period that isn't in the project by default).

(Should we stop INCLUDing legacy.asm by default? Nothing actually in pokecrystal needs it, and if anyone's porting old code they can add it.)

vulcandth commented 2 years ago

@Rangi42 @mid-kid
Alright first off.. I believe the drop in macros/script/audio.asm should be a Wiki thing. There is no point in including them in a current pokecrystal because the downstream user with an old version of pokecrystal wouldn't have the drop in due to the fact their on an old version lol. Anyways....

I have put together a working set of changes to macros/script/audio.asm for a version of pokecrystal from Aug 27th 2019 commit abee217 that provides support to CURRENT pokecrystal music.

  1. I rolled back a branch to commit abee217 and installed a local RGBDS 0.3.8
  2. I replaced all audio/music/*.asm files with CURRENT pokecrystal music files: vulcandth@03476ed
  3. I modified macros/scripts/audio.asm to provide missing MACROS for the music .asm files. vulcandth@5cceb12
  4. I verified the rom's build successfully with make compare OK

Please check out my branch: https://github.com/vulcandth/pokecrystal/tree/Aug272019Audio

If you like it, I'll write up a Wiki page for this.. and then I'll start working on a version that will work for a Dec 2017 Pokecrystal.

rawr51919 commented 2 years ago

@Rangi42 @mid-kid Alright first off.. I believe the drop in macros/script/audio.asm should be a Wiki thing. There is no point in including them in a current pokecrystal because the downstream user with an old version of pokecrystal wouldn't have the drop in due to the fact their on an old version lol. Anyways....

I have put together a working set of changes to macros/script/audio.asm for a version of pokecrystal from Aug 27th 2019 commit abee217 that provides support to CURRENT pokecrystal music.

1. I rolled back a branch to commit [abee217](https://github.com/pret/pokecrystal/commit/abee217ce0e29331f1ac691fd1d4edeacf2295f9) and installed a local RGBDS 0.3.8

2. I replaced all `audio/music/*.asm` files with CURRENT pokecrystal music files: [vulcandth@03476ed](https://github.com/vulcandth/pokecrystal/commit/03476ed2f6eb4e80aa9fdf71d4a61972d84124ae)

3. I modified `macros/scripts/audio.asm` to provide missing commands for the music .asm files.

4. I verified the rom's build successfully with `make compare` `OK`

Please check out my branch: https://github.com/vulcandth/pokecrystal/tree/Aug272019Audio

If you like it, I'll write up a Wiki page for this.. and then I'll start working on a version that will work for a Dec 2017 Pokecrystal.

Is it OK if I finish my modern_audio.asm with your macro changes? The older pre-DEF syntax should work fine as far back in pokecrystal as the macro support existed for the macros as they stand. It'd be best to have the modern_audio.asm work on a base as early as the first commit that got legacy.asm in the first place or even when we first got the legacy music commands so maximum compatibility is achieved with one, simple drop-in for all

EDIT: Yours seems better anyway and could be the way to do it with this particular approach, the fact that it should support as many pokecrystal audio restructures as it can remains Using that as a drag n drop file modern_audio.asm like in my PR #950 that can be downloaded from the wiki itself would be a good idea for the wiki page

Where does this leave #873? Once the relevant wiki entry for modern_audio.asm is added, it'd cause the same action as what merging #950 would have done

rawr51919 commented 2 years ago

@Rangi42 @mid-kid Alright first off.. I believe the drop in macros/script/audio.asm should be a Wiki thing. There is no point in including them in a current pokecrystal because the downstream user with an old version of pokecrystal wouldn't have the drop in due to the fact their on an old version lol. Anyways....

I have put together a working set of changes to macros/script/audio.asm for a version of pokecrystal from Aug 27th 2019 commit abee217 that provides support to CURRENT pokecrystal music.

1. I rolled back a branch to commit [abee217](https://github.com/pret/pokecrystal/commit/abee217ce0e29331f1ac691fd1d4edeacf2295f9) and installed a local RGBDS 0.3.8

2. I replaced all `audio/music/*.asm` files with CURRENT pokecrystal music files: [vulcandth/pokecrystal@03476ed](https://github.com/vulcandth/pokecrystal/commit/03476ed)

3. I modified `macros/scripts/audio.asm` to provide missing MACROS for the music .asm files. [vulcandth/pokecrystal@5cceb12](https://github.com/vulcandth/pokecrystal/commit/5cceb12)

4. I verified the rom's build successfully with `make compare` `OK`

Please check out my branch: https://github.com/vulcandth/pokecrystal/tree/Aug272019Audio

If you like it, I'll write up a Wiki page for this.. and then I'll start working on a version that will work for a Dec 2017 Pokecrystal.

Also LGTM, if you want you can finish up what I attempted to do

rawr51919 commented 2 years ago

I'll start on the wiki page today, hopefully that'll incentivize getting both this and #873 solved at the same time EDIT: Stub wiki page exists now at https://github.com/pret/pokecrystal/wiki/Add-audio-scripts-to-use-modern-audio-files-with-legacy-pokecrystal, edit as you wish

vulcandth commented 2 years ago

I'll start on the wiki page today, hopefully that'll incentivize getting both this and #873 solved at the same time EDIT: Stub wiki page exists now at https://github.com/pret/pokecrystal/wiki/Add-audio-scripts-to-use-modern-audio-files-with-legacy-pokecrystal, edit as you wish

This is more of what I had in mind.... https://github.com/pret/pokecrystal/wiki/Add-macros-to-support-modern-audio-files-in-legacy-pokecrystal

I'll need to test how far back the compatibility works.. and then add additional sections for even older versions.

rawr51919 commented 2 years ago

Good start there Vulcan! LGTM

mid-kid commented 2 years ago

@vulcandth Good work on this. I agree we should put it on the Wiki instead. However, I'd prefer this to be a file the user can simply copy to their existing repository, rather than a tutorial or a modification to the old macros files...

vulcandth commented 2 years ago

@vulcandth Good work on this. I agree we should put it on the Wiki instead. However, I'd prefer this to be a file the user can simply copy to their existing repository, rather than a tutorial or a modification to the old macros files...

@mid-kid 'll see what I can do. It may depend on what is needed for some of the older pokecrystal versions. My goal was to add compatibility for newer music to old pokecrystal while maintaining compatibility with old music. As I work on it more I'll determine what the best course of action is. Were you wanting a drop in replacement or a new file they can add. (Problem with a new file are some macro names didn't change, and instead were modified to support new/old functionality in current pokecrystal.). Which would mean the other better option would be a replacement.. but I didn't like that idea, as if this is for romhacks, they may have modified the file for other things. Either way let me finish figuring out the older versions and we can go from there.

vulcandth commented 2 years ago

@vulcandth Good work on this. I agree we should put it on the Wiki instead. However, I'd prefer this to be a file the user can simply copy to their existing repository, rather than a tutorial or a modification to the old macros files...

I updated the wiki page https://github.com/pret/pokecrystal/wiki/Add-macros-to-support-modern-audio-files-in-legacy-pokecrystal and included the entire file like you asked. It should work from pokecrystal 12/28/2017 b9a68fec2 - 08/27/2019 abee217. Let me know if you want me to provide support for older. If everything looks good, I think we can close this issue.

(I'll just need to find where I want to link it from the main page. I'm thinking a new wiki category called Legacy Support.. or something. Im still thinking on that, but I'm tired and it's time for bed.)

rawr51919 commented 2 years ago

@vulcandth Good work on this. I agree we should put it on the Wiki instead. However, I'd prefer this to be a file the user can simply copy to their existing repository, rather than a tutorial or a modification to the old macros files...

I updated the wiki page https://github.com/pret/pokecrystal/wiki/Add-macros-to-support-modern-audio-files-in-legacy-pokecrystal and included the entire file like you asked. It should work from pokecrystal 12/28/2017 b9a68fec2 - 08/27/2019 abee217. Let me know if you want me to provide support for older. If everything looks good, I think we can close this issue.

(I'll just need to find where I want to link it from the main page. I'm thinking a new wiki category called Legacy Support.. or something. I'm still thinking on that, but I'm tired and it's time for bed.)

873 has a fix in the new article now via a small blurb about the music_commands.md in the two commits this ranges from compatibility-wise OR whatever music_commands.md was used in their project(s).

Also added the wiki page to Tutorials for now, it can be moved where you want, for now where it is seems like the best spot with what I can figure out about it

mid-kid commented 2 years ago

I updated the wiki page https://github.com/pret/pokecrystal/wiki/Add-macros-to-support-modern-audio-files-in-legacy-pokecrystal and included the entire file like you asked. It should work from pokecrystal 12/28/2017 b9a68fec2 - 08/27/2019 abee217. Let me know if you want me to provide support for older. If everything looks good, I think we can close this issue.

(I'll just need to find where I want to link it from the main page. I'm thinking a new wiki category called Legacy Support.. or something. Im still thinking on that, but I'm tired and it's time for bed.)

Thanks a ton. Looks great. Have you tested this by dropping in the new music onto the older branch?

Also, can you provide a direct download for the file? You can clone the wiki as a repository, put the file in there and link to it I'm pretty sure, but something like a gist is good too.

And no, going pre-2017 is unnecessary. That's already way further back than I was really thinking :P

rawr51919 commented 2 years ago

I updated the wiki page https://github.com/pret/pokecrystal/wiki/Add-macros-to-support-modern-audio-files-in-legacy-pokecrystal and included the entire file like you asked. It should work from pokecrystal 12/28/2017 b9a68fec2 - 08/27/2019 abee217. Let me know if you want me to provide support for older. If everything looks good, I think we can close this issue. (I'll just need to find where I want to link it from the main page. I'm thinking a new wiki category called Legacy Support.. or something. Im still thinking on that, but I'm tired and it's time for bed.)

Thanks a ton. Looks great. Have you tested this by dropping in the new music onto the older branch?

Also, can you provide a direct download for the file? You can clone the wiki as a repository, put the file in there and link to it I'm pretty sure, but something like a gist is good too.

And no, going pre-2017 is unnecessary. That's already way further back than I was really thinking :P

iirc Vulcan already tested these changes with at least 2019 pokecrystal from the article, confirmed working fine

mid-kid commented 2 years ago

I was asking vulcan, but we ended up speaking over Discord. This issue is resolved.