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

GUI: Small Improvements #2489

Merged
merged 2 commits into from Oct 1, 2020
Merged

GUI: Small Improvements #2489

merged 2 commits into from Oct 1, 2020

Conversation

@aryanrawlani28
Copy link
Contributor

@aryanrawlani28 aryanrawlani28 commented Sep 30, 2020

  • Remove redundant constructor/function since shifted to C++11 from the U32 code
  • Hindi language proper name
  • improper GUI fixed when changing to Hebrew in the classic theme. (see pictures below.)

Before

before

After

after

@criezy
Copy link
Member

@criezy criezy commented Sep 30, 2020

Is there a clear benefit in using delegating constructors in this case?

While c++11 is now enabled by default, there is still an option to disable it in configure, and several of our ports do not yet use a compiler recent enough to use c++11. Until the new build server with updated toolchains is deployed, I would prefer if we avoid breaking compilation on the current one, unless there is a very good reason for it.

Using c++11 in engines is fine I think (we can disable those engines on buildbots for ports with an old compiler), but I think we should avoid using c++11 in common codes for now (unless it is optional and can be toggled with a #ifdef USE_CPP11 such as move constructors).

@aryanrawlani28
Copy link
Contributor Author

@aryanrawlani28 aryanrawlani28 commented Oct 1, 2020

While c++11 is now enabled by default, there is still an option to disable it in configure, and several of our ports do not yet use a compiler recent enough to use c++11. Until the new build server with updated toolchains is deployed, I would prefer if we avoid breaking compilation on the current one, unless there is a very good reason for it.

Ah, sorry! I guess I didn't think about it from that point of view. You are correct then, it doesn't make a good case to do this - it doesn't really do anything. I will drop that commit.

Thanks!

@aryanrawlani28 aryanrawlani28 force-pushed the aryanrawlani28:gui-patches branch from 2d3038e to 75b3d8f Oct 1, 2020
@sev-
Copy link
Member

@sev- sev- commented Oct 1, 2020

Thanks!

@sev- sev- merged commit 89d07e7 into scummvm:master Oct 1, 2020
5 checks passed
5 checks passed
Windows (win32, x86-windows, x86, --enable-faad --enable-mpeg2 --enable-discord --disable-fribidi...
Details
Windows (x64, x64, x64-windows, --enable-faad --enable-mpeg2 --enable-discord --disable-fribidi, ...
Details
Windows (arm64, arm64, arm64-windows, --enable-faad --enable-mpeg2 --enable-discord --disable-fri...
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deepcode-ci-bot Well done, no issues found!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.