Added Font_s dumper and loader#739
Conversation
Laupetin
left a comment
Laupetin
left a comment
There was a problem hiding this comment.
Hey there, thanks for the PR :)
the json structure looks fine, i just have a few comments regarding the writing and loading methods to make it fit to the rest of the codebase.
I think the asset would be a good fit for having the code templated for between the games (like e.g. the material asset) but i'm fine if you don't want to do this here as the templating format is a bit rough and odd 😅
| #include "AssetLoaderFontIW3.h" | ||
| #include "Game/IW3/IW3.h" |
There was a problem hiding this comment.
The project uses clang-format for formatting. You should be able to configure clang-format as formatter in most IDEs.
There was a problem hiding this comment.
okay i'm try add it formatter in visual studio, vs always used own standard 🤣
|
|
||
| AssetCreationResult CreateAsset(const std::string& assetName, AssetCreationContext& context) override | ||
| { | ||
| const auto file = m_search_path.Open(std::format("{}.json", assetName)); |
There was a problem hiding this comment.
What other assets do is having a "common" file in the "ObjCommon" component that returns the file name for an asset, e.g. MaterialCommon.h.
This makes it possible to have a single source of truth for the filename pattern that is shared between dumpers and loaders.
| font.pixelHeight = jRoot.value("pixelHeight", 0); | ||
| font.glyphCount = jRoot.value("glyphCount", 0); |
There was a problem hiding this comment.
This project uses the arbitrary type conversions feature of nlohmann json, e.g. see JsonLeaderboardDef.h for the types and JsonLoaderFontIconT6.cpp on them getting used.
Makes the loading code less verbose, prone to errors and makes the target format clearer in code imo.
Would be nice to not have this be an outlier in what style it uses😅
There was a problem hiding this comment.
I like your usage of ordered_json and i noticed that the extensions for arbitrary type conversions for nlohmann json i defined only support the normal json and not ordered_json. I got to fix that i guess 😅
There was a problem hiding this comment.
Did this here: #746 if you wanna use ordered_json still with arbitrary type conversions, feel free to rebase on main when its merged
| g.dx = static_cast<char>(jG.value("dx", 0)); | ||
| g.pixelWidth = static_cast<char>(jG.value("width", 0)); | ||
| g.pixelHeight = static_cast<char>(jG.value("height", 0)); |
There was a problem hiding this comment.
I made a mistake on the game types here, dx, width and height should be unsigned char (applies to all games).
| font.pixelHeight = jRoot.value("pixelHeight", 0); | ||
| font.glyphCount = jRoot.value("glyphCount", 0); | ||
|
|
||
| std::string fontName = jRoot.value("name", ""); |
There was a problem hiding this comment.
The name part of the font asset should be taken from the input arguments of the loader (and the "name" should not be part of the json).
Otherwise you could try to load a font asset with one name and get an entirely different name (potentially by accident because you renamed the file but not the value in it)
2 participants
This PRs adds feature to dump Font_s to json structure and load it for create resource. Related to #546
Supported games: