Skip to content
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

Fix AbortOnPanicAlert with PanicAlertFmt #10138

Merged
merged 1 commit into from Oct 3, 2021

Conversation

Pokechu22
Copy link
Contributor

PR #10066 added functionality to call std::abort when a panic alert occurs; however, that PR only implemented it for MsgAlert and not MsgAlertFmtImpl, meaning that the functionality was not used with PanicAlertFmt (only PanicAlert, which is not used frequently).

@Pokechu22
Copy link
Contributor Author

fifa-street and medabots-crash now fail to render. fifa-street is failing here:

constexpr u32 GetElementSize(ComponentFormat format)
{
switch (format)
{
case ComponentFormat::UByte:
case ComponentFormat::Byte:
return 1;
case ComponentFormat::UShort:
case ComponentFormat::Short:
return 2;
case ComponentFormat::Float:
return 4;
default:
PanicAlertFmt("Unknown format {}", format);
return 0;
}
}

The normal format and the format of both colors is set to 5, which is invalid (the other components are set to 4, indicating float). medabots-crash has invalid pointer errors. So, them failing to render is an expected result and should be fixed by resolving the underlying bugs.

@phire
Copy link
Member

phire commented Sep 30, 2021

Do you want to fix the FIFO_CI failures before merging this?

@Pokechu22
Copy link
Contributor Author

I don't think they have an easy fix, so I'm fine with leaving them erroring for now until I have time to research them further.

@Pokechu22 Pokechu22 force-pushed the abort-on-panic-alert-fmt branch 2 times, most recently from 2b52c96 to bad2217 Compare October 1, 2021 02:13

return true;
return ShowMessageAlert(caption, message.c_str(), yes_no, style);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The use of message.c_str() means the previous string_view change doesn't do anything. Will replace with just message.

PR dolphin-emu#10066 added functionality to call std::abort when a panic alert occurs; however, that PR only implemented it for MsgAlert and not MsgAlertFmtImpl, meaning that the functionality was not used with PanicAlertFmt (only PanicAlert, which is not used frequently).
@dolphin-emu-bot
Copy link
Contributor

FifoCI detected that this change impacts graphical rendering. Here are the behavior differences detected by the system:

automated-fifoci-reporter

@leoetlino leoetlino merged commit 480cd35 into dolphin-emu:master Oct 3, 2021
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants