From dfa0edfcdb85d612f5c7ab833b26b725a3716c6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20Br=C3=BCbach?= Date: Wed, 2 Sep 2026 10:27:44 +0200 Subject: [PATCH] [Client] Drop dead player event handler extension points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second PlayerLogic constructor taking a custom PlayerEventHandler was UB (converting this to PlayerLogic* in the base initializer, and connecting a freshly-built handler against a not-yet-constructed object), and its abstract extension surface — virtual initializeZones, virtual getPlayerEventHandler, virtual processGameEvent, protected player member — has no subclass consumer in the stack. Revert to the single primary constructor. Also: - Drop the unused GameMetaInfo::setIsTournament setter. Tournament status is authoritative from Event_GameJoined's ServerInfo_Game; the redundant is_tournament field on Event_GameStateChanged is removed in [Protocol]. - Gate the parent-game routing on has_parent_game_id() instead of comparing the proto default, so a future drop of the default keeps working. --- .../libcockatrice/game/game_event_handler.cpp | 5 ++-- .../libcockatrice/game/game_meta_info.h | 5 ---- .../game/player/player_event_handler.h | 10 ++++---- .../game/player/player_logic.cpp | 13 ---------- .../libcockatrice/game/player/player_logic.h | 25 +------------------ 5 files changed, 9 insertions(+), 49 deletions(-) diff --git a/libcockatrice_game/libcockatrice/game/game_event_handler.cpp b/libcockatrice_game/libcockatrice/game/game_event_handler.cpp index 27526a361..931e6425c 100644 --- a/libcockatrice_game/libcockatrice/game/game_event_handler.cpp +++ b/libcockatrice_game/libcockatrice/game/game_event_handler.cpp @@ -380,8 +380,9 @@ void GameEventHandler::eventGameStateChanged(const Event_GameStateChanged &event const GameEventContext & /*context*/) { // Sub-games of a tournament report their parent hub game so the client can - // route "close game" back to the parent tab. - if (event.parent_game_id() != -1) { + // route "close game" back to the parent tab. Use has_parent_game_id rather + // than comparing the default so a future drop of the proto default still works. + if (event.has_parent_game_id()) { game->getGameMetaInfo()->setParentGameId(event.parent_game_id()); } diff --git a/libcockatrice_game/libcockatrice/game/game_meta_info.h b/libcockatrice_game/libcockatrice/game/game_meta_info.h index 0ea5cd7cc..b408ef061 100644 --- a/libcockatrice_game/libcockatrice/game/game_meta_info.h +++ b/libcockatrice_game/libcockatrice/game/game_meta_info.h @@ -92,11 +92,6 @@ public: return gameInfo_.is_tournament(); } - void setIsTournament(bool t) - { - gameInfo_.set_is_tournament(t); - } - int parentGameId() const { return parentGameId_; diff --git a/libcockatrice_game/libcockatrice/game/player/player_event_handler.h b/libcockatrice_game/libcockatrice/game/player/player_event_handler.h index 59e448d7a..d253deef0 100644 --- a/libcockatrice_game/libcockatrice/game/player/player_event_handler.h +++ b/libcockatrice_game/libcockatrice/game/player/player_event_handler.h @@ -93,10 +93,10 @@ public: * @param context Additional context (undo, judge, etc.). * @param options Processing options (UI suppression, reveal behavior). */ - virtual void processGameEvent(GameEvent::GameEventType type, - const GameEvent &event, - const GameEventContext &context, - EventProcessingOptions options); + void processGameEvent(GameEvent::GameEventType type, + const GameEvent &event, + const GameEventContext &context, + EventProcessingOptions options); /** @} */ @@ -277,7 +277,7 @@ signals: */ void cardViewRefreshRequested(CardState *card, bool zoneChanged, PlayerLogic *newOwner); -protected: +private: /** Owning player instance. */ PlayerLogic *player; diff --git a/libcockatrice_game/libcockatrice/game/player/player_logic.cpp b/libcockatrice_game/libcockatrice/game/player/player_logic.cpp index 9098729eb..96bd07164 100644 --- a/libcockatrice_game/libcockatrice/game/player/player_logic.cpp +++ b/libcockatrice_game/libcockatrice/game/player/player_logic.cpp @@ -49,19 +49,6 @@ PlayerLogic::PlayerLogic(const ServerInfo_User &info, initializeZones(); } -PlayerLogic::PlayerLogic(const ServerInfo_User &info, - int _id, - bool _local, - bool _judge, - AbstractGame *_parent, - PlayerEventHandler *customEventHandler) - : QObject(_parent), game(_parent), playerInfo(new PlayerInfo(info, _id, _local, _judge)), - playerEventHandler(customEventHandler), playerActions(new PlayerActions(this)), active(false), conceded(false), - zoneId(0), dialogSemaphore(false) -{ - initializeZones(); -} - void PlayerLogic::initializeZones() { addZone(new PileZoneLogic(this, ZoneNames::DECK, false, true, false, this)); diff --git a/libcockatrice_game/libcockatrice/game/player/player_logic.h b/libcockatrice_game/libcockatrice/game/player/player_logic.h index 168d5fd5f..d6c4f5187 100644 --- a/libcockatrice_game/libcockatrice/game/player/player_logic.h +++ b/libcockatrice_game/libcockatrice/game/player/player_logic.h @@ -103,30 +103,7 @@ public: ICardPreferenceProvider *cardPreferences = nullptr); ~PlayerLogic() override; -protected: - /** - * @brief Constructor for subclasses that need a custom event handler (e.g. DraftPlayerLogic). - * - * @p customEventHandler must be non-null and either QObject-parented to this PlayerLogic - * or deleted externally; it is not owned by PlayerLogic. The handler connects to @c player - * during its own constructor, so passing a freshly built subclass handler from an - * initializer list is safe. - */ - PlayerLogic(const ServerInfo_User &info, - int _id, - bool _local, - bool _judge, - AbstractGame *_parent, - PlayerEventHandler *customEventHandler); - -public: - /** - * @brief Creates the standard zone set. - * - * Not virtually dispatched from constructors — subclasses overriding this must add - * their extra zones in their own constructor body. - */ - virtual void initializeZones(); + void initializeZones(); void updateZones(); void clear();