Display Additional Information When Printing Instrument Data#1
Display Additional Information When Printing Instrument Data#1Kurausukun wants to merge 1 commit intoahigerd:masterfrom
Conversation
| 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: |
There was a problem hiding this comment.
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
| { | ||
| if (type & 0x7) { | ||
| type = Type(type & 0x7); | ||
| if (type < 16 && type != 0 && type != 8) { |
There was a problem hiding this comment.
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.
| out << indent << " Base address: 0x" << std::hex << addr << std::dec << std::endl; | ||
| if (attack >= 0) { | ||
| 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 |
There was a problem hiding this comment.
Use type & 0x7 to simplify this expression.
| @@ -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) { | |||
There was a problem hiding this comment.
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.
| if (pan == 0) { | ||
| out << indent << " Pan=" << pan << std::endl; | ||
| } else { | ||
| out << indent << " Pan=" << (pan | 0x80) << std::endl; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
... also I thought we determined that it was always pan and never gate.
2 participants
No description provided.