f2e-spec
f2e-spec copied to clipboard
alibaba
Some fixes and refactors for the cache
What does this PR do?
Closes Issue(s)
Motivation
More
- [ ] Added/updated documentation
Additional Notes
I don't see why all of these is required:
- Json should be a header only library, why do we need to add the include of the libraries in CMake
- There is a lot of indent change that are not related to this PR
- I am not a bug fan of pulluting the fs library with json related code (I was already reluctant to have it in a core structure such as Songs) (it also complexify the CMake file),
- please don't use contains instead of count since it was carefully chosen in order to lower the dependency version,
- also this patch since to activate the cache reading in all Performous versions whereas it was only activated when compiling using webserver feature (this was carefully chosen because it seemed that the cache loading still had problems).
I don't see why all of these is required:
- Json should be a header only library, why do we need to add the include of the libraries in CMake
That's a fair point, and it might be unnecessary.
- There is a lot of indent change that are not related to this PR
Yes. It just seemed awkward to make a PR to correct the indent of code that hasn't been touched in ages, when I'm changing something literally next to it.
- I am not a bug fan of pulluting the fs library with json related code (I was already reluctant to have it in a core structure such as Songs) (it also complexify the CMake file),
The reason I'm doing this is because my reworked webserver uses json elsewhere and thus we'd have to begin duplicating code.
I put it in fs because it seemed the most fitting.
Where else would you put it? Its own file?
- please don't use contains instead of count since it was carefully chosen in order to lower the dependency version,
And I tried at first, but truly what's the point? What system/configuration are you trying to support out of the box here? Ubuntu 18.04? That has nlohmann 2, the last version of which was released five years ago. Nlohmann 3 is faster, more robust and supports exceptions properly. If you can show me a single relevant case where using 3.6.0 instead of an earlier 3.x version will make it easier for anyone, I'll change it back to count.
- also this patch since to activate the cache reading in all Performous versions whereas it was only activated when compiling using webserver feature (this was carefully chosen because it seemed that the cache loading still had problems).
Does it? @Baklap4 correct me if I'm wrong but I think the only actual reason the cache depended on webserver is because they were both supplied by CppRest (which came with several other dependencies as well)
Download the artifacts for this pull request:
- Performous-1.2.0-721-ff9bcff-alpha.AppImage.zip
- Performous-1.2.0-721-ff9bcff-alpha.dmg.zip
- Performous-1.2.0-721-ff9bcff-alpha.exe.zip
- Performous-1.2.0-721-ff9bcff-alpha-debian_10.deb.zip
- Performous-1.2.0-721-ff9bcff-alpha-debian_11.deb.zip
- Performous-1.2.0-721-ff9bcff-alpha-fedora_34.rpm.zip
- Performous-1.2.0-721-ff9bcff-alpha-fedora_35.rpm.zip
- Performous-1.2.0-721-ff9bcff-alpha-fedora_36.rpm.zip
- Performous-1.2.0-721-ff9bcff-alpha-mingw-w64.exe.zip
- Performous-1.2.0-721-ff9bcff-alpha-ubuntu_18.04.deb.zip
- Performous-1.2.0-721-ff9bcff-alpha-ubuntu_20.04.deb.zip
- Performous-1.2.0-721-ff9bcff-alpha-ubuntu_22.04.deb.zip
This service is provided by nightly.link. These artifacts will expire in 90 days and will not be available for download after that time.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Does it? @Baklap4 correct me if I'm wrong but I think the only actual reason the cache depended on webserver is because they were both supplied by CppRest (which came with several other dependencies as well)
@Lord-Kamina They were both supplied by CppRest so that's why webserver support 'depended' on json. Now with nlohmann-json it can be added as an option or become part of the core system. If it becomes part of the core system the lib should be required, else can be optional.
I removed the change to reload_internal entirely.
I still left three commits, with the removal of the #IFDEF WEBSERVER bit on its own commit.
Played a few songs with vocals + guitar using the AppImage and packages on Ubuntu 18.04 & 20.04. Didn't notice any effects on gameplay, which is to be expected I believe. If anything, the cached songs are loaded faster, and load in a few seconds rather than the 30 seconds + that it used to take.
