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

Use ncurses accessor macros #73586

Merged
merged 1 commit into from
May 10, 2024
Merged

Use ncurses accessor macros #73586

merged 1 commit into from
May 10, 2024

Conversation

kevingranade
Copy link
Member

@kevingranade kevingranade commented May 8, 2024

Summary

None

Purpose of change

It was pointed out that the experimental builds on cataclysm.org weren't updating, that means one of the release builds is broken.
Fixes #73562
Turns out it's macOS curses build https://github.com/CleverRaven/Cataclysm-DDA/actions/runs/9003013225/job/24732683327#step:27:370
It's broken because macOS defaults to treating the WINDOW struct as opaque, i.e. you're not allowed to directly access data members and must use the accessor macros.

Describe the solution

Use the accessor macros.

Describe alternatives you've considered

You CAN make WINDOW un-opaque, but this is best practice for portability.

Testing

Builds and works locally, I'll need to see if it succeeds the macOS build in CI since I don't have a macOS build env.

@NetSysFire
Copy link
Member

This would fix #73562

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` ImGui Anything related to the new ImGui UI for SDL/tiles or ImTui for curses builds astyled astyled PR, label is assigned by github actions json-styled JSON lint passed, label assigned by github actions labels May 8, 2024
@kevingranade
Copy link
Member Author

Just noticed that we don't have a macOS, Curses build which is how this slipped though, maybe think about rearranging the build targets to get coverage for that combination.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label May 9, 2024
@PGR-14
Copy link

PGR-14 commented May 10, 2024

When will this be merged? It's been a bit more than a week since the website was updated.

@Maleclypse Maleclypse merged commit 9734160 into master May 10, 2024
27 of 28 checks passed
@Maleclypse
Copy link
Member

When will this be merged? It's been a bit more than a week since the website was updated.

When it was merged. Mergers get to things as they come through and see it’s ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` ImGui Anything related to the new ImGui UI for SDL/tiles or ImTui for curses builds json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Experimental builds on the website
4 participants