ericw-tools
ericw-tools copied to clipboard
Switch to Jsoncpp, set up vcpkg in CI
- replace nlohmann json (via git submodule) with jsoncpp (via vcpkg)
- As a result, bspinfo.cc build time reduced from 2.6s to 1.9s
- I only really did this switch out of concern for build times.
- Disadvantages: jsoncpp development is stalled. It's a bit flaky in places, e.g. I had to add this cast to get this line to compile on macOS:
model["numbrushes"] = static_cast<Json::UInt64>(src_model.brushes.size()); - I think moving away from header-only is the right move, but I'm not sure if changing worth it?
- Set up vcpkg in our GH Actions config, also install lua (our binaries previously lacked lua)
- Disadvantage, this adds another minute or so to CI
- We probably do want our
maputilrelease binaries to be fully functional, so if it needs lua, it needs lua..
- Drop appveyor
- It was considerably slower (12 min, vs 3-6min for GH Actions) and we don't need it anymore (all release binaries are built on GitHub actions)
- Setting up vcpkg in GH Actions was enough work that I don't feel like dealing with another CI system - this will keep being a maintenance burden every time we touch CI build settings
- As a negative, if we drop appveyor, getting access to our dev builds will require login to GitHub now
- We could keep appveyor as well..
- Fix an ODR violation reported by GCC
- It was specifically complaining about
struct texdef_valve_tandstruct map_entity_t - I'm not sure why this only came up now, maybe jsoncpp is leaking some warning flags, but it was a valid problem
- It was specifically complaining about
All these (except the ODR fix) are kind of subjective, what do you think @Paril ?
Alternatively we could just keep the ODR fix, vcpkg + lua, but keep nlohmann?
The ODR change is probably because of a thing I did recently including map.hh in the old tools. I still need to move map.cc over to the new map types.