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 missing icon #9772

Merged
merged 1 commit into from Jan 5, 2022
Merged

Fix missing icon #9772

merged 1 commit into from Jan 5, 2022

Conversation

matthijskooijman
Copy link
Contributor

@matthijskooijman matthijskooijman commented Dec 28, 2021

These are two fixes for small problems I noticed while preparing the Debian package

  • The openttd.32.bmp icon was no longer installed since the cmake conversion, but it is still referenced by the SDL driver.
  • There was a super-long line that lintian complains about. Apparently super-long lines are discouraged because they might hinder other lintian checks and are generally a sign of bad code. There were some more complaints (about lang files, the changelog, a data uri in shell.html and the 3rdparty glext.h), but it did not seem appropriate or possible to change those, so I ignored those warnings instead. Commit dropped, turned out the line was parsed and needs to stay one line.

Checklist for review

Some things are not automated, and forgotten often. This list is a reminder for the reviewers.

  • The bug fix is important enough to be backported? -> No
  • This PR touches english.txt or translations? Check the guidelines -> No
  • This PR affects the save game format? (label 'savegame upgrade') -> No
  • This PR affects the GS/AI API? (label 'needs review: Script API') -> No (just touches a comment in the API)
  • This PR affects the NewGRF API? (label 'needs review: NewGRF') -> No

This file is used to set the SDL application icon, but it was no longer
installed since the switch to CMake.
@matthijskooijman matthijskooijman changed the title Fix missing icon and wrap long line Fix missing icon Dec 28, 2021
@LordAro LordAro added the backport requested This PR should be backport to current release (RC / stable) label Dec 28, 2021
@matthijskooijman
Copy link
Contributor Author

Hm, there is a link failure in the CI, but I don't think my change could have caused that. Any known problems in that area?

@glx22
Copy link
Contributor

glx22 commented Dec 28, 2021

Yeah, not your fault.
MSYS2 ld for x86_64 targets seems broken, but only for debug builds (which are used as CI check).

@rubidium42
Copy link
Contributor

Technically the bmp is only needed when it's being built with SDL or SDL2, so it should probably also only be copied in those cases. I have no idea how to accomplish that with CMake though.
Conceptually the copying of the bmp probably belongs more in the cmake file of the parent folder, as it has little to do with basesets. On the other hand, opntitle.dat doesn't belong there either so I'm not that bothered with that.

Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

#shipit

@TrueBrain TrueBrain merged commit 5777649 into OpenTTD:master Jan 5, 2022
TrueBrain pushed a commit to TrueBrain/OpenTTD that referenced this pull request Apr 2, 2022
…#9772)

This file is used to set the SDL application icon, but it was no longer
installed since the switch to CMake.
TrueBrain pushed a commit to TrueBrain/OpenTTD that referenced this pull request Apr 2, 2022
…#9772)

This file is used to set the SDL application icon, but it was no longer
installed since the switch to CMake.
TrueBrain pushed a commit that referenced this pull request Apr 2, 2022
This file is used to set the SDL application icon, but it was no longer
installed since the switch to CMake.
@TrueBrain TrueBrain added backported This PR is backported to a current release (RC / stable) and removed backport requested This PR should be backport to current release (RC / stable) labels Apr 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backported This PR is backported to a current release (RC / stable)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants