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

Add PDCurses to Windows #74057

Merged
merged 27 commits into from
Jul 6, 2024
Merged

Add PDCurses to Windows #74057

merged 27 commits into from
Jul 6, 2024

Conversation

alef
Copy link
Contributor

@alef alef commented May 24, 2024

Summary

Build "Add PDCurses to Windows"

Purpose of change

Allow to build TILES=0 for Windows with PDCurses from VCPKG.

Describe the solution

  • Decouple defined(_WIN32) from defined(TILES) by using a new #define TUI
  • Disable wincurse.cpp
  • Add USE_PDCURSES to CMake builds
  • Add more CMake presets

More details in the commits comments

Describe alternatives you've considered

None

Testing

Built with:

  1. CMake Visual Studio presets
  • windows-x64-msvc
  • windows-tiles-sounds-x64-msvc
  1. CMake WSL2 MSYS2 UCRT64 presets
  • windows-x64
  • windows-tiles-sounds-x64
  1. CMake WSL2 Ubuntu presets
  • linux-x64
  • linux-tiles-sounds-x64
  1. Makefile WSL2 MSYS2 UCRT64 with arguments
  • CCACHE=1 RELEASE=1 MSYS=1 DYNAMIC_LINKING=1 SDL=0 TILES=0 SOUND=0 LOCALIZE=0 LINTJSON=0 ASTYLE=0 TESTS=0 cataclysm.exe

Run inside Windows Terminal 1.19, open the main menu, load a debug game, open the overmap, quit:

  • MSVC has red and blue colors swapped. Can not be fixed with PDCurses' -DPDC_RGB.
  • UCRT64 prints extra characters in some places
  • MSYS' Makefile build crashes wit failed assertion in imgui.cpp:9123 "Need a positive DeltaTime!"
  • Console resizing doesn't work for now

Run SDL native or under WSLg.

Additional context

This PR in draft to check the CI build and the linters.

Related PR/issues:

This is the MSYS UCRT64 MINGW NCURSES build running in cmd.exe 80x25:
image

This is the MSVC PDCurses build running in cmd.exe 80x25:
image

@github-actions github-actions bot added Info / User Interface Game - player communication, menus, etc. Code: Build Issues regarding different builds and build environments [JSON] Changes (can be) made in JSON Code: Tests Measurement, self-control, statistics, balancing. [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 astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels May 24, 2024
@katemonster33
Copy link
Contributor

katemonster33 commented May 25, 2024

Looks like this PR would fix #73011 which would be a huge weight off my shoulders when changes to ImTui (TUI version of ImGui) need to be made

@alef
Copy link
Contributor Author

alef commented May 26, 2024

Regarding UCRT64 prints extra characters in some places I am now trying using mingw-w64-ucrt-x86_64-pdcurses instead of mingw-w64-ucrt-x86_64-ncurses as suggested by ImTui.

Edit: it doesn't work because PDCurses uses 64-bit chtype while the game always assume 32-bit:
image

@alef alef marked this pull request as ready for review May 31, 2024 22:05
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-requesting reviews from non-collaborators: @Qrox

@katemonster33
Copy link
Contributor

I get these statis assertion failures when building on Windows using VS2022 Community Edition:

1>C:\Users\Kat\Documents\Cataclysm-DDA\src\ncurses_def.cpp(290,35): error C2338: static_assert failed: 'yellow must have the same value as COLOR_YELLOW (from ncurses)'
1>C:\Users\Kat\Documents\Cataclysm-DDA\src\ncurses_def.cpp(292,33): error C2338: static_assert failed: 'blue must have the same value as COLOR_BLUE (from ncurses)'
1>C:\Users\Kat\Documents\Cataclysm-DDA\src\ncurses_def.cpp(296,33): error C2338: static_assert failed: 'cyan must have the same value as COLOR_CYAN (from ncurses)'
1>C:\Users\Kat\Documents\Cataclysm-DDA\src\ncurses_def.cpp(320,11): error C2039: 'is_term_resized': is not a member of '`global namespace''
1>C:\Users\Kat\Documents\Cataclysm-DDA\src\ncurses_def.cpp(320,11): error C3861: 'is_term_resized': identifier not found
1>C:\Users\Kat\Documents\Cataclysm-DDA\src\ncurses_def.cpp(338,47): error C2039: 'newscr': is not a member of '`global namespace''
1>C:\Users\Kat\Documents\Cataclysm-DDA\src\ncurses_def.cpp(338,43): error C2440: '<function-style-cast>': cannot convert from 'initializer list' to 'std::shared_ptr<void>'
1>C:\Users\Kat\Documents\Cataclysm-DDA\src\ncurses_def.cpp(349,5): error C3861: 'set_escdelay': identifier not found
1>C:\Users\Kat\Documents\Cataclysm-DDA\src\ncurses_def.cpp(437,17): error C2660: 'getmouse': function does not take 1 arguments```

@Maleclypse
Copy link
Member

Is this ready or waiting on akrieger or something else?

@akrieger
Copy link
Member

I made the non-curses build work but I don't have time to validate the curses build still works given the changes I had to make that I called out.

@alef
Copy link
Contributor Author

alef commented Jun 19, 2024

This caused crashes in Windows debug mode because the debug RTL injects own things and memset destroy them.

@Maleclypse Maleclypse merged commit 5cd2c01 into CleverRaven:master Jul 6, 2024
27 of 28 checks passed
@alef alef deleted the pdcurses-msvc branch August 16, 2024 17:10
SurFlurer added a commit to SurFlurer/Cataclysm-DDA that referenced this pull request Aug 26, 2024
This reverts commit 5cd2c01, reversing
changes made to c8cc550.
this
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` Code: Build Issues regarding different builds and build environments Code: Tests Measurement, self-control, statistics, balancing. ImGui Anything related to the new ImGui UI for SDL/tiles or ImTui for curses builds Info / User Interface Game - player communication, menus, etc. [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants