BeamMP-Server icon indicating copy to clipboard operation
BeamMP-Server copied to clipboard

158 bug running settings help returns nothing

Open jimkoen opened this issue 1 year ago • 5 comments

Fix #158

jimkoen avatar Feb 26 '24 08:02 jimkoen

todo:

  • support concurrent access to config
  • cleanup in multiple places
  • change settings access to new settings.get... everywhere

jimkoen avatar Feb 26 '24 08:02 jimkoen

@jimkoen what's the status on this?

lionkor avatar Apr 20 '24 18:04 lionkor

@jimkoen what's the status on this?

I have a working version on my end where I need to fix 1-2 remaining build errors related to formatting of console output.

However as discussed privately I'm held up by exams, so this won't land before the week of the 29th of April. If I'm holding something up, please tell me, I might be able to reschedule, but otherwise it would be very considerate if you could respect my exam schedule.

jimkoen avatar Apr 20 '24 18:04 jimkoen

Thank you for the info

lionkor avatar Apr 23 '24 08:04 lionkor

@lionkor Should be ready for review.

Unfortunately I cannot reproduce the test failures on Ubuntu 20.04 on my end. Tests work fine on FreeBSD (and other OS's as indicated by the Actions result). For now I think it'd make sense to start the review, so the issue doesn't become any more stale than it already is.

jimkoen avatar May 15 '24 12:05 jimkoen

I implemented the changes. Currently I'm looking into a way to test Ubuntu 20.04 locally, as gh actions has stopped working for this branch, despite the fact that I haven't made any changes to the actions scripts.

jimkoen avatar May 21 '24 18:05 jimkoen

run this in your branch:

docker run -it --rm -v .:/test ubuntu:20.04

and then just run all the scrips in ./scripts/ubuntu-20.04/ (you can skip the 3-build script and only build the tests). I made this as simple as possible without having to write it twice.

lionkor avatar May 23 '24 14:05 lionkor

ready for review

jimkoen avatar Jun 26 '24 10:06 jimkoen

And please run clang-format

lionkor avatar Jun 26 '24 10:06 lionkor

A few things I noticed while testing:

  1. I get this when I run settings list: image

  2. When I enter an incomplete command I get a difficult to understand error: image

  3. AuthKey is gettable and settable: image

lionkor avatar Jun 26 '24 10:06 lionkor

Also, we need to do this:

diff --git a/src/THeartbeatThread.cpp b/src/THeartbeatThread.cpp
index a7f37c6..3e6699c 100644
--- a/src/THeartbeatThread.cpp
+++ b/src/THeartbeatThread.cpp
@@ -40,8 +40,8 @@ void THeartbeatThread::operator()() {
     static std::chrono::high_resolution_clock::time_point LastUpdateReminderTime = std::chrono::high_resolution_clock::now();
     bool isAuth = false;
     std::chrono::high_resolution_clock::duration UpdateReminderTimePassed;
-    auto UpdateReminderTimeout = ChronoWrapper::TimeFromStringWithLiteral(Application::Settings.getAsString(Settings::Key::Misc_UpdateReminderTime));
     while (!Application::IsShuttingDown()) {
+        auto UpdateReminderTimeout = ChronoWrapper::TimeFromStringWithLiteral(Application::Settings.getAsString(Settings::Key::Misc_UpdateReminderTime));
         Body = GenerateCall();
         // a hot-change occurs when a setting has changed, to update the backend of that change.
         auto Now = std::chrono::high_resolution_clock::now();

We need to do this because UpdateReminderTime is (correctly) read+write, but it's only ever read once. That's broken :)

lionkor avatar Jun 26 '24 10:06 lionkor

Also remove line ChronoWrapper.cpp:13 so it doesn't spam ;)

lionkor avatar Jun 26 '24 10:06 lionkor

The error message when accessing an unknown key or category sucks, can we get that to say something like "that's not a setting ya doofus"

lionkor avatar Jun 26 '24 11:06 lionkor

Remove ubuntu 20.04 support due to their outdated ahh compiler as well please

lionkor avatar Jun 26 '24 11:06 lionkor