Apply more code review comments

.vscode/c_cpp_properties.json

@@ -64,7 +64,8 @@
             "defines": [
                 "_DEBUG",
                 "UNICODE",
-                "_UNICODE"
+                "_UNICODE",
+                "__ENABLE_SCRIPTING__"
             ],
             "intelliSenseMode": "msvc-x64",
             "browse": {

src/openrct2-ui/scripting/CustomWindow.cpp

@@ -31,7 +31,7 @@ using namespace OpenRCT2::Scripting;
 
 namespace OpenRCT2::Ui::Windows
 {
-    enum
+    enum CUSTOM_WINDOW_WIDX
     {
         WIDX_BACKGROUND,
         WIDX_TITLE,
@@ -298,7 +298,7 @@ namespace OpenRCT2::Ui::Windows
             windowFlags |= WF_RESIZABLE;
         }
 
-        rct_window* window;
+        rct_window* window{};
         if (desc.X && desc.Y)
         {
             window = window_create(
@@ -412,7 +412,7 @@ namespace OpenRCT2::Ui::Windows
                 const auto numItems = std::min<size_t>(items.size(), DROPDOWN_ITEMS_MAX_SIZE);
                 for (size_t i = 0; i < numItems; i++)
                 {
-                    gDropdownItemsFormat[i] = selectedIndex == (int32_t)i ? STR_OPTIONS_DROPDOWN_ITEM_SELECTED
+                    gDropdownItemsFormat[i] = selectedIndex == static_cast<int32_t>(i) ? STR_OPTIONS_DROPDOWN_ITEM_SELECTED
                                                                                        : STR_OPTIONS_DROPDOWN_ITEM;
                     auto sz = items[i].c_str();
                     std::memcpy(&gDropdownItemsArgs[i], &sz, sizeof(const char*));
@@ -455,7 +455,7 @@ namespace OpenRCT2::Ui::Windows
                     InvokeEventHandler(info.Owner, widgetDesc->OnChange, args);
 
                     auto& widget = w->widgets[widgetIndex - 1];
-                    widget.string = (utf8*)widgetDesc->Items[dropdownIndex].c_str();
+                    widget.string = const_cast<utf8*>(widgetDesc->Items[dropdownIndex].c_str());
 
                     widgetDesc->SelectedIndex = dropdownIndex;
                 }
@@ -568,7 +568,7 @@ namespace OpenRCT2::Ui::Windows
             else
             {
                 widget.type = WWT_BUTTON;
-                widget.string = (utf8*)desc.Text.c_str();
+                widget.string = const_cast<utf8*>(desc.Text.c_str());
                 widget.flags |= WIDGET_FLAGS::TEXT_IS_STRING;
             }
             widgetList.push_back(widget);
@@ -576,7 +576,7 @@ namespace OpenRCT2::Ui::Windows
         else if (desc.Type == "checkbox")
         {
             widget.type = WWT_CHECKBOX;
-            widget.string = (utf8*)desc.Text.c_str();
+            widget.string = const_cast<utf8*>(desc.Text.c_str());
             widget.flags |= WIDGET_FLAGS::TEXT_IS_STRING;
             if (desc.IsChecked)
             {
@@ -589,7 +589,7 @@ namespace OpenRCT2::Ui::Windows
             widget.type = WWT_DROPDOWN;
             if (desc.SelectedIndex >= 0 && (size_t)desc.SelectedIndex < desc.Items.size())
             {
-                widget.string = (utf8*)desc.Items[desc.SelectedIndex].c_str();
+                widget.string = const_cast<utf8*>(desc.Items[desc.SelectedIndex].c_str());
             }
             widget.flags |= WIDGET_FLAGS::TEXT_IS_STRING;
             widgetList.push_back(widget);
@@ -610,21 +610,21 @@ namespace OpenRCT2::Ui::Windows
         else if (desc.Type == "groupbox")
         {
             widget.type = WWT_GROUPBOX;
-            widget.string = (utf8*)desc.Text.c_str();
+            widget.string = const_cast<utf8*>(desc.Text.c_str());
             widget.flags |= WIDGET_FLAGS::TEXT_IS_STRING;
             widgetList.push_back(widget);
         }
         else if (desc.Type == "label")
         {
             widget.type = WWT_LABEL;
-            widget.string = (utf8*)desc.Text.c_str();
+            widget.string = const_cast<utf8*>(desc.Text.c_str());
             widget.flags |= WIDGET_FLAGS::TEXT_IS_STRING;
             widgetList.push_back(widget);
         }
         else if (desc.Type == "spinner")
         {
             widget.type = WWT_SPINNER;
-            widget.string = (utf8*)desc.Text.c_str();
+            widget.string = const_cast<utf8*>(desc.Text.c_str());
             widget.flags |= WIDGET_FLAGS::TEXT_IS_STRING;
             widgetList.push_back(widget);
 

src/openrct2/Game.cpp

@@ -597,7 +597,7 @@ void game_load_scripts()
 #endif
 }
 
-void game_finish()
+void game_unload_scripts()
 {
 #ifdef __ENABLE_SCRIPTING__
     GetContext()->GetScriptEngine().UnloadPlugins();
@@ -816,7 +816,7 @@ static void game_load_or_quit_no_save_prompt_callback(int32_t result, const utf8
 {
     if (result == MODAL_RESULT_OK)
     {
-        game_finish();
+        game_unload_scripts();
         window_close_by_class(WC_EDITOR_OBJECT_SELECTION);
         context_load_park_from_file(path);
         game_load_scripts();
@@ -860,12 +860,12 @@ void game_load_or_quit_no_save_prompt()
             }
             gGameSpeed = 1;
             gFirstTimeSaving = true;
-            game_finish();
+            game_unload_scripts();
             title_load();
             break;
         }
         default:
-            game_finish();
+            game_unload_scripts();
             openrct2_finish();
             break;
     }

src/openrct2/Game.h

@@ -158,7 +158,7 @@ void game_load_or_quit_no_save_prompt();
 void load_from_sv6(const char* path);
 void game_load_init();
 void game_load_scripts();
-void game_finish();
+void game_unload_scripts();
 void pause_toggle();
 bool game_is_paused();
 bool game_is_not_paused();

src/openrct2/core/FileWatcher.cpp

@@ -165,7 +165,7 @@ void FileWatcher::WatchDirectory()
                 int offset = 0;
                 while (offset < length)
                 {
-                    auto e = (inotify_event*)(eventData.data() + offset);
+                    auto e = reinterpret_cast<inotify_event*>(eventData.data() + offset);
                     if ((e->mask & IN_CLOSE_WRITE) && !(e->mask & IN_ISDIR))
                     {
                         log_verbose("FileWatcher: inotify event received for %s", e->name);

src/openrct2/network/Network.cpp

@@ -1473,7 +1473,7 @@ void Network::Server_Send_OBJECTS(NetworkConnection& connection, const std::vect
 void Network::Server_Send_SCRIPTS(NetworkConnection& connection) const
 {
     std::unique_ptr<NetworkPacket> packet(NetworkPacket::Allocate());
-    *packet << (uint32_t)NETWORK_COMMAND_SCRIPTS;
+    *packet << static_cast<uint32_t>(NETWORK_COMMAND_SCRIPTS);
 #    ifdef __ENABLE_SCRIPTING__
     using namespace OpenRCT2::Scripting;
 
@@ -1490,18 +1490,18 @@ void Network::Server_Send_SCRIPTS(NetworkConnection& connection) const
     }
 
     log_verbose("Server sends %u scripts", pluginsToSend.size());
-    *packet << (uint32_t)pluginsToSend.size();
+    *packet << static_cast<uint32_t>(pluginsToSend.size());
     for (const auto& plugin : pluginsToSend)
     {
         const auto& metadata = plugin->GetMetadata();
         log_verbose("Script %s", metadata.Name.c_str());
 
         const auto& code = plugin->GetCode();
-        *packet << (uint32_t)code.size();
+        *packet << static_cast<uint32_t>(code.size());
-        packet->Write((const uint8_t*)code.c_str(), code.size());
+        packet->Write(reinterpret_cast<const uint8_t*>(code.c_str()), code.size());
     }
 #    else
-    *packet << (uint32_t)0;
+    *packet << static_cast<uint32_t>(0);
 #    endif
     connection.QueuePacket(std::move(packet));
 }
@@ -2451,7 +2451,7 @@ void Network::Client_Handle_SCRIPTS(NetworkConnection& connection, NetworkPacket
     {
         uint32_t codeLength{};
         packet >> codeLength;
-        auto code = std::string_view((const char*)packet.Read(codeLength), codeLength);
+        auto code = std::string_view(reinterpret_cast<const char*>(packet.Read(codeLength)), codeLength);
         scriptEngine.AddNetworkPlugin(code);
     }
 #    else
@@ -2886,6 +2886,42 @@ void Network::Client_Handle_CHAT([[maybe_unused]] NetworkConnection& connection,
     }
 }
 
+static bool ProcessChatMessagePluginHooks(const NetworkPlayer& player, std::string& text)
+{
+#    ifdef __ENABLE_SCRIPTING__
+    auto& hookEngine = GetContext()->GetScriptEngine().GetHookEngine();
+    if (hookEngine.HasSubscriptions(OpenRCT2::Scripting::HOOK_TYPE::NETWORK_CHAT))
+    {
+        auto ctx = GetContext()->GetScriptEngine().GetContext();
+
+        // Create event args object
+        auto objIdx = duk_push_object(ctx);
+        duk_push_number(ctx, static_cast<int32_t>(player.Id));
+        duk_put_prop_string(ctx, objIdx, "player");
+        duk_push_string(ctx, text.c_str());
+        duk_put_prop_string(ctx, objIdx, "message");
+        auto e = DukValue::take_from_stack(ctx);
+
+        // Call the subscriptions
+        hookEngine.Call(OpenRCT2::Scripting::HOOK_TYPE::NETWORK_CHAT, e, false);
+
+        // Update text from object if subscriptions changed it
+        if (e["message"].type() != DukValue::Type::STRING)
+        {
+            // Subscription set text to non-string, do not relay message
+            return false;
+        }
+        text = e["message"].as_string();
+        if (text.empty())
+        {
+            // Subscription set text to empty string, do not relay message
+            return false;
+        }
+    }
+#    endif
+    return true;
+}
+
 void Network::Server_Handle_CHAT(NetworkConnection& connection, NetworkPacket& packet)
 {
     auto szText = packet.ReadString();
@@ -2904,37 +2940,11 @@ void Network::Server_Handle_CHAT(NetworkConnection& connection, NetworkPacket& p
     std::string text = szText;
     if (connection.Player != nullptr)
     {
-#    ifdef __ENABLE_SCRIPTING__
+        if (!ProcessChatMessagePluginHooks(*connection.Player, text))
-        auto& hookEngine = GetContext()->GetScriptEngine().GetHookEngine();
-        if (hookEngine.HasSubscriptions(OpenRCT2::Scripting::HOOK_TYPE::NETWORK_CHAT))
         {
-            auto ctx = GetContext()->GetScriptEngine().GetContext();
+            // Message not to be relayed
-
-            // Create event args object
-            auto objIdx = duk_push_object(ctx);
-            duk_push_number(ctx, (int32_t)connection.Player->Id);
-            duk_put_prop_string(ctx, objIdx, "player");
-            duk_push_string(ctx, text.c_str());
-            duk_put_prop_string(ctx, objIdx, "message");
-            auto e = DukValue::take_from_stack(ctx);
-
-            // Call the subscriptions
-            hookEngine.Call(OpenRCT2::Scripting::HOOK_TYPE::NETWORK_CHAT, e, false);
-
-            // Update text from object if subscriptions changed it
-            if (e["message"].type() != DukValue::Type::STRING)
-            {
-                // Subscription set text to non-string, do not relay message
             return;
         }
-            text = e["message"].as_string();
-            if (text.empty())
-            {
-                // Subscription set text to empty string, do not relay message
-                return;
-            }
-        }
-#    endif
     }
 
     const char* formatted = FormatChat(connection.Player, text.c_str());

src/openrct2/scripting/Duktape.hpp

@@ -151,7 +151,6 @@ namespace OpenRCT2::Scripting
                 duk_set_top(_ctx, _top);
                 _ctx = {};
                 std::fprintf(stderr, "duktape stack was not returned to original state!");
-                // assert(false);
             }
             _ctx = {};
         }

src/openrct2/scripting/Plugin.cpp

@@ -98,10 +98,6 @@ void Plugin::Stop()
     _hasStarted = false;
 }
 
-void Plugin::Update()
-{
-}
-
 void Plugin::LoadCodeFromFile()
 {
     std::string code;

src/openrct2/scripting/Plugin.h

@@ -89,7 +89,6 @@ namespace OpenRCT2::Scripting
         void Load();
         void Start();
         void Stop();
-        void Update();
 
     private:
         void LoadCodeFromFile();

src/openrct2/scripting/ScTile.hpp

@@ -34,7 +34,7 @@ namespace OpenRCT2::Scripting
         TileElement* _element;
 
     public:
-        ScTileElement(CoordsXY coords, TileElement* element)
+        ScTileElement(const CoordsXY& coords, TileElement* element)
             : _coords(coords)
             , _element(element)
         {
@@ -896,7 +896,7 @@ namespace OpenRCT2::Scripting
         CoordsXY _coords;
 
     public:
-        ScTile(CoordsXY coords)
+        ScTile(const CoordsXY& coords)
             : _coords(coords)
         {
         }
@@ -904,12 +904,12 @@ namespace OpenRCT2::Scripting
     private:
         int32_t x_get()
         {
-            return _coords.x / 32;
+            return _coords.x / COORDS_XY_STEP;
         }
 
         int32_t y_get()
         {
-            return _coords.y / 32;
+            return _coords.y / COORDS_XY_STEP;
         }
 
         size_t numElements_get()

src/openrct2/scripting/ScriptEngine.cpp

@@ -884,6 +884,8 @@ void ScriptEngine::LoadSharedStorage()
 
     auto path = _env.GetFilePath(PATHID::PLUGIN_STORE);
     try
+    {
+        if (File::Exists(path))
         {
             auto data = File::ReadAllBytes(path);
             auto result = DuktapeTryParseJson(_context, std::string_view((const char*)data.data(), data.size()));
@@ -892,8 +894,10 @@ void ScriptEngine::LoadSharedStorage()
                 _sharedStorage = std::move(*result);
             }
         }
+    }
     catch (const std::exception&)
     {
+        fprintf(stderr, "Unable to read '%s'\n", path.c_str());
     }
 }