BeamMP / BeamMP-Server

Server for the multiplayer mod BeamMP for BeamNG.drive
https://beammp.com
GNU Affero General Public License v3.0
116 stars 49 forks source link

Refactor config, add `settings` command #295

Closed jimkoen closed 6 days ago

jimkoen commented 4 months ago

Fix #158

jimkoen commented 4 months ago

todo:

lionkor commented 2 months ago

@jimkoen what's the status on this?

jimkoen commented 2 months ago

@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.

lionkor commented 2 months ago

Thank you for the info

jimkoen commented 1 month ago

@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 commented 1 month ago

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.

lionkor commented 1 month ago

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.

jimkoen commented 6 days ago

ready for review

lionkor commented 6 days ago

And please run clang-format

lionkor commented 6 days ago

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 commented 6 days ago

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 commented 6 days ago

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

lionkor commented 6 days ago

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 commented 6 days ago

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