refactor: the Sonar remarks on the small fixes taken care of

- the log stops through a flag kept in a function, not a global one
- the messages are put together with std::format
- a vehicle dropped on a trainset in the launcher goes in once the
  trainset is drawn, so the vehicles are gone through with a plain loop
  again; the drop split into a new vehicle and a moved one
- the scenario that can't be opened is caught as the runtime error it
  is, a file name outside of the code page as the system error it is
- a track of an unknown type is left out through the pointer owning it
- the keyword of a track type written out by a function of its own
- the picked node forgotten with ranges::replace_if, the empty
  Forget_Node of the base renderer told why it is empty
- FileCased takes the path it works on

application/application.cpp

@@ -696,8 +696,8 @@ void eu07_application::exit()
 	Global.applicationQuitOrder = true;
 	// each step of the closing in the log, to tell from it where a closing that hangs stops
 	auto const closing{std::chrono::steady_clock::now()};
-	auto const step = [&](char const *What) {
+	auto const step = [closing](char const *What) {
-		WriteLog(std::string{"Closing: "} + What + " at " + std::to_string(std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - closing).count()) + " ms");
+		WriteLog(std::format("Closing: {} at {} ms", What, std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() - closing).count()));
 	};
 	step("the modes");
 	for (auto &mode : m_modes)

application/scenarioloadermode.cpp

@@ -21,6 +21,8 @@ http://mozilla.org/MPL/2.0/.
 #include "utilities/translation.h"
 #include "audio/audiorenderer.h"
 
+#include <format>
+
 scenarioloader_mode::scenarioloader_mode() {
     m_userinterface = std::make_shared<scenarioloader_ui>();
 }
@@ -52,8 +54,9 @@ bool scenarioloader_mode::update() {
 		try {
 			state = simulation::State.deserialize_begin(Global.SceneryFile);
 		}
-		catch (std::exception const &e) {
+		// the file it can't read, as the parser and the scenery opening it report it
-			ErrorLog("Bad init: scenario \"" + Global.SceneryFile + "\" couldn't be opened (" + e.what() + ")");
+		catch (std::runtime_error const &e) {
+			ErrorLog(std::format("Bad init: scenario \"{}\" couldn't be opened ({})", Global.SceneryFile, e.what()));
 			state.reset();
 			Global.SceneryFile.clear();
 			Application.pop_mode();

launcher/scenery_list.cpp

@@ -8,6 +8,28 @@
 #include "utilities/translation.h"
 
 #include <filesystem>
+#include <format>
+
+namespace
+{
+
+// the skin of the bank of vehicles as it's shared, the one a skin dragged out of the bank points at; none for one gone since
+std::shared_ptr<ui::skin_set> shared_skin(ui::skin_set const *Skin)
+{
+	auto const vehicle{Skin->vehicle.lock()};
+	if (vehicle == nullptr)
+		return {};
+	auto const found{std::ranges::find_if(vehicle->matching_skinsets, [Skin](std::shared_ptr<ui::skin_set> const &Set) { return Set.get() == Skin; })};
+	return found != vehicle->matching_skinsets.end() ? *found : nullptr;
+}
+
+// the name of a vehicle put in a trainset, with a number of its own
+std::string numbered(std::string const &Name)
+{
+	return std::format("{}_{}", Name, static_cast<int>(LocalRandom(0.0, 10000000.0)));
+}
+
+} // namespace
 
 ui::scenerylist_panel::scenerylist_panel(scenery_scanner &scanner)
     : ui_panel(STR("Scenario list"), false), scanner(scanner), placeholder_mini("textures/mini/other")
@@ -283,8 +305,8 @@ void ui::scenerylist_panel::draw_trainset(trainset_desc &trainset)
 	draw_droptarget(trainset, position++);
 	ImGui::SameLine(15.0f);
 
-	for (std::size_t index = 0; index < trainset.vehicles.size(); ++index) {
+	// a vehicle dropped between them goes in once they're all drawn, so they're gone through unchanged
-		auto &dyn_desc = trainset.vehicles[index];
+	for (auto &dyn_desc : trainset.vehicles) {
 		deferred_image *mini = nullptr;
 
 		if (dyn_desc.skin && dyn_desc.skin->mini.get() != deferred_image::invalid_id)
@@ -333,6 +355,10 @@ void ui::scenerylist_panel::draw_trainset(trainset_desc &trainset)
 
 		ImGui::PopID();
 	}
+	if (m_drop) {
+		m_drop();
+		m_drop = nullptr;
+	}
 
 	ImGui::NewLine();
 	ImGui::PopID();
@@ -341,44 +367,47 @@ void ui::scenerylist_panel::draw_trainset(trainset_desc &trainset)
 void ui::scenerylist_panel::draw_droptarget(trainset_desc &trainset, int position)
 {
 	ImGui::Dummy(ImVec2(15, 30));
-	if (ImGui::BeginDragDropTarget()) {
+	if (false == ImGui::BeginDragDropTarget())
-		const ImGuiPayload *payload = ImGui::AcceptDragDropPayload("vehicle_pure");
+		return;
-		if (payload) {
+	if (auto const *payload{ImGui::AcceptDragDropPayload("vehicle_pure")})
-			skin_set *ptr = *reinterpret_cast<skin_set **>(payload->Data);
+		drop_new_vehicle(trainset, position, *static_cast<skin_set *const *>(payload->Data));
-			std::shared_ptr<skin_set> skin;
+	if (auto const *payload{ImGui::AcceptDragDropPayload("vehicle_set")})
-			if (auto const vehicle = ptr->vehicle.lock())
+		drop_moved_vehicle(trainset, position, *static_cast<vehicle_moved const *>(payload->Data));
-				for (auto &s : vehicle->matching_skinsets)
+	ImGui::EndDragDropTarget();
-					if (s.get() == ptr)
+}
-						skin = s;
 
-			if (skin) {
+// a vehicle of the bank dropped on a gap of a trainset, put in there as a new one of it
-				trainset.vehicles.emplace(trainset.vehicles.begin() + position);
+void ui::scenerylist_panel::drop_new_vehicle(trainset_desc &trainset, int const position, skin_set const *Skin)
-				dynamic_desc &desc = trainset.vehicles[position];
+{
+	auto skin{shared_skin(Skin)};
+	if (skin == nullptr)
+		return;
+	m_drop = [&trainset, position, skin] {
+		auto &desc{*trainset.vehicles.emplace(trainset.vehicles.begin() + position)};
+		desc.name = numbered(skin->skin);
+		desc.vehicle = skin->vehicle.lock();
+		desc.skin = skin;
+	};
+}
 
-				desc.name = skin->skin + "_" + std::to_string((int)LocalRandom(0.0, 10000000.0));
+// a vehicle of a trainset dropped on a gap of it or of another one: moved there within it, from another one copied in with a
-				desc.vehicle = skin->vehicle.lock();
+// name of its own
-				desc.skin = skin;
+void ui::scenerylist_panel::drop_moved_vehicle(trainset_desc &trainset, int const position, vehicle_moved const &Moved)
-			}
+{
+	auto copy{Moved.dynamic};
+	auto &source{Moved.source};
+	auto const from{Moved.position};
+	if (&trainset != &source)
+		copy.name = numbered(copy.skin ? copy.skin->skin : copy.name);
+	m_drop = [&trainset, &source, from, position, copy] {
+		auto offset{0};
+		if (&trainset == &source) {
+			trainset.vehicles.erase(trainset.vehicles.begin() + from);
+			if (from < position)
+				offset = -1;
 		}
-
+		trainset.vehicles.insert(trainset.vehicles.begin() + position + offset, copy);
-		payload = ImGui::AcceptDragDropPayload("vehicle_set");
+	};
-		if (payload) {
-			vehicle_moved *moved = reinterpret_cast<vehicle_moved*>(payload->Data);
-
-			dynamic_desc desc_copy = moved->dynamic;
-			int offset = 0;
-			if (&trainset == &moved->source) {
-				trainset.vehicles.erase(trainset.vehicles.begin() + moved->position);
-				if (moved->position < position)
-					offset = -1;
-			} else {
-				desc_copy.name = (desc_copy.skin ? desc_copy.skin->skin : desc_copy.name) + "_" + std::to_string((int)LocalRandom(0.0, 10000000.0));
-			}
-
-			trainset.vehicles.insert(trainset.vehicles.begin() + position + offset, desc_copy);
-		}
-		ImGui::EndDragDropTarget();
-	}
 }
 
 void ui::scenerylist_panel::purge_selected_trainset()

launcher/scenery_list.h

@@ -54,6 +54,8 @@ private:
 	scenery_desc *selected_scenery = nullptr;
 	trainset_desc *selected_trainset = nullptr;
 	deferred_image placeholder_mini;
+	// a vehicle dropped on a gap between the vehicles of a trainset: put in there once they're all drawn, not while they are
+	std::function<void()> m_drop;
 
 	void draw_scenery_list();
 	void draw_scenery_info();
@@ -67,6 +69,8 @@ private:
 	void open_link(const std::string &link);
 	void draw_trainset(trainset_desc &trainset);
 	void draw_droptarget(trainset_desc &trainset, int position);
+	void drop_new_vehicle(trainset_desc &trainset, int position, skin_set const *Skin);
+	void drop_moved_vehicle(trainset_desc &trainset, int position, vehicle_moved const &Moved);
 	void purge_selected_trainset();
 };
 } // namespace ui

launcher/scenery_scanner.cpp

@@ -2,6 +2,8 @@
 #include "scenery_scanner.h"
 #include "utilities/Logs.h"
 
+#include <system_error>
+
 scenery_scanner::scenery_scanner(ui::vehicles_bank &bank)
     : bank(bank)
 {
@@ -17,7 +19,8 @@ void scenery_scanner::scan()
 		try {
 			name = path.string();
 		}
-		catch (std::exception const &) {
+		catch (std::system_error const &) {
+			// a name outside of the code page, which the path can't give as a string on Windows
 			continue;
 		}
 

rendering/opengl33renderer.cpp

@@ -5414,7 +5414,7 @@ void opengl33_renderer::Forget_Node(scene::basic_node const *Node)
 {
 	if (m_picksceneryitem == Node)
 		m_picksceneryitem = nullptr;
-	std::replace_if(m_picksceneryitems.begin(), m_picksceneryitems.end(), [Node](scene::basic_node const *Item) { return Item == Node; }, nullptr);
+	std::ranges::replace_if(m_picksceneryitems, [Node](scene::basic_node const *Item) { return Item == Node; }, nullptr);
 }
 
 glm::dvec3 opengl33_renderer::Update_Mouse_Position()

rendering/openglrenderer.cpp

@@ -1894,7 +1894,7 @@ opengl_renderer::Forget_Node( scene::basic_node const *Node ) {
     if( m_picksceneryitem == Node ) {
         m_picksceneryitem = nullptr;
     }
-    std::replace_if( m_picksceneryitems.begin(), m_picksceneryitems.end(), [Node]( scene::basic_node const *Item ) { return Item == Node; }, nullptr );
+    std::ranges::replace_if( m_picksceneryitems, [Node]( scene::basic_node const *Item ) { return Item == Node; }, nullptr );
 }
 
 void

rendering/renderer.h

@@ -91,7 +91,9 @@ public:
     virtual auto Pick_Control() const -> TSubModel const * = 0;
     virtual auto Pick_Node() const -> scene::basic_node const * = 0;
     // a node deleted from the scene is no longer given as picked
-    virtual void Forget_Node( scene::basic_node const *Node ) {}
+    virtual void Forget_Node( scene::basic_node const * /*Node*/ ) {
+        // a renderer which keeps no node picked has none to forget
+    }
 
     virtual auto Mouse_Position() const -> glm::dvec3 = 0;
     virtual auto Mouse_Hit() const -> bool { return true; }

simulation/simulationstateserializer.cpp

@@ -36,6 +36,7 @@ http://mozilla.org/MPL/2.0/.
 #include "utilities/utilities.h"
 #include "editor/editorTerrainStreamer.hpp"
 
+#include <format>
 
 namespace simulation {
 
@@ -542,13 +543,14 @@ state_serializer::deserialize_node( cParser &Input, scene::scratch_data &Scratch
     }
     else if( nodedata.type == "track" ) {
 
-        auto *path { deserialize_path( Input, Scratchpad, nodedata ) };
+        std::unique_ptr<TTrack> made { deserialize_path( Input, Scratchpad, nodedata ) };
-        if( path->eType == tt_Unknown ) {
+        if( made->eType == tt_Unknown ) {
-            // with no path made it can't be joined nor drawn, and nothing could run on it
+            // with no path made it can't be joined nor drawn, and nothing could run on it: it goes with its holder
-            ErrorLog( "Bad scenario: track \"" + path->name() + "\" of an unknown type defined in file \"" + Input.Name() + "\" (line " + std::to_string( inputline ) + ") left out" );
+            ErrorLog( std::format( "Bad scenario: track \"{}\" of an unknown type defined in file \"{}\" (line {}) left out", made->name(), Input.Name(), inputline ) );
-            delete path;
             return;
         }
+        // from here on the scene owns it
+        auto *path { made.release() };
         // duplicates of named tracks are currently experimentally allowed
         if( false == simulation::Paths.insert( path ) ) {
             ErrorLog( "Bad scenario: duplicate track name \"" + path->name() + "\" defined in file \"" + Input.Name() + "\" (line " + std::to_string( inputline ) + ")" );

utilities/Logs.cpp

@@ -73,12 +73,19 @@ std::deque<std::string> ErrorStack;
 
 // lock for log stacks
 std::mutex logMutex;
-// the log is written through the closing of the simulator, to the end
+namespace
-std::atomic<bool> logStop{false};
+{
+// the log is written through the closing of the simulator, to the end: set once the closing has nothing more to tell
+std::atomic<bool> &log_stopped()
+{
+	static std::atomic<bool> stopped{false};
+	return stopped;
+}
+} // namespace
 
 void StopLogService()
 {
-	logStop = true;
+	log_stopped() = true;
 }
 
 
@@ -87,7 +94,7 @@ void LogService()
 	// one more pass after the stop, so what came just before it gets written too
 	for (bool last{false}; false == last;)
 	{
-		last = logStop;
+		last = log_stopped();
 		{
 			// --- Obsługa InfoStack ---
 			while (!InfoStack.empty())

utilities/utilities.cpp

@@ -471,23 +471,22 @@ bool FileExists(std::string const &Filename)
 	return std::filesystem::exists(Filename);
 }
 
-std::string FileCased(std::string const &Filename)
+std::string FileCased(std::filesystem::path const &Filename)
 {
 	std::error_code error;
 	if (std::filesystem::exists(Filename, error))
-		return Filename;
+		return Filename.string();
 #ifdef _WIN32
 	// the names there are found in any case already: no folder to go through for every file missing, nor names outside of the
 	// code page to throw on
 	return {};
 #else
 	// only the name of the file itself, a folder named in another case isn't found
-	std::filesystem::path const path{Filename};
+	auto const directory{Filename.has_parent_path() ? Filename.parent_path() : std::filesystem::path{"."}};
-	auto const directory{path.has_parent_path() ? path.parent_path() : std::filesystem::path{"."}};
+	auto const wanted{ToLower(Filename.filename().string())};
-	auto const wanted{ToLower(path.filename().string())};
 	for (std::filesystem::directory_iterator entry{directory, error}, end; entry != end; entry.increment(error))
 		if (ToLower(entry->path().filename().string()) == wanted)
-			return path.has_parent_path() ? (directory / entry->path().filename()).string() : entry->path().filename().string();
+			return Filename.has_parent_path() ? (directory / entry->path().filename()).string() : entry->path().filename().string();
 	return {};
 #endif
 }

utilities/utilities.h

@@ -219,7 +219,7 @@ template <> bool extract_value(bool &Variable, std::string const &Key, std::stri
 bool FileExists(std::string const &Filename);
 // the file as it's named on the disk when only the case of its name differs, as in the sceneries made on Windows; empty when
 // there's none. the folders have to be named in their case; on Windows, finding the names in any case itself, the file given or none
-std::string FileCased(std::string const &Filename);
+std::string FileCased(std::filesystem::path const &Filename);
 
 std::pair<std::string, std::string> FileExists(std::vector<std::string> const &Names, std::vector<std::string> const &Extensions);
 

world/Track.cpp

@@ -29,6 +29,8 @@ http://mozilla.org/MPL/2.0/.
 #include "utilities/Logs.h"
 #include "rendering/renderer.h"
 
+#include <format>
+
 // 101206 Ra: trapezoidalne drogi i tory
 // 110720 Ra: rozprucie zwrotnicy i odcinki izolowane
 
@@ -429,7 +431,7 @@ void TTrack::Load(cParser *parser, glm::dvec3 const &pOrigin)
     {
         // dopływ: no path of it was ever made, the simulator falling over it as it loaded. a river of its first path now, the
         // second one kept and written back
-        ErrorLog("Bad track: tributary \"" + m_name + "\" in file \"" + parser->Name() + "\" (line " + std::to_string(parser->Line()) + ") is drawn and run on as a river of its first path");
+        ErrorLog(std::format("Bad track: tributary \"{}\" in file \"{}\" (line {}) is drawn and run on as a river of its first path", m_name, parser->Name(), parser->Line()));
         eType = tt_Normal;
         iCategoryFlag = 4;
         m_tributary = true;
@@ -2319,25 +2321,37 @@ TTrack::deserialize_( std::istream &Input ) {
     // TODO: implement
 }
 
+// the keyword the track is written with, by its type and for a plain one by what runs on it
+char const *
+TTrack::type_keyword() const {
+
+    if( eType == tt_Normal && m_tributary ) {
+        return "tributary";
+    }
+    if( eType == tt_Normal ) {
+        switch( iCategoryFlag ) {
+            case 1: { return "normal"; }
+            case 2: { return "road"; }
+            case 4: { return "river"; }
+            default: { return "none"; }
+        }
+    }
+    switch( eType ) {
+        case tt_Switch: { return "switch"; }
+        case tt_Cross: { return "cross"; }
+        case tt_Table: { return "turn"; }
+        case tt_Tributary: { return "tributary"; }
+        default: { return "none"; }
+    }
+}
+
 // export() subclass details, sends basic content of the class in legacy (text) format to provided stream
 void
 TTrack::export_as_text_( std::ostream &Output ) const {
     // header
     Output << "track ";
     // type
-    Output << (
+    Output << type_keyword() << ' ';
-            eType == tt_Normal ? (
-                m_tributary ? "tributary" :
-                iCategoryFlag == 1 ? "normal" :
-                iCategoryFlag == 2 ? "road" :
-                iCategoryFlag == 4 ? "river" :
-                "none" ) :
-            eType == tt_Switch ? "switch" :
-            eType == tt_Cross ? "cross" :
-            eType == tt_Table ? "turn" :
-            eType == tt_Tributary ? "tributary" :
-            "none" )
-            << ' ';
     // basic attributes
     Output
         << Length() << ' '

world/Track.h

@@ -358,6 +358,8 @@ private:
     void deserialize_( std::istream &Input );
     // export() subclass details, sends basic content of the class in legacy (text) format to provided stream
     void export_as_text_( std::ostream &Output ) const;
+    // the keyword the track is written with, by its type and for a plain one by what runs on it
+    char const *type_keyword() const;
     // locates specified profile in the profile database, potentially loading it from a file
     static std::pair<std::string, int> fetch_track_rail_profile( std::string const &Profile );
     // loads content of specified file and converts it into a vertex array