Skip to content

Isolate wxWidgets code from non-GUI code#1633

Merged
Exzap merged 11 commits into
cemu-project:mainfrom
SSimco:wx_widgets_decouple
Jul 15, 2025
Merged

Isolate wxWidgets code from non-GUI code#1633
Exzap merged 11 commits into
cemu-project:mainfrom
SSimco:wx_widgets_decouple

Conversation

@SSimco

@SSimco SSimco commented Jul 7, 2025

Copy link
Copy Markdown
Collaborator

This PR refactors the core code to remove its dependency on wxWidgets. These changes should make it easier to add a different frontend to Cemu. A new frontend would only need to implement the interface defined in src/gui/interface/WindowSystem.h.

Changes:

  • Added a interface in src/gui/interface/WindowSystem.h, which the core code will use to communicate with the UI code. The wxWidgets implementation is in src/gui/wxgui/wxWindowSystem.cpp (which replaces the old guiWrapper.cpp).
  • Updated GLCanvas, LoggingWindow, and DebuggerWindow to register with the core using callbacks.
  • Moved all translatable strings (marked with wxTRANSLATE or _()) to the wxWidgets subproject.
  • Replaced all usages of wxMessageBox in non-GUI code to just log the errors, or just removed them.

@Exzap

Exzap commented Jul 7, 2025

Copy link
Copy Markdown
Member

First of all thank you for submitting this, it's definitively a huge step in the right direction. Just a heads up, but there is an earlier PR that will conflict with these changes #1519, which out of fairness I have to merge first. They have been waiting a while.

Moved all translatable strings (marked with wxTRANSLATE or _()) to the wxWidgets subproject.

I think on paper this sounds like the right move but in practice it comes with many downsides. Some strings are formatted and translated in the backend by necessity. Of course there is always a workaround be it via error enums or more complex callback functions, but the simplicity of the _("string") macro and doing formatting on the spot is definitively preferable and also avoids having to duplicate translate-able strings into all the frontends.

I think the preferable solution would be to define a custom helper function in the backend aka std::string _cemuTranslate(std::string_view) (feel free to think of a shorter name) which is decoupled from wxWidgets and instead uses a callback to the UI to translate the string. There is no reason to use wxString directly up to the point where we are actually calling wx functions, so in the backend all string handling and formatting could be done on platform independent std::string (utf8 encoded as they already are). Of course wxWidget's implementation of the translation callback would then have to convert from std::string to wxString and back, but that's fine.

Replaced all usages of wxMessageBox in non-GUI code to just log the errors, or just removed them.

If it's only logged it means the user will not be made aware of those errors unless they explicitly search the log for it. I think it makes more sense to have a callback that can be used to forward error box messages to the UI frontend. I believe that on Android you don't want message pop ups for most of the stuff because thats bad UI design there? As a potential solution you could pass an additional error id enum (but optional, so we dont have to define it for every single message box, no need to over-engineer) so that the front end can decide to instead handle it some other way if a message box isn't wanted.

On a related note since I saw it while skimming the changes, it's safe to drop the CheckAndMoveLegacySaves function as a whole since it's beyond ancient, no need to bother with it anymore.

@SSimco SSimco force-pushed the wx_widgets_decouple branch from f9d118e to e6e6e16 Compare July 8, 2025 18:27
@SSimco

SSimco commented Jul 8, 2025

Copy link
Copy Markdown
Collaborator Author

First of all thank you for submitting this, it's definitively a huge step in the right direction. Just a heads up, but there is an earlier PR that will conflict with these changes #1519, which out of fairness I have to merge first. They have been waiting a while.

Moved all translatable strings (marked with wxTRANSLATE or _()) to the wxWidgets subproject.

I think on paper this sounds like the right move but in practice it comes with many downsides. Some strings are formatted and translated in the backend by necessity. Of course there is always a workaround be it via error enums or more complex callback functions, but the simplicity of the _("string") macro and doing formatting on the spot is definitively preferable and also avoids having to duplicate translate-able strings into all the frontends.

I think the preferable solution would be to define a custom helper function in the backend aka std::string _cemuTranslate(std::string_view) (feel free to think of a shorter name) which is decoupled from wxWidgets and instead uses a callback to the UI to translate the string. There is no reason to use wxString directly up to the point where we are actually calling wx functions, so in the backend all string handling and formatting could be done on platform independent std::string (utf8 encoded as they already are). Of course wxWidget's implementation of the translation callback would then have to convert from std::string to wxString and back, but that's fine.

Replaced all usages of wxMessageBox in non-GUI code to just log the errors, or just removed them.

If it's only logged it means the user will not be made aware of those errors unless they explicitly search the log for it. I think it makes more sense to have a callback that can be used to forward error box messages to the UI frontend. I believe that on Android you don't want message pop ups for most of the stuff because thats bad UI design there? As a potential solution you could pass an additional error id enum (but optional, so we dont have to define it for every single message box, no need to over-engineer) so that the front end can decide to instead handle it some other way if a message box isn't wanted.

On a related note since I saw it while skimming the changes, it's safe to drop the CheckAndMoveLegacySaves function as a whole since it's beyond ancient, no need to bother with it anymore.

I've updated the code.

The backend can now translate strings using the _tr function or mark them as translate-able using the TR_NOOP macro. The frontend would have to register the translation callback by calling the SetTranslationCallback function.

I've also added a new function in the GUI interface for showing error dialogs (WindowSystem::ShowErrorDialog), that takes an optional error category.

@SSimco SSimco force-pushed the wx_widgets_decouple branch from bfb69c0 to 0d1c6bd Compare July 10, 2025 17:48
@SSimco SSimco force-pushed the wx_widgets_decouple branch from 0d1c6bd to b95f833 Compare July 10, 2025 17:50
@SSimco SSimco force-pushed the wx_widgets_decouple branch from 4a01ac5 to c038e19 Compare July 12, 2025 08:11
Comment thread src/config/CemuConfig.cpp
}

void CemuConfig::Load(XMLConfigParser& parser)
XMLConfigParser CemuConfig::Load(XMLConfigParser& parser)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I couldn't really make sense of this change. Why are CemuConfig::Load and CemuConfig::Save returning a copy of the XMLConfigParser thats passed by reference?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The intent was to return a parser for a subtree XML element where child settings would read/store their settings.
And CemuConfig returns a parser for the content XML element instead of the original parser.

XMLConfigParser CemuConfig::Load(XMLConfigParser& parser)
{
	auto new_parser = parser.get("content");
	if (new_parser.valid())
		parser = new_parser;

	// ...

	return parser;
}

XMLConfigParser CemuConfig::Save(XMLConfigParser& parser)
{
	auto config = parser.set("content");

	// ...

	return config;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah I didn't see the reassignment. Thanks

@Exzap

Exzap commented Jul 15, 2025

Copy link
Copy Markdown
Member

All looking good. Thanks again for working on this

@Exzap Exzap merged commit 67de63b into cemu-project:main Jul 15, 2025
6 checks passed
@SSimco SSimco deleted the wx_widgets_decouple branch July 15, 2025 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants