Skip to content

Display Additional Information When Printing Instrument Data #1

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.

Sign up for GitHub

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

Open
Kurausukun wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Display Additional Information When Printing Instrument Data #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 36 additions & 8 deletions src/instrumentdata.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,18 +88,19 @@ MpInstrument* MpInstrument::load(const ROMFile* rom, uint32_t addr, bool isSplit
}

uint8_t normType = type;
if (normType < 0x10) {
normType &= 0x7;
}
try {
switch (normType) {
case GBSample:
case GBSample_Alt:
case Sample:
case FixedSample:
return new SampleInstrument(rom, addr);
case Square1:
case Square1_Alt:
case Square2:
case Square2_Alt:
case Noise:
case Noise_Alt:
Comment on lines 90 to +103
Copy link
Owner

@ahigerd ahigerd Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I was going to say that these changes aren't necessary, but then I realized that this is better from a coding style perspective. +1

return new PSGInstrument(rom, addr);
case KeySplit:
case Percussion:
Expand All @@ -122,8 +123,8 @@ MpInstrument* MpInstrument::load(const ROMFile* rom, uint32_t addr, bool isSplit
MpInstrument::MpInstrument(const ROMFile* rom, uint32_t addr)
: rom(rom), addr(addr), type(Type(rom->read<uint8_t>(addr))), forcePan(false), pan(0), gate(0)
{
if (type & 0x7) {
type = Type(type & 0x7);
if (type < 16 && type != 0 && type != 8) {
Copy link
Owner

@ahigerd ahigerd Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you're going to write the condition out like this, switch the if and else-if cases so you don't have to repeat the 0 and 8 checks.

type = Type(type);
attack = (rom->read<uint8_t>(addr + 8) & 0x7) / 7.0;
decay = rom->read<uint8_t>(addr + 9) / 60.0;
sustain = rom->read<uint8_t>(addr + 10) / 15.0;
Expand Down Expand Up @@ -255,6 +256,8 @@ std::string SampleInstrument::displayName() const
std::ostringstream ss;
if (type == GBSample) {
ss << "Waveform";
} else if (type == GBSample_Alt) {
ss << "Waveform (Alt)";
} else {
ss << "Sample";
}
Expand Down Expand Up @@ -296,7 +299,7 @@ Channel::Note* SampleInstrument::noteEvent(Channel* channel, std::shared_ptr<Bas
PSGInstrument::PSGInstrument(const ROMFile* rom, uint32_t addr)
: MpInstrument(rom, addr), sweep(0)
{
if (type == Square1) {
if (type == Square1 || type == Square1_Alt) {
sweep = rom->read<uint8_t>(addr + 3);
}
mode = rom->read<uint8_t>(addr + 4);
Expand All @@ -314,8 +317,16 @@ PSGInstrument::PSGInstrument(const ROMFile* rom, uint32_t addr)
std::string PSGInstrument::displayName() const
{
std::ostringstream ss;
if (type == Square1 || type == Square2) {
ss << "Square ";
if (type == Square1 || type == Square1_Alt || type == Square2 || type == Square2_Alt) {
if (type == Square1) {
ss << "Square 1 ";
} else if (type == Square1_Alt) {
ss << "Square 1 (Alt) ";
} else if (type == Square2) {
ss << "Square 2 ";
} else if (type == Square2_Alt) {
ss << "Square 2 (Alt) ";
}
if (mode == 0) {
ss << "(12.5%)";
} else if (mode == 1) {
Expand All @@ -327,6 +338,8 @@ std::string PSGInstrument::displayName() const
}
} else if (type == Noise) {
ss << "Noise (type " << int(mode) << ")";
} else if (type == Noise_Alt) {
ss << "Noise (Alt) (type " << int(mode) << ")";
} else {
ss << "PSG (type " << int(type) << ")";
}
Expand Down Expand Up @@ -485,6 +498,21 @@ void MpInstrument::showParsed(std::ostream& out, std::string indent) const
out << indent << displayName() << ":" << std::endl;
out << indent << " Base address: 0x" << std::hex << addr << std::dec << std::endl;
if (attack >= 0) {
Copy link
Owner

@ahigerd ahigerd Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is my code but now that we've got more content in here I think I would prefer to see this become if (attack < 0) { return; } so the rest of the logic can drop an indent level.

out << indent << " Base key=" << int(rom->read<uint8_t>(addr + 1)) << std::endl;
if (type == Square1 || type == Square1_Alt || type == Square2 || type == Square2_Alt || type == GBSample || type == GBSample_Alt
Copy link
Owner

@ahigerd ahigerd Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use type & 0x7 to simplify this expression.

|| type == Noise || type == Noise_Alt) {
out << indent << " Gate=" << int(rom->read<uint8_t>(addr + 2)) << std::endl;
if (type == Square1 || type == Square1_Alt) {
out << indent << " Sweep=" << int(rom->read<uint8_t>(addr + 3)) << std::endl;
}
} else {
int pan = rom->read<uint8_t>(addr + 2);
if (pan == 0) {
out << indent << " Pan=" << pan << std::endl;
} else {
out << indent << " Pan=" << (pan | 0x80) << std::endl;
Copy link
Owner

@ahigerd ahigerd Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does 0 pan actually mean "centered" or does it mean that it doesn't modify the channel pan? (And do you know if 0 means hard left for channel pan?) If it means it doesn't modify it I think it would be better to leave this line out.

Copy link
Owner

@ahigerd ahigerd Aug 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... also I thought we determined that it was always pan and never gate.

}
}
out << indent << " A=" << int(rom->read<uint8_t>(addr + 8)) << " (" << attack << ")" << std::endl;
out << indent << " D=" << int(rom->read<uint8_t>(addr + 9)) << " (" << decay << ")" << std::endl;
out << indent << " S=" << int(rom->read<uint8_t>(addr + 10)) << " (" << sustain << ")" << std::endl;
Expand Down
4 changes: 4 additions & 0 deletions src/instrumentdata.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ class MpInstrument: public IInstrument {
GBSample = 3,
Noise = 4,
FixedSample = 8,
Square1_Alt = 9,
Square2_Alt = 10,
GBSample_Alt = 11,
Noise_Alt = 12,
KeySplit = 0x40,
Percussion = 0x80,
};
Expand Down