Enable default overlay interaction forwarding#49
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| std::function<void(const inventory_entry&)> OverlayInteractionBridge::make_inventory_click_default() { | ||
| return [this](const inventory_entry& entry) { | ||
| event_bus_adapter_.publish(InventoryOverlayForwardedClickEvent(entry)); | ||
| }; | ||
| } | ||
|
|
||
| std::function<void(const SDL_KeyboardEvent&)> OverlayInteractionBridge::make_inventory_key_default() { | ||
| return [this](const SDL_KeyboardEvent& key_event) { | ||
| event_bus_adapter_.publish(InventoryOverlayForwardedKeyEvent(key_event)); | ||
| }; | ||
| } | ||
|
|
||
| std::function<void(const std::string&)> OverlayInteractionBridge::make_character_tab_default() { | ||
| return [this](const std::string& tab_id) { | ||
| event_bus_adapter_.publish(CharacterOverlayForwardedTabEvent(tab_id)); | ||
| }; | ||
| } | ||
|
|
||
| std::function<void(const std::string&, int)> OverlayInteractionBridge::make_character_row_default() { | ||
| return [this](const std::string& tab_id, int row_index) { | ||
| event_bus_adapter_.publish(CharacterOverlayForwardedRowEvent(tab_id, row_index)); | ||
| }; | ||
| } | ||
|
|
||
| std::function<void(CharacterCommand)> OverlayInteractionBridge::make_character_command_default() { | ||
| return [this](CharacterCommand command) { | ||
| event_bus_adapter_.publish(CharacterOverlayForwardedCommandEvent(command)); |
There was a problem hiding this comment.
Default forwarding lambdas never run
The new default handlers publish InventoryOverlayForwarded* and CharacterOverlayForwarded* events when no custom callbacks are installed, but OverlayManager::StartInventoryForwarding/StartCharacterForwarding still unconditionally replace these handlers with its own stored defaults, which are initialized to no‑op lambdas. When the overlay is opened without explicit handlers, the bridge therefore continues to drop click/key/tab events and the new assertions in RunOverlayInventoryBridgeDefaultForwardingTest and RunOverlayCharacterBridgeDefaultForwardingTest will fail. Consider only assigning handlers when non‑null callbacks are supplied or initializing the manager’s defaults to nullptr so that these lambdas remain in effect.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| class CharacterOverlayForwardedCommandEvent : public GuiEvent { | ||
| public: | ||
| CharacterOverlayForwardedCommandEvent() : GuiEvent("overlay_interaction") {} | ||
| explicit CharacterOverlayForwardedCommandEvent(CharacterCommand command) | ||
| : GuiEvent("overlay_interaction"), command_(command) {} | ||
|
|
||
| std::string getEventTypeName() const override { return "character_overlay_forwarded_command"; } | ||
| std::unique_ptr<Event> clone() const override { | ||
| auto cloned = std::make_unique<CharacterOverlayForwardedCommandEvent>(command_); | ||
| cloned->setSource(getSource()); | ||
| return cloned; | ||
| } | ||
|
|
||
| CharacterCommand getCommand() const { return command_; } | ||
|
|
||
| private: | ||
| CharacterCommand command_ = CharacterCommand::HELP; | ||
| }; |
There was a problem hiding this comment.
Declare CharacterCommand before using it in forwarded event class
The new CharacterOverlayForwardedCommandEvent stores a CharacterCommand but the enum is only declared later in the header. Because there is no forward declaration before this class, the compiler sees CharacterCommand as an unknown type and the header will fail to compile. Move the enum class CharacterCommand above this class or add a forward declaration (enum class CharacterCommand;) before the class definition.
Useful? React with 👍 / 👎.
Summary
Testing
Codex Task