Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IO: fix unexpected MouseSouce type in AddMousePosEvent and AddMouseButtonEvent. #6727

Closed
wants to merge 1 commit into from

Conversation

RickHuang2001
Copy link
Contributor

This is a PR to fix unexpected MouseSouce type in AddMousePosEvent and AddMouseButtonEvent.

Hi, ocornut.

I've been using segross/UnrealImGui for implementing in-game dev tools for a long time. Because of some issues on touch screen devices, I upgraded the ImGui Library to the latest version for the convience of ImGuiMouseSource_TouchScreen.

However, in the AddMousePosEvent and AddMouseButtonEvent, unexpected MouseSouce type was used, so it didn't work well.

@ocornut ocornut added the bug label Aug 16, 2023
ocornut pushed a commit that referenced this pull request Aug 16, 2023
…ePosEvent and AddMouseButtonEvent. (#6727, #2702)

Technically may have had no side-effects unless non-standard alignment used.
@ocornut
Copy link
Owner

ocornut commented Aug 16, 2023

Wow, thanks for finding this!
AFAIK although semantically very incorrect, this would not have any side-effects with typical compiler alignments settings, as the MouseSource value of all three unions would align to the same location.

Did your setup use non-default struct packing? You can use offsetof() to tell the offset of all three variations of MouseSource.

I have pushed as a6857ed with minor amends (debug log prints also used the wrong union).

@ocornut ocornut closed this Aug 16, 2023
@RickHuang2001
Copy link
Contributor Author

RickHuang2001 commented Aug 17, 2023

Thanks, ocornut. It's right that this can work with the same alignment!

Yesterday I edited a few other lines of codes at the same time in UnrealImGui Plugin, and it started to function correctly, so I didn't notice that I don't need changing the variation of MouseSource to make it work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants