Added status bar #40

ChrisBlueStone · 2023-04-10T02:36:23Z

Added a status bar to the app that displays helpful notes about the currently selected command in the tracker tab and ADSR info for the currently selected instrument in the instruments tab.

Added status bar and code tips

Split the status bar to multiple parts so more useful info can be displayed in the other parts.

- Removed the multiple sections in the status bar until they're useful for something. - Tweaked some tracker help messages.

Added a menu item to toggle the status bar's visibility

Added format attribute to format_status function

- Made the sign casting explicit since some printf implementations don't support %hhd. - Wrapped variable declaration within braces for switch case.

PhoenixBound

Code LGTM

PhoenixBound · 2023-04-16T05:10:28Z

src/inst.c

+					int sel = SendMessage((HWND)lParam, LB_GETCURSEL, 0, 0);
+					struct channel_state *c = &state.chan[0];
+					set_inst(&state, c, valid_insts[sel]);
+					format_status(0, "ADSR: %02d/15  %d/7  %d/7  %02d/31", 0xF - (c->inst_adsr1 & 0xF), (c->inst_adsr1 >> 4) & 0x7, (c->inst_adsr2 >> 5) & 7, c->inst_adsr2 & 0x1F);


Most resources for SNES envelopes out there leave attack as "higher = ramps up faster," so it might be better to drop the 0xF - attack part and display it raw. Your choice, though.

This doesn't handle GAIN either (when (c->inst_adsr1 & 0x80) == 0) but that's not relevant to EarthBound.

Compared to actual ADSR this is weird, but ok

PhoenixBound · 2023-04-16T05:14:42Z

src/status.c

+		format_status(p, "Percussion %d", code[0] - 0xC9);
+	} else {
+		switch (code[0]) {
+			case 0xE0: format_status(p, "Instrument %d", code[1]); break;


To-do: special case for code[1] >= 0xCA, since that changes to a percussion instrument

Added special note for values >= 0xCA

PhoenixBound · 2023-04-16T05:16:32Z

src/status.c

+	} else {
+		switch (code[0]) {
+			case 0xE0: format_status(p, "Instrument %d", code[1]); break;
+			case 0xE1: format_status(p, "Pan %d", code[1]); break;


To-do: the two most-significant bits in code[1] are bit flags to invert the left/right channels. Only in [E1 XX] though, not the other pan code.

Added additional note for phase inversions

PhoenixBound · 2023-04-16T05:19:16Z

src/status.c

+			case 0xE9: format_status(p, "Global transpose %d", (signed char)code[1]); break;
+			case 0xEA: format_status(p, "Channel transpose %d", (signed char)code[1]); break;


Personal preference: %d --> %+d, so that it shows as +1 for positive numbers. I can confirm this works correctly with msvcrt.

PhoenixBound · 2023-04-16T05:28:16Z

src/status.c

+			case 0xF1: format_status(p, "Enable portamento (delay: %d, duration: %d, range: +%d)", code[1], code[2], code[3]); break;
+			case 0xF2: format_status(p, "Enable portamento (delay: %d, duration: %d, range: -%d)", code[1], code[2], code[3]); break;


This description doesn't seem accurate. The difference between F1 and F2 is that F1 starts at the target note and bends up/down range semitones (cast it to signed char!), while F2 starts displaced and bends back up/down to the actual named note.

What would be a good way to describe that... on SNESLab, they're called "Note Pitch Envelope To" and "Note Pitch Envelope From". Maybe something like "Enable portamento away from note" and "Enable portamento back to note"?

Modified this slightly

PhoenixBound · 2023-04-16T05:31:35Z

src/status.c

+			case 0xF5:
+				format_status(p, "Enable echo (channels: %c%c%c%c%c%c%c%c, left volume: %d/255, right volume: %d/255)",
+					'0' + (code[1] & 1),
+					'0' + ((code[1] >> 1) & 1),
+					'0' + ((code[1] >> 2) & 1),
+					'0' + ((code[1] >> 3) & 1),
+					'0' + ((code[1] >> 4) & 1),
+					'0' + ((code[1] >> 5) & 1),
+					'0' + ((code[1] >> 6) & 1),
+					'0' + ((code[1] >> 7) & 1),
+					code[2],
+					code[3]);


I believe left volume and right volume are more signed chars. These numbers are written directly to DSP registers and they're out of 128. (Docs: fullsnes, S-DSP REGISTERS section of anomie's DSP documentation

Changed this to signed values without specifying phase inversions. Hopefully that should be intuitive

PhoenixBound · 2023-04-16T05:33:04Z

src/status.c

+					code[3]);
+				break;
+			case 0xF6: format_status(p, "Disable echo"); break;
+			case 0xF7: format_status(p, "Echo settings (delay: %d, feedback: %d, filter: %d)", code[1], code[2], code[3]); break;


Feedback is like a reverb strength setting. So basically another signed char volume out of 128

PhoenixBound · 2023-04-16T05:34:36Z

src/status.c

+				break;
+			}
+			case 0xFA: format_status(p, "Base percussion instrument %d", code[1]); break;
+			case 0xFB: format_status(p, "Unknown [FB %02X %02X]", code[1], code[2]); break;


This command doesn't do anything. It calls the function to consume an extra byte and then immediately returns: relevant disassembly section

Relabeled to No op

PhoenixBound

It builds on my end. I haven't thoroughly tested it or anything but it all looks good.

ChrisBlueStone added 5 commits April 6, 2023 19:43

Added status bar

1885d71

Added status bar and code tips

Fixed some whitespaces

086b937

Split status bar

4ba887d

Split the status bar to multiple parts so more useful info can be displayed in the other parts.

Removed status parts

bf4b4de

- Removed the multiple sections in the status bar until they're useful for something. - Tweaked some tracker help messages.

Added menu item for status bar

6d4a3f0

Added a menu item to toggle the status bar's visibility

ChrisBlueStone requested a review from PhoenixBound April 10, 2023 02:36

ChrisBlueStone self-assigned this Apr 10, 2023

ChrisBlueStone added 3 commits April 9, 2023 23:58

Added format attribute

317c1e3

Added format attribute to format_status function

Added status.c to solution files

821cd26

Fixed compiler issues

330a15f

- Made the sign casting explicit since some printf implementations don't support %hhd. - Wrapped variable declaration within braces for switch case.

PhoenixBound reviewed Apr 16, 2023

View reviewed changes

Addressed PR comments

c6f647e

ChrisBlueStone requested a review from PhoenixBound April 16, 2023 20:53

PhoenixBound approved these changes Apr 16, 2023

View reviewed changes

ChrisBlueStone merged commit 73ddfae into PKHackers:master Apr 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added status bar #40

Added status bar #40

ChrisBlueStone commented Apr 10, 2023

PhoenixBound left a comment

PhoenixBound Apr 16, 2023

ChrisBlueStone Apr 16, 2023

PhoenixBound Apr 16, 2023

ChrisBlueStone Apr 16, 2023

PhoenixBound Apr 16, 2023

ChrisBlueStone Apr 16, 2023

PhoenixBound Apr 16, 2023

ChrisBlueStone Apr 16, 2023

PhoenixBound Apr 16, 2023

ChrisBlueStone Apr 16, 2023

PhoenixBound Apr 16, 2023

ChrisBlueStone Apr 16, 2023

PhoenixBound Apr 16, 2023

ChrisBlueStone Apr 16, 2023

PhoenixBound Apr 16, 2023

ChrisBlueStone Apr 16, 2023

PhoenixBound left a comment

		case 0xE9: format_status(p, "Global transpose %d", (signed char)code[1]); break;
		case 0xEA: format_status(p, "Channel transpose %d", (signed char)code[1]); break;

		case 0xF1: format_status(p, "Enable portamento (delay: %d, duration: %d, range: +%d)", code[1], code[2], code[3]); break;
		case 0xF2: format_status(p, "Enable portamento (delay: %d, duration: %d, range: -%d)", code[1], code[2], code[3]); break;

Added status bar #40

Added status bar #40

Conversation

ChrisBlueStone commented Apr 10, 2023

PhoenixBound left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhoenixBound left a comment

Choose a reason for hiding this comment