Update audio macros and arguments

[dannye]

and update related labels/constants/comments
this fixes a lot of mistakes and maximizes compatibility with red

Addresses #556 and goes along with pret/pokered#223

docs/music_commands.md still needs updating but I figured I'd start the PR so people can review while I fix the docs

[dannye]

Old Macro Name	New Macro Name
musicheader	channel
sound	square_note
noise	noise_note
notetype	note_type
pitchoffset	transpose
dutycycle	duty_cycle
intensity	volume_envelope
soundinput	pitch_sweep
sound_duty	duty_cycle_pattern
togglesfx	toggle_sfx
slidepitchto	pitch_slide
togglenoise	toggle_noise
panning	force_stereo_panning
tone	pitch_offset
restartchannel	restart_channel
newsong	new_song
sfxpriorityon	sfx_priority_on
sfxpriorityoff	sfx_priority_off
stereopanning	stereo_panning
sfxtogglenoise	sfx_toggle_noise
setcondition	set_condition
jumpif	sound_jump_if
jumpchannel	sound_jump
loopchannel	sound_loop
callchannel	sound_call
endchannel	sound_ret

[dannye]

Macro	Old Macro Arguments	New Macro Arguments	Reason
channel	number of channels, channel id, address	channel id, address	Number of channels is now handled by channel_count macro.
square_note	pitch, length, intensity, frequency	length, volume, fade, frequency	The "pitch" byte is actually the high byte of the length. Intensity was split into volume and fade.
noise_note	pitch, length, intensity, frequency	length, volume, fade, frequency	Pitch makes no sense for noise notes. The "pitch" byte is actually the high byte of the length. Intensity was split into volume and fade.
note_type	length, intensity	length, volume, fade	Intensity was split into volume and fade.
transpose	octave, pitch	number of octaves, number of pitches	"pitch" does not refer to an actual pitch, but how many pitches to raise each note.
volume_envelope	intensity	volume, fade	Intensity was split into volume and fade.
pitch_sweep	"input"	duration, sweep	Was mistakenly documented as being written to NR52 but is actually written to NR10.
duty_cycle_pattern	duty cycle list (backwards)	duty cycle list	Each duty cycle is now listed in the order they are used.
pitch_slide	duration, octave (backwards), pitch	duration, octave, pitch (modded)	The octave value was not being subtracted from 8 like usual. Also mod the pitch with 12 (number of pitches in an octave) to fix "B_". This improves compatibility with red.
vibrato	delay, extent	delay, extent, rate	"extent" actually represents extent (depth) and rate.
force_stereo_panning	tracks	left enable (bool), right enable (bool)	"tracks" was a full 8 bit value, but it really represents two booleans.
volume	volume	left volume, right volume	The whole byte is really two nibbles representing the volume level for each speaker
stereo_panning	tracks	left enable (bool), right enable (bool)	"tracks" was a full 8 bit value, but it really represents two booleans.

[dannye]

Newly Added Macro	Reason
channel_count	Used at the start of each audio header to simplify the channel macro.
dnote drum_note	Like note but exclusively for the noise channel since the noise channel uses drum instrument IDs instead of pitch values. This is also necessary for compatibility with red.
rest	Replaces the clumsy note __, xx. For compatibility with red.
dspeed drum_speed	The note_type macro exclusively for the noise channel. Necessary for compatibility with red.
execute_music	Alias for toggle_sfx. Necessary for compatibility with red.
toggle_perfect_pitch	Alias for pitch_offset 1. Necessary for compatibility with red.

[mid-kid]

I'll preface this by saying that I haven't ever written my own song for
pokecrystal, so I'm sure someone else will be more apt at judging this PR than
me, but I'll weigh in with some comments, anyway.

First of all, and I know this is kind of a pain in the ass to do, but you
should update the docs/music_commands.md file with the correct info for your
new macros.

Second of all, you should make sure that old songs still build with your
new macros. Add whatever is necessary to macros/legacy.asm and label
whatever is necessary with LEGACY. We don't want to lose the tonnes of
tracks made by the community over the years.

Now, for more specific comments:

sound → square_note
noise → noise_note
Why? sound worked perfectly fine here, and adding _note when it's just
playing a sound, makes little sense.

The length argument, being so common over the different kinds of notes,
should either be consistently the first argument or the last. Same goes for
the frequency in note, square_note and noise_note.
I personally prefer length being the last argument, with frequency being the
first, but anything goes.

Are you sure (force_)stereo_panning are two booleans, instead of four
booleans for each ear? IIRC there was something with this but I don't recall.

As for the Red compatibility macros, why do we want this? Will these macros
ensure music can be copied from Red and work on Crystal without changes? Why
aren't these renamed to have the same name as Red's, then?
If any kind of porting effort is required to properly bring over music from
Red, these macros shouldn't exist.
I'm not too big on the rest macro, anyway, and the dnote macro seems
useless when it does the same as note. I don't know, I'm mostly of the
opinion that macros should map almost directly to the actual commands used.

[Rangi42]

@mid-kid has covered everything I'd comment about this. :P

One thing: since some commands only make sense in a particular channel, it might be worth adding a cur_channel N macro that comes first and sets which one we're in, so commands can check that and error if they're misused.

[dannye]

First of all, and I know this is kind of a pain in the ass to do, but you
should update the docs/music_commands.md file with the correct info for your
new macros.

Yep, I'm working on it.

Second of all, you should make sure that old songs still build with your
new macros. Add whatever is necessary to macros/legacy.asm and label
whatever is necessary with LEGACY. We don't want to lose the tonnes of
tracks made by the community over the years.

I'll take care of this too.

sound → square_note
noise → noise_note
Why? sound worked perfectly fine here, and adding _note when it's just
playing a sound, makes little sense.

I think square_note and noise_note are much less ambiguous than sound and noise.
"square" and "noise" are the type of channel. "square note" and "noise note" are notes played on that type of channel. "sound" and "noise" sound like synonyms and it isn't immediately clear that they refer to a single note.
This also matches what is already used by pokered.

The length argument, being so common over the different kinds of notes,
should either be consistently the first argument or the last. Same goes for
the frequency in note, square_note and noise_note.
I personally prefer length being the last argument, with frequency being the
first, but anything goes.

square_note and noise_note would be the only ones affected by what you're referring to, but I think it is the least confusing for the arguments to be defined in the same order they are used in.
The current argument order for square_note and noise_note is what pokered has already been using for a long time so I think to switch it up would be confusing and I don't think the reasoning justifies the change.

Are you sure (force_)stereo_panning are two booleans, instead of four
booleans for each ear? IIRC there was something with this but I don't recall.

Yes, I'm sure. Before the arguments for (force_)stereo_panning are applied, they are masked with the current channel's track mask, which pokecrystal calls "LRTracks".
So these macros only have the ability to affect the left/right output of the current channel.

As for the Red compatibility macros, why do we want this? Will these macros
ensure music can be copied from Red and work on Crystal without changes?

There are 2 problems that prevent song files from pokered being thrown straight into pokecrystal without any modification.
Number 1 is pokecrystal keeps the audio header at the top of each song, but pokered keeps all the audio headers in one location and all the song data in another location. The fix for this is as simple as copying the header into the top of the song file.
Number 2 is that noise channels in pokecrystal have to specify a drumkit id with the toggle_noise x command.

The only exception is that the stereo_panning command behaves differently, but it least it takes the same arguments, and it only affects one song from pokered (Music_Dungeon1). So the song still builds and sounds very close to correct without any modification.

This nice short list of only 2 necessary changes is only possible with these alias macros.

Why aren't these renamed to have the same name as Red's, then?
If any kind of porting effort is required to properly bring over music from
Red, these macros shouldn't exist.

Because they aren't the same macros. In the case of execute_music and toggle_sfx, in pokecrystal toggle_sfx is a true toggle that can flipped off and on (although it's never used more than once per channel per song/sfx) but in pokered execute_music is a switch that can't be flipped back for the duration of the song/sfx once it is set. Therefore, there isn't an appropriate name that can be used for both. But we can mediate compatibility by letting execute_music be an alias for toggle_sfx.
The difference is even more important for toggle_perfect_pitch and pitch_offset.
In pokered, toggle_perfect_pitch takes 0 arguments. If the flag is set, pitch frequencies for that channel are incremented by 1.
In pokecrystal, pitch_offset takes one 16-bit argument which is the value to add to each pitch frequency for that channel. This is simply functionality that didn't exist in pokered. But then this means there is no toggle_perfect_pitch command in pokecrystal. Of course. we can achieve the same effect with pitch_offset 1. So when adding pokered song files into pokecrystal, why require all toggle_perfect_pitchs to be changed when we can simply provide the equivalent alias up front?
I'm mostly thinking of pokered-crysaudio when thinking about how to maximize compatibility.
The original pokered song files should have to be changed as little as possible in order to get them to work in the pokecrystal sound engine.

I'm not too big on the rest macro, anyway

Not sure what you mean by this. rest xx is without a doubt more clear than note __, xx in music terms. It's also what pokered has used since 2013:
pret/pokered@8edfcc4#diff-0e161c2847e875f13192c5553aa46d49R3431

and the dnote macro seems
useless when it does the same as note. I don't know, I'm mostly of the
opinion that macros should map almost directly to the actual commands used.

I usually agree, but this bit of abstraction goes a long way for compatibility with pokered.
In pokered, note and dnote are completely different commands that cannot be combined into one macro. This is because dnote takes a whole byte for the instrument id (1-19).
But in pokecrystal, dnote only takes a 4-bit instrument id so therefore the structure looks a lot more like note. But they should still be separate macros for clarity. note takes a pitch constant (C_, C#, etc) but dnote takes an instrument id. Yes, technically you can use note in place of dnote but it is less clear and hurts compatibility with pokered.

Same thing with dspeed. It should be a separate macro from the 1-arg note_type because it needs to be made clear that it is only valid on the drum channel. Otherwise one can easily be misled to think that the 1-arg note_type can be used by any channel which would break the song.

[dannye]

One thing: since some commands only make sense in a particular channel, it might be worth adding a cur_channel N macro that comes first and sets which one we're in, so commands can check that and error if they're misused.

I think that's a lot of overhead and doesn't provide any kind of robust protection.

[iimarckus]

I have only one request: if this makes it in, please update pokegold as well.

[dannye]

I have only one request: if this makes it in, please update pokegold as well.

Yep, I plan on it. And pokeyellow

[dannye]

Addressed everything except for the docs, will do that tomorrow

[dannye]

Does anyone know why there is both:
https://github.com/pret/pokecrystal/blob/master/docs/music_commands.md
and
https://github.com/pret/pokecrystal/wiki/Sound-engine-commands

Seems unnecessary

[Rangi42]

"Sound engine commands" was one of the few pages on the old wiki; I didn't delete it since maybe not all of the info was in music_commands.md.

[mid-kid]

In case you haven't done this yet (I can't see it, but idk if I'm looking
right), please update the names for the commands in
audio/engine.asm:MusicCommands. If you can, use the lowercase format for
these functions found in engine/overworld/scripting.asm.

square_note and noise_note would be the only ones affected by what you're referring to, but I think it is the least confusing for the arguments to be defined in the same order they are used in.
The current argument order for square_note and noise_note is what pokered has already been using for a long time so I think to switch it up would be confusing and I don't think the reasoning justifies the change.

Here in pokecrystal we don't really care about the macro argument order
matching the order of the arguments in the actual binary, since they're often
inconsistent (as shown here), and may be transformed in various different ways
within the macro, anyway.
That said, I guess I don't mind it here as much, since apparently it only
applies to these two macros.

Not sure what you mean by this. rest xx is without a doubt more clear than note __, xx in music terms. It's also what pokered has used since 2013

It seems kind of weird to advocate for making sure macros match the binary
output while also advocating for this. Also, we don't really care about how
long something's been the case, when we're actively looking to change things
:P

That said, I guess rest seems to be interpreted by the code differently
enough to any regular note, so it works, I guess.
This leads into the next point I wanted to make, which is that it seems that
note commands are indeed interpreted differently in Noise channels (see
audio/engine.asm:ParseMusic.readnote), so I agree drum_note makes sense.
This also applies to SFX channels in audio/sfx.asm in which the note
commands will be parsed differenty (square_note) if toggle_sfx hasn't been
used. Maybe this command means something different to what it's named right
now?

However, drumkits.asm isn't a regular music script. It's parsed by
audio/engine.asm:ReadNoiseSample. As such, you can't use any other music
commands in this script. Maybe we should have a different enum for this
script? noise_note being related to square_note is wrong, anyhow, and the
sound_ret found here isn't the same as the one in regular sound scripts. I
propose simply noise and noise_end.

[mid-kid]

I also realized noise_note is being used in audio/sfx.asm. This is wrong, as this is parsed in exactly the same way as square_note. Maybe some generic name should be used for both, as it's essentially "duration, intensity, frequency". Also, are you absolutely sure these commands have four arguments? As far as I can glance from audio/engine.asm:ParseSFXOrRest there's just three.

EDIT: I take it back, they are parsed differently, and the macros seem to be implemented properly.

[mid-kid]

why require all [incompatible commands] to be changed when we can simply provide the equivalent alias up front?

My reasoning for this is mostly that if things might behave even slightly differently, it's better to error hard instead of assuming the wrong actions without warning, so the user has more to work with when porting a song from Red to Crystal or the other way around.
That said, upon closer inspection, it seems that toggle_perfect_pitch and execute_music perform the exact same operations on crystal as they would in Red, so I guess it's good to keep them.
I'm still of the opinion that they could be made to share names, but whatever.

[dannye]

In case you haven't done this yet (I can't see it, but idk if I'm looking
right), please update the names for the commands in
audio/engine.asm:MusicCommands. If you can, use the lowercase format for
these functions found in engine/overworld/scripting.asm.

I have already done this, but not with the lowercase format you are asking for.
https://github.com/pret/pokecrystal/pull/650/files#diff-a36525c353787e8a66e2473d224a4373L1376

It seems kind of weird to advocate for making sure macros match the binary
output while also advocating for this.

Sigh. Based on my other comments it's pretty obvious that I am in favor of appropriate abstraction when there is a good reason for it. And until there is a good reason, using the same order as they are used in the rom is a fine default. My position is not as contradictory as you're trying to make it.

Also, we don't really care about how
long something's been the case, when we're actively looking to change things
:P

Sigh. My point is not just to appeal to how things have always been. The whole point of this is to give pokered and pokecrystal the best reasonable compatibility. pokered has used rest xx for years and pokecrystal has used note __, xx for years. So either pokered should be changed to match pokecrystal or pokecrystal should be changed to match pokered. I'm simply arguing for the case that pokecrystal should be changed to match the more-readable, music-terminology-oriented solution that pokered has already established.

This leads into the next point I wanted to make, which is that it seems that
note commands are indeed interpreted differently in Noise channels (see
audio/engine.asm:ParseMusic.readnote), so I agree drum_note makes sense.
This also applies to SFX channels in audio/sfx.asm in which the note
commands will be parsed differenty (square_note) if toggle_sfx hasn't been
used. Maybe this command means something different to what it's named right
now?

Huh? I was following you until the end. When you say "Maybe this command" which command exactly are you referring to? toggle_sfx?
toggle_sfx switches the channel between interpreting commands in sfx format (must specify exact frequency values for all notes etc) or interpreting commands in song format (using pitches (C_, C#, etc) instead of exact frequencies).

However, drumkits.asm isn't a regular music script. It's parsed by
audio/engine.asm:ReadNoiseSample. As such, you can't use any other music
commands in this script.

That's a fair point. But..

Maybe we should have a different enum for this script?

Why? The purpose, format, and effect of noise_note in drumkits.asm vs noise_note in sfx.asm is identical.
It's true that a different routine reads drumkits.asm vs sfx.asm and it's true that other audio macros are not legal in drumkits.asm but why have separate macros if they are identical?

noise_note being related to square_note is wrong, anyhow

Huh? They are processed by the same routine, ParseSFXOrRest.
I'm hoping this is related to your next comment which you corrected, but then forgot to correct this comment here as well.

and the sound_ret found here isn't the same as the one in regular sound scripts. I
propose simply noise and noise_end.

That might technically be true... But again, when the purpose, format, and effect of sound_ret in drumkits.asm vs sound_ret in sfx.asm is identical, why separate sound_ret into sound_ret and noise_end?

In addition to being pretty pointless, this also complicates my parser/dumper that generates these files for no good reason.

[dannye]

I'm still of the opinion that they could be made to share names, but whatever.

Which ones are you referring to? toggle_perfect_pitch/pitch_offset or execute_music/toggle_sfx? Or both? I've already explained why it's not really fitting for either pair to be combined.

toggle_perfect_pitch and pitch_offset don't take the same number of arguments.

toggle_sfx is a toggle and execute_music is not.
It's just luck that toggle_sfx is never used more than once per channel per sound effect in pokecrystal so it isn't ever used like a true toggle.

[mid-kid]

Huh? I was following you until the end. When you say "Maybe this command" which command exactly are you referring to? toggle_sfx?
toggle_sfx switches the channel between interpreting commands in sfx format (must specify exact frequency values for all notes etc) or interpreting commands in song format (using pitches (C_, C#, etc) instead of exact frequencies).

Exactly, what makes this "SFX format" and not "Arbitrary square wave format"
other than that it happens to be used in SFX? Can it be used outside of the
SFX pseudo-channels?

That's a fair point. But..
Why? The purpose, format, and effect of noise_note in drumkits.asm vs noise_note in sfx.asm is identical.
It's true that a different routine reads drumkits.asm vs sfx.asm and it's true that other audio macros are not legal in drumkits.asm but why have separate macros if they are identical?

I thought they were more different than what seems to be the case, but beyond
the fact that the length of a single noise_note in drumkits.asm may only
be 15 at most, they seem to be identical.
My primary concern was that you may not use any other music commands in
drumkits, making it essentially a different kind of script (a noise script, if
you will), but I guess it's so closely related to the actual music counterpart
it'd be a bit silly to do so.

noise_note being related to square_note is wrong, anyhow

Huh? They are processed by the same routine, ParseSFXOrRest.
I'm hoping this is related to your next commend which you corrected, but then forgot to correct this comment here as well.

I said this before I realized noise_note was also used outside of the
drumkits. Sorry for that.

Which ones are you referring to? toggle_perfect_pitch/pitch_offset or execute_music/toggle_sfx? Or both? I've already explained why it's not really fitting for either pair to be combined.

toggle_perfect_pitch and pitch_offset don't take the same number of arguments.

toggle_sfx is a toggle and execute_music is not.
It's just luck that toggle_sfx is never used more than once per channel per sound effect in pokecrystal so it isn't ever used like a true toggle.

If toggle_sfx really doesn't reflect what it does, and it switches to "music
mode" (i.e. the mode where notes are interpreted using pitches), there's no
reason we couldn't name both switch_music or whatever. If the fact that the
Crystal one can be toggled is something that really should be noted, however,
I guess that's fine.

As for toggle_perfect_pitch/pitch_offset, there's no reason pitch_offset
couldn't accept simply one argument. Also why toggle_perfect_pitch when it
increments the pitch?

[dannye]

Exactly, what makes this "SFX format" and not "Arbitrary square wave format"
other than that it happens to be used in SFX? Can it be used outside of the
SFX pseudo-channels?

When a song is started (using channels 1-4) bit SOUND_SFX is reset.
When a sound effect is started (using channels 5-8) bit SOUND_SFX is set.
toggle_sfx flips this bit.

Functionally speaking, I think toggle_sfx would work if used on a song (channels 1-4) to change from pitch-based notes to frequency-based notes, but no song in pokecrystal does this.
This is not true for pokered. execute_music is a no-op for channels 1-4, so songs are not capable of switching from pitch-based notes to frequency-based notes.

On a conceptual level, being "song-based" and being "pitch-based" are virtually synonymous, and being "sound-effect-based" and being "non-pitch-based" are virtually synonymous so I don't see what the issue is.

As for toggle_perfect_pitch/pitch_offset, there's no reason pitch_offset
couldn't accept simply one argument

What? It does accept one argument.

Also why toggle_perfect_pitch when it increments the pitch?

Because it is a toggle. A name like increment_pitch would be inappropriate and misleading.

But that does make me realize something about using pitch_offset 1 as the compatibility alias for toggle_perfect_pitch.
There is one case in pokered (Music_BikeRiding) where toggle_perfect_pitch is used twice to turn it on and then off in the same channel.
This means toggle_perfect_pitch should be replaced with pitch_offset 1 in the first use and then pitch_offset 0 in the second use.
But this is the only exception from the songs in pokered.

Because of these caveats, I can agree that maybe the compatibility macros toggle_perfect_pitch and execute_music should be removed from macros/scripts/audio.asm. And instead, the documentation can be edited to describe how to translate these commands when moving songs from red to crystal.

EDIT:

but beyond the fact that the length of a single noise_note in drumkits.asm may only
be 15 at most, they seem to be identical.

No, noise_note length is not limited to 15 in drumkits.asm or sfx.asm

[mid-kid]

When a song is started (using channels 1-4) bit SOUND_SFX is reset.
When a sound effect is started (using channels 5-8) bit SOUND_SFX is set.
toggle_sfx flips this bit.

Functionally speaking, I think toggle_sfx would work if used on a song (channels 1-4) to change from pitch-based notes to frequency-based notes, but no song in pokecrystal does this.
This is not true for pokered. execute_music is a no-op for channels 1-4, so songs are not capable of switching from pitch-based notes to frequency-based notes.

On a conceptual level, being "song-based" and being "pitch-based" are virtually synonymous, and being "sound-effect-based" and being "non-pitch-based" are virtually synonymous so I don't see what the issue is.

The issue is mostly that toggle_sfx doesn't really reflect what it does. If you read the name without additional context I'd think it's something that enables/disables the SFX, not something that switches between being able to use note or square_note.

What? It does accept one argument.

Sorry, I meant it can be made to accept zero arguments.

But that does make me realize something about using pitch_offset 1 as the compatibility alias for toggle_perfect_pitch.
There is one case in pokered (Music_BikeRiding) where toggle_perfect_pitch is used twice to turn it on and then off in the same channel.
This means toggle_perfect_pitch should be replaced with pitch_offset 1 in the first use and then pitch_offset 0 in the second use.
But this is the only exception from the songs in pokered.

Implementing something like that is imo a bit out-of-scope for this repository, reimplementing a pokered music command using macro magic sounds like a slippery slope (not to mention the state of toggle_perfect_pitch would have to be properly reset between each channel, and even then it'd mess up when using any kind of branching command, such as sound_call). This is an obvious behavioral difference that should be addressed when porting the song, as should stereo_panning (What's the different behavior about that, anyway?). If anything here is pokered-compatible, it should be compatible with any arbitrary pokered song, not just whatever happens to be in the game.

[dannye]

The issue is mostly that toggle_sfx doesn't really reflect what it does. If you read the name without additional context I'd think it's something that enables/disables the SFX, not something that switches between being able to use note or square_note.

Then I think that issue mostly comes from toggle_sfx being a little too brief. Perhaps toggle_sfx_mode would be better? But I don't really find it problematic how it is currently.

Implementing something like that is imo a bit out-of-scope for this repository, reimplementing a pokered music command using macro magic sounds like a slippery slope (not to mention the state of toggle_perfect_pitch would have to be properly reset between each channel, and even then it'd mess up when using any kind of branching command, such as sound_call).

Yeah, I agree. I apologize, I think I edited my comment after you started typing your response.

This is an obvious behavioral difference that should be addressed when porting the song, as should stereo_panning (What's the different behavior about that, anyway?).

In pokered, when stereo_panning is used on any channel, it takes two 4-bit args and unconditionally uses those args as the left enable and right enable for all 4 channels (NR51).
In pokecrystal, the "4-bit args" only ever have the value $0 or $f and these "4-bit args" are masked with the channel mask for the current channel that is executing stereo_panning (%0001, %0010, etc). This means the current channel can only affect the output of itself. It cannot change the output of the other channels.

As far as "This is an obvious behavioral difference that should be addressed when porting the song", I agree. This PR does nothing to try to handle this incompatibility.

[mid-kid]

Because of these caveats, I can agree that maybe the compatibility macros toggle_perfect_pitch and execute_music should be removed from macros/scripts/audio.asm. And instead, the documentation can be edited to describe how to translate these commands when moving songs from red to crystal.

Sorry, I had missed this part in your edit. I completely agree. Either a tutorial on the wiki on how to port a song, or simply a list of commands that differ between the games with possible caveats would make it easy to port a song, while making it easier to spot problems while doing so.

[rawr51919]

Can this be updated and the conflicts resolved?

[mid-kid]

Once you're done making your final changes, this can be merged. Everything looks good.
It should be tested if old songs still work without modification, however.

[dannye]

I already fully tested the new macros/scripts/audio.asm and macros/legacy.asm on the old audio/music/, and audio/sfx.asm files etc. I can do a final re-test before merging.

[Rangi42]

Before merging, I hope we can get feedback from at least one of the people who demixes or writes music for pokecrystal. The legacy macros will allow old music files to still work, but the command changes ought to be helpful for actually hand-writing music, or processing it with custom tools.

Let's also merge pokered PR 223 simultaneously.

[dannye]

Before merging, I hope we can get feedback from at least one of the people who demixes or writes music for pokecrystal. The legacy macros will allow old music files to still work, but the command changes ought to be helpful for actually hand-writing music, or processing it with custom tools.

I am someone that hand-writes songs for pokecrystal and pokered :)
https://github.com/dannye/CrystalComplete/blob/music/audio/music/custom/prototype_route1.asm
https://github.com/dannye/pokered-prototype/blob/master/audio/music/titlescreen.asm

Let's also merge pokered PR 223 simultaneously.

That's the idea :)

[Rangi42]

I am someone that hand-writes songs for pokecrystal and pokered :)

Fair enough. :P Still, it's easier to understand the new API when you've designed it.

[Rangi42]

make compare is OK, even with the old music files due to legacy macros.

I see the updated music files still have .local labels; thank you for that. Is the conversion tool already updated to output those?

The docs aren't complete, but they never were. :P

Overall it looks good! It'll be nice to have similarity with pokered.