From 8faa1111fce7b64f8d2f9f9e54fe14016c864650 Mon Sep 17 00:00:00 2001 From: Yaroslav Chvanov Date: Fri, 24 Apr 2026 19:01:45 +0300 Subject: [PATCH] Return an optional instead of casting -1 into an enum For enums without a fixed underlying type, casting into them from numbers outside of their enumeration values is undefined behavior. Fixes running tests with UBSan: `runtime error: load of value 4294967295, which is not a valid value for type 'Trigger'`. --- src/game_event.cpp | 8 +++++--- src/game_event.h | 3 ++- src/game_map.cpp | 12 +++++++----- src/game_player.cpp | 8 ++++---- tests/game_character.cpp | 3 ++- 5 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/game_event.cpp b/src/game_event.cpp index 039e232104..9eef4decda 100644 --- a/src/game_event.cpp +++ b/src/game_event.cpp @@ -34,6 +34,7 @@ #include "output.h" #include #include +#include Game_Event::Game_Event(int map_id, const lcf::rpg::Event* event) : Game_EventBase(Event), @@ -309,9 +310,10 @@ bool Game_Event::WasStartedByDecisionKey() const { return data()->triggered_by_decision_key; } -lcf::rpg::EventPage::Trigger Game_Event::GetTrigger() const { - int trigger = page ? page->trigger : -1; - return static_cast(trigger); +std::optional Game_Event::GetTrigger() const { + if (page) + return static_cast(page->trigger); + return std::nullopt; } diff --git a/src/game_event.h b/src/game_event.h index b2f3dfd07f..3bc1a83712 100644 --- a/src/game_event.h +++ b/src/game_event.h @@ -19,6 +19,7 @@ #define EP_GAME_EVENT_H // Headers +#include #include #include #include "game_character.h" @@ -99,7 +100,7 @@ class Game_Event : public Game_EventBase { * * @return trigger condition. */ - lcf::rpg::EventPage::Trigger GetTrigger() const; + std::optional GetTrigger() const; /** * Gets event commands list. diff --git a/src/game_map.cpp b/src/game_map.cpp index 0a4e982d20..c5aeff2967 100644 --- a/src/game_map.cpp +++ b/src/game_map.cpp @@ -1452,8 +1452,8 @@ bool Game_Map::UpdateForegroundEvents(MapUpdateAsyncContext& actx) { if (run_ev) { if (run_ev->WasStartedByDecisionKey()) { interp.Push(run_ev); - } else { - switch (run_ev->GetTrigger()) { + } else if (auto t = run_ev->GetTrigger()) { + switch (*t) { case lcf::rpg::EventPage::Trigger_touched: interp.Push(run_ev); break; @@ -1464,10 +1464,12 @@ bool Game_Map::UpdateForegroundEvents(MapUpdateAsyncContext& actx) { interp.Push(run_ev); break; case lcf::rpg::EventPage::Trigger_action: - default: + case lcf::rpg::EventPage::Trigger_parallel: interp.Push(run_ev); break; } + } else { + interp.Push(run_ev); } run_ev->ClearWaitingForegroundExecution(); } @@ -1723,7 +1725,7 @@ void Game_Map::SetPositionX(int x, bool reset_panorama) { const int map_width = GetTilesX() * SCREEN_TILE_SIZE; if (LoopHorizontal()) { x = Utils::PositiveModulo(x, map_width); - + // If the map is too small to fit in the screen, add an offset corresponding to the black border's size if (Player::game_config.fake_resolution.Get()) { int map_width_in_pixels = Game_Map::GetTilesX() * TILE_SIZE; @@ -2157,7 +2159,7 @@ void Game_Map::Parallax::ResetPositionX() { if (!params.scroll_horz && !LoopHorizontal()) { // What is the width of the panorama to display on screen? int pan_screen_width = Player::screen_width; - if (Player::game_config.fake_resolution.Get()) { + if (Player::game_config.fake_resolution.Get()) { int map_width = Game_Map::GetTilesX() * TILE_SIZE; if (map_width < pan_screen_width) { pan_screen_width = map_width; diff --git a/src/game_player.cpp b/src/game_player.cpp index ce34065a00..42e920955b 100644 --- a/src/game_player.cpp +++ b/src/game_player.cpp @@ -459,8 +459,8 @@ bool Game_Player::CheckEventTriggerHere(TriggerSet triggers, bool triggered_by_d && ev.GetX() == GetX() && ev.GetY() == GetY() && ev.GetLayer() != lcf::rpg::EventPage::Layers_same - && trigger >= 0 - && triggers[trigger]) { + && trigger.has_value() + && triggers[*trigger]) { SetEncounterCalling(false); result |= ev.ScheduleForegroundExecution(triggered_by_decision_key, face_player); } @@ -480,8 +480,8 @@ bool Game_Player::CheckEventTriggerThere(TriggerSet triggers, int x, int y, bool && ev.GetX() == x && ev.GetY() == y && ev.GetLayer() == lcf::rpg::EventPage::Layers_same - && trigger >= 0 - && triggers[trigger]) { + && trigger.has_value() + && triggers[*trigger]) { SetEncounterCalling(false); result |= ev.ScheduleForegroundExecution(triggered_by_decision_key, face_player); } diff --git a/tests/game_character.cpp b/tests/game_character.cpp index 09f0ab281f..beedbb0f34 100644 --- a/tests/game_character.cpp +++ b/tests/game_character.cpp @@ -4,6 +4,7 @@ #include "game_vehicle.h" #include "doctest.h" #include +#include static_assert(Game_Character::GetMaxStopCountForStep(1) == 256, "StopCountBroken"); static_assert(Game_Character::GetMaxStopCountForStep(2) == 128, "StopCountBroken"); @@ -225,7 +226,7 @@ TEST_CASE("InitEventNoPage") { REQUIRE(!ch.IsWaitingForegroundExecution()); REQUIRE(!ch.WasStartedByDecisionKey()); - REQUIRE_EQ(ch.GetTrigger(), -1); + REQUIRE_EQ(ch.GetTrigger(), std::nullopt); REQUIRE_EQ(ch.GetOriginalMoveRouteIndex(), 0); REQUIRE_EQ(ch.GetPage(1), nullptr); REQUIRE_EQ(ch.GetActivePage(), nullptr);