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

BACKENDS: Use Common::U32String for OSystem::setWindowCaption #2588

Merged
merged 1 commit into from
Nov 22, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions backends/platform/android/android.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -581,9 +581,7 @@ void OSystem_Android::quit() {
pthread_join(_timer_thread, 0);
}

void OSystem_Android::setWindowCaption(const char *caption) {
ENTER("%s", caption);

void OSystem_Android::setWindowCaption(const Common::U32String &caption) {
JNI::setWindowCaption(caption);
}

Expand Down
2 changes: 1 addition & 1 deletion backends/platform/android/android.h
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class OSystem_Android : public ModularMutexBackend, public ModularGraphicsBacken

virtual void quit() override;

virtual void setWindowCaption(const char *caption) override;
virtual void setWindowCaption(const Common::U32String &caption) override;

virtual Audio::Mixer *getMixer() override;
virtual void getTimeAndDate(TimeDate &t) const override;
Expand Down
4 changes: 2 additions & 2 deletions backends/platform/android/jni-android.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,9 @@ bool JNI::isConnectionLimited() {
return limited;
}

void JNI::setWindowCaption(const Common::String &caption) {
void JNI::setWindowCaption(const Common::U32String &caption) {
JNIEnv *env = JNI::getEnv();
jstring java_caption = convertToJString(env, caption.decode(Common::kISO8859_1));
jstring java_caption = convertToJString(env, caption);

env->CallVoidMethod(_jobj, _MID_setWindowCaption, java_caption);

Expand Down
2 changes: 1 addition & 1 deletion backends/platform/android/jni-android.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class JNI {

static void setReadyForEvents(bool ready);

static void setWindowCaption(const Common::String &caption);
static void setWindowCaption(const Common::U32String &caption);
static void getDPI(float *values);
static void displayMessageOnOSD(const Common::U32String &msg);
static bool openUrl(const Common::String &url);
Expand Down
4 changes: 1 addition & 3 deletions backends/platform/android3d/android.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -490,9 +490,7 @@ void OSystem_Android::quit() {
dynamic_cast<AndroidGraphicsManager *>(_graphicsManager)->deinitSurface();
}

void OSystem_Android::setWindowCaption(const char *caption) {
ENTER("%s", caption);

void OSystem_Android::setWindowCaption(const Common::U32String &caption) {
JNI::setWindowCaption(caption);
}

Expand Down
2 changes: 1 addition & 1 deletion backends/platform/android3d/android.h
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ class OSystem_Android : public ModularMutexBackend, public ModularGraphicsBacken

virtual void quit();

virtual void setWindowCaption(const char *caption);
virtual void setWindowCaption(const Common::U32String &caption);
virtual void showVirtualKeyboard(bool enable);

virtual Audio::Mixer *getMixer();
Expand Down
2 changes: 1 addition & 1 deletion backends/platform/dc/dc.h
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ class OSystem_Dreamcast : private DCHardware, public EventsBaseBackend, public P

// Set a window caption or any other comparable status display to the
// given value.
void setWindowCaption(const char *caption);
void setWindowCaption(const Common::U32String &caption);

// Modulatized backend
Audio::Mixer *getMixer() { return _mixer; }
Expand Down
6 changes: 3 additions & 3 deletions backends/platform/dc/dcmain.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@


Icon icon;
const char *gGameName;
const char gGameName[32];


OSystem_Dreamcast::OSystem_Dreamcast()
Expand Down Expand Up @@ -143,9 +143,9 @@ bool DCCDManager::isPlaying() const {
return getCdState() == 3;
}

void OSystem_Dreamcast::setWindowCaption(const char *caption)
void OSystem_Dreamcast::setWindowCaption(const Common::U32String &caption)
{
gGameName = caption;
Common::strlcpy(gGameName, cap.encode(Common::kISO8859_1).c_str(), 32);
}

void OSystem_Dreamcast::quit() {
Expand Down
2 changes: 1 addition & 1 deletion backends/platform/dc/vmsave.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ class VMSaveManager : public Common::SaveFileManager {

void OutVMSave::finalize()
{
extern const char *gGameName;
extern const char gGameName[32];
extern Icon icon;

if (committed >= _pos)
Expand Down
17 changes: 2 additions & 15 deletions backends/platform/maemo/maemo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -99,21 +99,8 @@ void OSystem_SDL_Maemo::setXWindowName(const char *caption) {
}
}

void OSystem_SDL_Maemo::setWindowCaption(const char *caption) {
Common::String cap;
byte c;

// The string caption is supposed to be in LATIN-1 encoding.
// SDL expects UTF-8. So we perform the conversion here.
while ((c = *(const byte *)caption++)) {
if (c < 0x80)
cap += c;
else {
cap += 0xC0 | (c >> 6);
cap += 0x80 | (c & 0x3F);
}
}

void OSystem_SDL_Maemo::setWindowCaption(const Common::U32String &caption) {
Common::String cap = caption.encode();
_window->setWindowCaption(cap);

Common::String cap2("ScummVM - "); // 2 lines in OS2008 task switcher, set first line
Expand Down
2 changes: 1 addition & 1 deletion backends/platform/maemo/maemo.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class OSystem_SDL_Maemo final : public OSystem_POSIX {
virtual void initBackend() override;
virtual void quit() override;
virtual void fatalError() override;
virtual void setWindowCaption(const char *caption) override;
virtual void setWindowCaption(const Common::U32String &caption) override;
virtual Common::HardwareInputSet *getHardwareInputSet() override;
virtual Common::KeymapArray getGlobalKeymaps() override;
virtual Common::KeymapperDefaultBindings *getKeymapperDefaultBindings() override;
Expand Down
17 changes: 2 additions & 15 deletions backends/platform/sdl/sdl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -470,21 +470,8 @@ void OSystem_SDL::addSysArchivesToSearchSet(Common::SearchSet &s, int priority)

}

void OSystem_SDL::setWindowCaption(const char *caption) {
Common::String cap;
byte c;

// The string caption is supposed to be in LATIN-1 encoding.
// SDL expects UTF-8. So we perform the conversion here.
while ((c = *(const byte *)caption++)) {
if (c < 0x80)
cap += c;
else {
cap += 0xC0 | (c >> 6);
cap += 0x80 | (c & 0x3F);
}
}

void OSystem_SDL::setWindowCaption(const Common::U32String &caption) {
Common::String cap = caption.encode();
_window->setWindowCaption(cap);
}

Expand Down
2 changes: 1 addition & 1 deletion backends/platform/sdl/sdl.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ class OSystem_SDL : public ModularMutexBackend, public ModularMixerBackend, publ
virtual bool setTextInClipboard(const Common::U32String &text) override;
#endif

virtual void setWindowCaption(const char *caption) override;
virtual void setWindowCaption(const Common::U32String &caption) override;
virtual void addSysArchivesToSearchSet(Common::SearchSet &s, int priority = 0) override;
virtual uint32 getMillis(bool skipRecord = false) override;
virtual void delayMillis(uint msecs) override;
Expand Down
4 changes: 0 additions & 4 deletions backends/platform/wii/osystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -257,10 +257,6 @@ void OSystem_Wii::deleteMutex(MutexRef mutex) {
free(mutex);
}

void OSystem_Wii::setWindowCaption(const char *caption) {
printf("window caption: %s\n", caption);
}

Audio::Mixer *OSystem_Wii::getMixer() {
assert(_mixer);
return _mixer;
Expand Down
2 changes: 0 additions & 2 deletions backends/platform/wii/osystem.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,8 +206,6 @@ class OSystem_Wii final : public EventsBaseBackend, public PaletteManager {

virtual void quit() override;

virtual void setWindowCaption(const char *caption) override;

virtual Audio::Mixer *getMixer() override;
virtual FilesystemFactory *getFilesystemFactory() override;
virtual void getTimeAndDate(TimeDate &t) const override;
Expand Down
4 changes: 2 additions & 2 deletions base/main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ static Common::Error runGame(const Plugin *plugin, OSystem &system, const Common
if (caption.empty())
caption = target;
if (!caption.empty()) {
system.setWindowCaption(caption.c_str());
system.setWindowCaption(caption.decode());
}

//
Expand Down Expand Up @@ -360,7 +360,7 @@ static void setupGraphics(OSystem &system) {
GUI::GuiManager::instance();

// Set initial window caption
system.setWindowCaption(gScummVMFullVersion);
system.setWindowCaption(Common::U32String(gScummVMFullVersion));

// Clear the main screen
system.fillScreen(0);
Expand Down
8 changes: 2 additions & 6 deletions common/system.h
Original file line number Diff line number Diff line change
Expand Up @@ -1467,13 +1467,9 @@ class OSystem : Common::NonCopyable {
* Set a window caption or any other comparable status display to the
* given value.
*
* The caption must be a pure ISO LATIN 1 string. Passing a string
* with a different encoding may lead to unexpected behavior,
* even crashes.
*
* @param caption The window caption to use, as an ISO LATIN 1 string.
* @param caption The window caption to use.
*/
virtual void setWindowCaption(const char *caption) {}
virtual void setWindowCaption(const Common::U32String &caption) {}

/**
* Display a message in an 'on-screen display'.
Expand Down
2 changes: 1 addition & 1 deletion engines/engine.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ void GUIErrorMessage(const Common::String &msg, const char *url) {
}

void GUIErrorMessage(const Common::U32String &msg, const char *url) {
g_system->setWindowCaption("Error");
g_system->setWindowCaption(_("Error"));
g_system->beginGFXTransaction();
initCommonGFX();
g_system->initSize(320, 200);
Expand Down
2 changes: 1 addition & 1 deletion engines/icb/surface_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ unsigned int _surface_manager::Init_direct_draw() {
// Debug info
Zdebug("*SURFACE_MANAGER* Initalizing the SDL video interface");

g_system->setWindowCaption("In Cold Blood (C)2000 Revolution Software Ltd");
g_system->setWindowCaption(Common::U32String("In Cold Blood (C)2000 Revolution Software Ltd"));
initGraphics(640, 480, nullptr);

_zb = new TinyGL::FrameBuffer(640, 480, g_system->getScreenFormat()); // TODO: delete
Expand Down
12 changes: 3 additions & 9 deletions engines/scumm/he/script_v100he.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1721,18 +1721,12 @@ void ScummEngine_v100he::o100_setSystemMessage() {
switch (subOp) {
case 80: // Set Window Caption
// TODO: The 'name' string can contain non-ASCII data. This can lead to
// problems, because (a) the encoding used for "name" is not clear,
// (b) OSystem::setWindowCaption only supports ASCII. As a result, odd
// behavior can occur, from strange wrong titles, up to crashes (happens
// under Mac OS X).
// problems, because the encoding used for "name" is not clear,
//
// Possible fixes/workarounds:
// - Simply stop using this. It's a rather unimportant "feature" anyway.
// - Try to translate the text to ASCII.
// - Refine OSystem to accept window captions that are non-ASCII, e.g.
// by enhancing all backends to deal with UTF-8 data. Of course, then
// one still would have to convert 'name' to the correct encoding.
//_system->setWindowCaption((const char *)name);
// - Try to translate the text to UTF-32.
//_system->setWindowCaption(Common::U32String((const char *)name));
break;
case 131: // Set Version
debug(1,"o100_setSystemMessage: (%d) %s", subOp, name);
Expand Down
12 changes: 3 additions & 9 deletions engines/scumm/he/script_v70he.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -490,18 +490,12 @@ void ScummEngine_v70he::o70_setSystemMessage() {
break;
case 243: // Set Window Caption
// TODO: The 'name' string can contain non-ASCII data. This can lead to
// problems, because (a) the encoding used for "name" is not clear,
// (b) OSystem::setWindowCaption only supports ASCII. As a result, odd
// behavior can occur, from strange wrong titles, up to crashes (happens
// under Mac OS X).
// problems, because the encoding used for "name" is not clear.
//
// Possible fixes/workarounds:
// - Simply stop using this. It's a rather unimportant "feature" anyway.
// - Try to translate the text to ASCII.
// - Refine OSystem to accept window captions that are non-ASCII, e.g.
// by enhancing all backends to deal with UTF-8 data. Of course, then
// one still would have to convert 'name' to the correct encoding.
//_system->setWindowCaption((const char *)name);
// - Try to translate the text to UTF-32.
//_system->setWindowCaption(Common::U32String((const char *)name));
break;
default:
error("o70_setSystemMessage: default case %d", subOp);
Expand Down
12 changes: 3 additions & 9 deletions engines/scumm/he/script_v72he.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1993,18 +1993,12 @@ void ScummEngine_v72he::o72_setSystemMessage() {
break;
case 243: // Set Window Caption
// TODO: The 'name' string can contain non-ASCII data. This can lead to
// problems, because (a) the encoding used for "name" is not clear,
// (b) OSystem::setWindowCaption only supports ASCII. As a result, odd
// behavior can occur, from strange wrong titles, up to crashes (happens
// under Mac OS X).
// problems, because the encoding used for "name" is not clear.
//
// Possible fixes/workarounds:
// - Simply stop using this. It's a rather unimportant "feature" anyway.
// - Try to translate the text to ASCII.
// - Refine OSystem to accept window captions that are non-ASCII, e.g.
// by enhancing all backends to deal with UTF-8 data. Of course, then
// one still would have to convert 'name' to the correct encoding.
//_system->setWindowCaption((const char *)name);
// - Try to translate the text to UTF-32.
//_system->setWindowCaption(Common::U32String((const char *)name));
break;
default:
error("o72_setSystemMessage: default case %d", subOp);
Expand Down
2 changes: 1 addition & 1 deletion engines/sword2/maketext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -674,7 +674,7 @@ void Sword2Engine::initializeFontResourceFlags() {
else
textLine = (char *)fetchTextLine(textFile, 54) + 2;

_system->setWindowCaption(textLine);
_system->setWindowCaption(Common::U32String(textLine));
Copy link
Member

Choose a reason for hiding this comment

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

I am wondering if this is correct there. Do we know what encoding might be used in the resource? Assuming ASCII may not be right and maybe we need proper conversion?

By the way here is the title I get for the French version: Les Boucliers de Quetzalcoatl
So all the titles listed in the code, as well as this one, only contain ASCII characters. So maybe it does not matter.

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I can tell, setWindowCaption was specified to use ASCII in 188cdf9, and was changed to use ISO-8859-1 in bb28ed7, while the relevant code in the Sword2 engine predates both of those commits. As such, I would assume that the caption used here is always ASCII. Having said that though, it might be worth removing the use of setWindowCaption both here and in the icb engine, since those are the only two engines to use a custom window caption, and there doesn't seem to be much of an advantage to this compared to using the standard caption.

Copy link
Member

@criezy criezy Nov 10, 2020

Choose a reason for hiding this comment

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

Yes, it is probably fine to assume ASCII here since that is what was initially assumed.
Or indeed we could just remove this code and use the standard caption.

_resman->closeResource(TEXT_RES);
}

Expand Down