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 config/font.json generation to append correct paths for Android #72923

Closed
wants to merge 2 commits into from

Conversation

oosyrag
Copy link
Contributor

@oosyrag oosyrag commented Apr 9, 2024

Summary

Bugfixes "Android: Added correct path for default Terminus font"

Purpose of change

Fixes #72922

This is a really old Android bug, as was noted in https://www.reddit.com/r/cataclysmdda/comments/ppwq9f/cataclysm_dda_on_android_an_indepth_tweaking_guide/, nearly 3 years ago. This will let Android users see/use the bundled Terminus font by default, without having to jump through hoops replacing or editing files manually. Showing Terminus as the default makes a world of difference in how nice the game appears compared to unifont (see screenshots in attached issue and reddit post).

Describe the solution

Copied the function that adds the correct path to unifont (which works properly), adjusted for Terminus.

Describe alternatives you've considered

This solution would force terminus to be added even if it was removed (to back of the list, but in front of unifont). Perhaps a more elegant solution would be to craft a function that just validates and fixes an invalid path for any entry in the list only if the entry exists to begin with, while leaving the ensure_unifont_loaded function be the one that specifically adds unifont to the back if it doesn't exist.

(I don't know how to do this off the top of my head but pretty confident I could figure it out. Eventually.)

Testing

None. I have no idea how to compile for Android. Help please and thanks.

Edit: compiled and tested on Windows, nothing looks out of the ordinary, the functions don't appear to change anything, as expected.
fonds.json result:

{
  "typeface": [ "data/font/Terminus.ttf", "data/font/unifont.ttf" ],
  "map_typeface": [ "data/font/Terminus.ttf", "data/font/unifont.ttf" ],
  "overmap_typeface": [ "data/font/Terminus.ttf", "data/font/unifont.ttf" ]
}

Additional context

Review appreciated, I have no c++ background and also don't know if this is the proper way to solve the issue. And as mentioned above not sure how to compile to test...

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Apr 9, 2024
@oosyrag oosyrag marked this pull request as draft April 11, 2024 15:56
@oosyrag
Copy link
Contributor Author

oosyrag commented Apr 11, 2024

Draft, tested on Android and didn't work.
Or I compiled wrong...

@oosyrag
Copy link
Contributor Author

oosyrag commented Apr 11, 2024

This approach did not work. While the outputted fonts.json ended up as expected, Terminus still doesn't get loaded unless the entries with incorrect paths ahead of it are deleted or the correct path is moved to the front of the list.

{
  "typeface": [ "data/font/Terminus.ttf", "data/font/unifont.ttf", "/storage/emulated/0/Android/data/com.cleverraven.cataclysmdda.experimental/files/data/font/Terminus.ttf", "/storage/emulated/0/Android/data/com.cleverraven.cataclysmdda.experimental/files/data/font/unifont.ttf" ],
  "map_typeface": [ "data/font/Terminus.ttf", "data/font/unifont.ttf", "/storage/emulated/0/Android/data/com.cleverraven.cataclysmdda.experimental/files/data/font/Terminus.ttf", "/storage/emulated/0/Android/data/com.cleverraven.cataclysmdda.experimental/files/data/font/unifont.ttf" ],
  "overmap_typeface": [ "data/font/Terminus.ttf", "data/font/unifont.ttf", "/storage/emulated/0/Android/data/com.cleverraven.cataclysmdda.experimental/files/data/font/Terminus.ttf", "/storage/emulated/0/Android/data/com.cleverraven.cataclysmdda.experimental/files/data/font/unifont.ttf" ]
}

The better approach is probably to not "hardcode" the paths in json, and write them to the fonts.json file by prepending the PATH_INFO::fontdir() instead, and having only the font name in fontdata.json. But I'm pretty sure it was changed FROM that a few years ago... time to find out why.

@oosyrag oosyrag closed this Apr 11, 2024
@oosyrag oosyrag deleted the Android-Terminus-font-fix branch April 11, 2024 22:32
@ZhilkinSerg
Copy link
Contributor

IMO font selection should be reworked to allow specifying font names explicitly (e.g. in Android launch menu and optionally duplicated in game settings).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android requires full path to terminus added to generated config/fonts.json
2 participants