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.
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
VideoCommon: Reword the unknown opcode error message #11586
Conversation
When faced with this error, users often don't try disabling dual core, even though the error message suggests it. Perhaps the message is just too long and lists too many things? To try to improve the situation, I'm rewording the message and making it say different things depending on what settings you are using.
| "This means one of the following:\n" | ||
| "* The emulated GPU got desynced, disabling dual core can help\n" | ||
| "* Command stream corrupted by some spurious memory bug\n" | ||
| "* This really is an unknown opcode (unlikely)\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"* This really is an unknown opcode (unlikely)\n"
I must admit, I do have a fondness for this line. I'd like it if you could say something like "This may actually be an unknown opcode" for the case that where the obvious answers don't apply.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't we already enumerated and investigated all the possible opcodes by now? In particular, I'm thinking of the last sentence of this comment:
dolphin/Source/Core/VideoCommon/CommandProcessor.cpp
Lines 644 to 654 in fbb3db7
| // Datel software uses 0x01 during startup, and Mario Party 5's Wiggler capsule accidentally uses | |
| // 0x01-0x03 due to sending 4 more vertices than intended (see https://dolp.in/i8104). | |
| // Prince of Persia: Rival Swords sends 0x3f if the home menu is opened during the intro cutscene | |
| // due to a game bug resulting in an incorrect vertex desc that results in the float value 1.0, | |
| // encoded as 0x3f800000, being parsed as an opcode (see https://dolp.in/i9203). | |
| // | |
| // Hardware testing indicates that these opcodes do nothing, so to avoid annoying the user with | |
| // spurious popups, we don't create a panic alert in those cases. Other unknown opcodes | |
| // (such as 0x18) seem to result in actual hangs on real hardware, so the alert still is important | |
| // to keep around for unexpected cases. | |
| const bool suppress_panic_alert = (cmd_byte <= 0x7) || (cmd_byte == 0x3f); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, we thought we knew all of the opcodes before too, and well... https://dolphin-emu.org/blog/2022/02/08/dolphin-progress-report-nov-and-dec-2021-jan-2022/#50-15931-disable-unknown-opcode-popup-for-0x1-to-0x7-by-pokechu22
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps you could have the message talk about it being an unknown opcode in single core only?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's what May is suggesting. I would appreciate if someone who knows more about the GC GPU could comment on this, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I think this is an improvement.
As for unknown opcodes... I'm not sure if it's worth showing every user who runs into this (which will be many) a suggestion that both doesn't help them and is also exceedingly unlikely. The current message made sense when Dolphin wasn't mature yet, but I don't think that it's worth keeping that part these days.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe poke @Pokechu22 on this; have you ever tried mapping out all the possible opcodes and how the unusual ones behave on hardware?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't explicitly test all of them. I determined that some invalid opcodes result in hangs, and others are just ignored (though I wasn't scientific in testing if it was a single byte that was ignored or what happened).
For what it's worth, there's still GX_CMD_UNKNOWN_METRICS (0x44) and GX_CMD_INVL_VC (0x48), neither of which we currently handle. The latter is used by many games and is related to the vertex cache (which is not emulated currently, see #10383); I have no idea what the former does (this comment says it's used by Zelda Four Swords though).
Patent US6411301 (and a few others) has a table that lists various "example" opcodes (Table I, page 30 in the PDF), including xxx for various bits that are presumably ignored (but dolphin doesn't ignore). It also specifies SU_ByPassCmd as 0x60|SUatrr, where what dolphin refers to as a BPMem command is 0x61 (and that seems to be the only one of that form). Theoretically there could be other hardware that is addressed using command 0x60, 0x62, etc (and I think there might be a second patent with a diagram that mentions this, but I can't find it), but I don't think any game actually does this.
It's highly unlikely that any games intentionally use an opcode that triggers our unknown opcode message to do something interesting. There are still cases where games accidentally write something invalid, but there's only been a few of them.
|
This is a definite improvement. Is there any reason not to merge this and sort out the absolute edge-cases later on? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
While I will miss the "This really is an unknown opcode (unlikely)" language, the way this PR handles this situation is way better than current master. Why is this not merged yet.
When faced with this error, users often don't try disabling dual core, even though the error message suggests it. Perhaps the message is just too long and lists too many things?
To try to improve the situation, I'm rewording the message and making it say different things depending on what settings you are using.