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

Adding DIMCustomSymbols #441

Merged
merged 11 commits into from
Jun 13, 2024

Conversation

Gix3612
Copy link
Contributor

@Gix3612 Gix3612 commented Feb 25, 2023

Coincides with Support for DIMCustomSymbols #9116

Added a new symbol font to extend those available from Destiny_Keys.

Font generates its own separate set of enums with 'yarn generate:custom-glyph-enums'. Sorry if I changed something again that isn't meant to be manually edited.

Added all of the missing abilities to the symbol picker, as well as a handful of activity and other useful ui symbols and organized everything so they will be easier to find.

I expanded the Tables list to support more API data and extended Objective functionality to include non-richtext entries so there will be less manualTranslations work.

Marked as draft until I have recreated the font file using a different Private Use Area or a SVG-to-font generator is created, so that users can't start using these before the code points change.

@Gix3612 Gix3612 marked this pull request as ready for review March 2, 2023 18:44
@Gix3612
Copy link
Contributor Author

Gix3612 commented Mar 2, 2023

Everything should now be functional enough to use. The custom font is using PUA-A instead of BMP (u+F0000 instead of u+F000). I converted and placed DIMSymbols.woff2 into the output folder manually since the font converter isn't handling the 6 digit code points properly. Once the converter issue is cleared up, this should generate and overwrite as normal so I thought this would be the simplest solution for now. New strand symbols are incorporated, both from Destiny Keys and custom ones for non-kill feed abilities, and added code for the color strand symbol (may need to confirm I did this part right).

As for generating the font from SVGs in the future, I made an extra effort to make sure all the symbol names match or won't conflict with justrealmilk/destiny-icons file names, but I don't think using an outside repository would be a good idea since not all of the images are suitable to use for icons as-is and because of how greatly changes there could affect things on our end.

@delphiactual
Copy link
Collaborator

delphiactual commented Mar 7, 2023

DIM-custom-font/SVGs/ugly/ also has duplicate pngs?

do we even need a copy of the svgs in the repo if they are inside the .otf?

@Gix3612
Copy link
Contributor Author

Gix3612 commented Mar 7, 2023

Added the missing files. I've been running yarn generate:custom-glyph-enums directly to generate them and forgot that yarn generate-data would fail if they didn't already exist.

DIM-custom-font/SVGs/ugly/ also has duplicate pngs?

do we even need a copy of the svgs in the repo if they are inside the .otf?

At the moment, no. However, robojumper has said previously that he would like to automate the generation of the font file in the future to make adding/updating glyphs easier, so I included them for that. As for the PNGs, the "ugly" SVGs are ones I made hastily with a image-to-SVG converter. Most of them are passable as low resolution font glyphs, but many of them didn't generate well with missing lines and rounded corners so I plan to go back through and fix them and I included the source PNG file I generated them from for anyone else who might want to contribute to that.

@delphiactual delphiactual requested a review from nev-r March 7, 2023 19:52
@bhollis
Copy link
Contributor

bhollis commented Mar 16, 2023

Seems pretty cool. I'm a bit worried about the impact to DIM's size, but maybe we can at least lazy-load the chunk for the symbol editor.

@Gix3612
Copy link
Contributor Author

Gix3612 commented Mar 31, 2023

Okay, I've made some headway with the font generator. There's still room for improvement, but it is working and there shouldn't be any problems with maintaining codepoints unless someone deliberately does something to mess it up. I ended up using itgalaxy/webfont (which I would appreciate if someone could explain how I can make sure the node_module is setup properly with my PR to work correctly), it uses Unicode in the file name to maintain the codepoints between generations so a file archive is a must but adding new glyphs is as simple as dropping it in the folder because it will automatically assign it the next unused Unicode and prepend it to the file name.

Some things to note about adding glyphs: because of how it resizes the images, the normalize option couldn't be used to generate the font, so any new SVGs need to be adjusted manually. The bounding box should be 960px tall (the glyph can extend some above and below the box) and should have ~120px of empty space on both sides of the glyph. Also, the file name can't contain any numbers that aren't part of the Unicode. This is due to the generator also converting the glyph name into a ligature for use in CSS files and I couldn't find a way to disable this so it ends up adding all of the letters in the name as codepoints and causes a An enum member cannot have a numeric name. ts(2452) error that interrupts the build.

As for the code itself, I think the module should be able to write directly to the desired font type, but I couldn't figure out how to make it work.

@robojumper
Copy link
Member

Okay, in general I like what I'm seeing. I think @delphiactual mentioned that he'd want the font file generation be handled in a separate repository, which is something worth thinking about -- I can take care of moving that stuff (after someone with the keys creates a new repo).

One thing I noticed testing this out is that rerunning the generation script causes changes to the font files, presumably because one of the exported converters this one is using (maybe the SVG -> TTF?) embeds the current timestamp and that obviously changes every run. d2-additional-info generally wants to ensure that all generation scripts have been run and the committed outputs match the inputs, and this makes it a bit tricky -- maybe we need to depend on the individual converters directly to override the timestamp.

@Gix3612
Copy link
Contributor Author

Gix3612 commented May 1, 2023

What files exactly are causing the timestamp problem? Is there something I can run on my end to get the same feedback you are seeing about file changes?

I have to assume it's DIM-Symbols.svg since writeFile('./DIM-custom-font/DIM-Symbols.svg', result.svg); is run with every execution and the other font files are being converted through @hayes0724/web-font-converter, which is the same one used in generate-font-glyph-enums.ts so any issues with them would have already been present with the DestinySymbols font. If so, is there a way to compare the result.svg buffer data against the old font for changes before overwriting it?

@bhollis
Copy link
Contributor

bhollis commented Sep 8, 2023

So to fix this we need to drop web-font-converter and just call svg2ttf and ttf2woff2 ourselves, but passing in the ts option.

@delphiactual
Copy link
Collaborator

what would be a good ts option to pass in?

@delphiactual
Copy link
Collaborator

@bhollis @robojumper I have updated this to keep its timestamps between runs and work with the updated workflows... please re-review

set ts to 0

remove temp files

cleanuper

add logging

pretty and options

.
@delphiactual
Copy link
Collaborator

https://github.com/DestinyItemManager/dim-custom-symbols has been provisioned for the font generation

@delphiactual delphiactual merged commit 36803f9 into DestinyItemManager:master Jun 13, 2024
6 checks passed
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.

4 participants