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
Initial SDL2 port #241
Initial SDL2 port #241
Conversation
I would prefer this not be merged as my particular target platform only has SDL 1.2 and not SDL2. A preferred solution would be to make it optional. |
What platform is this? Dreamcast? |
Yes... but there are also other old platforms in the same boat. Also, I'm aware DC has OpenGL support I could use but that means dealing with everything else separately. I would like to see this patch be modified to be an option to use SDL2 rather than displacing SDL1.2 entirely. |
Our major platforms are Linux, macOS and Windows (actual platforms for most of users). We cannot support both SDL 1.2 and SDL 2 at same time. SDL 1.2 nearly reached EOL support and lacks of features like hardware acceleration support and advanced rendering support, that we count on in future development. |
SDL2 is currently in the pipeline as we work on getting windows up to parity with Ryan's 2020 Linux and mac ports. If you want to keep SDL1.2, you should create a fork of this project and branch it. |
ea32991
to
815628b
Compare
This causes keyboard controls to act weird. pitch and yaw start out fast but quickly ramp down to a crawl. roll is limited to about 45°. might be related to removal of |
Currently Windows gets this from vcpkg. Mac will get it from Homebrew, and the GitHub linux builders will just install it with apt-get. This might not be the perfect solution (having to install Homebrew is a pain, GitHub Actions has an ancient SDL2), but it gets all the common platforms running for now without much fuss.
Rebased against the latest in main; it's one thing to merge PRs instead of rebase them, but merging main to an unmerged PR seems like a bridge too far, y'all. :) |
Whoops, there's a later commit that fixed this I haven't moved over yet. I'll verify today and get that pushed to this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested out this pull request on NixOS 23.11. It worked very well. Here are my notes:
- This implements Ability to unlock keybord/mouse #264 which is very helpful and appreciated.
- When I run the version of the game that’s in the main branch (bea6bec), cutscene audio doesn’t work properly. When I run the version of the game that’s in this PR, cutscene audio still doesn’t work, but it’s much worse. Here’s what it sounds like, but please turn down your volume before the SDL2 version of the cutscenes starts playing.
- Dependencies in
README.md
need to be updated. They still say to install SDL 1.2. - I can’t bind controls to the scroll wheel.
This doesn't fix the audio gaps, just the static introduced in the SDL2 port. SDL2 does not initialize the audio callback's buffer, unlike SDL 1.2, under the assumption the callback is going to fully write it anyhow. But since the movie player wants to mix against the current contents of the buffer, we need to explicitly initialize it to silence first.
Fixes keyboard input during gameplay.
This is fixed in 851f475 (and yes, it was a key-repeat thing, good call!). |
This is fixed in 65b1a7d. (This was because I had abandoned the MVE code in my fork, so this never got fixed before.) |
Update SDL 1.2 references, remove SDL_image references.
Resolved in bd3a596, thanks! |
Fixed in 8033513 ...this also adds support for mouse buttons beyond left/middle/right, but this is untested, as I don't have a mouse that supports it. |
I do, I'll give this a try today and let you know....assuming I can try on windows. |
Eventually you can, but right now the SDL2 work only builds on Linux and macOS; Windows still talks directly to the Win32 API and DirectX instead. |
Okay, I think I've resolved all the concerns for this PR; there will be more SDL2 work in the future, including moving it to Windows, and other patches on top of this, like all the video resolution improvements, but this seems like a reasonable baseline, if people are happy with it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only thing that I've noticed - app runs in windowed mode, but that ok.
Tested on Linux 64bit.
Oh yeah, I changed it back to default to fullscreen right before shipping. It's a one-line change, I'll switch it shortly. |
Fullscreen mode defaults to on as of 6d837e2. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, tested on Linux x64. We can merge as it is and solve further problems from here
@@ -115,6 +115,7 @@ CLnxVideoModes::~CLnxVideoModes() { | |||
// enumerate all the possible video modes | |||
bool CLnxVideoModes::Init(void) // Display *dpy,int screen) | |||
{ | |||
#if 0 // with SDL2, we'll just use FULLSCREEN_DESKTOP and never change the physical vidmode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
to be deleted sometime
This just gets the basic SDL2 work in place. The resolution changes, etc, are coming in a separate PR.
These are split across several commits because that's how it was in my fork, but I could squash this down to a single unit of work and force-push this PR if that's preferable.
Note that SDL2 has no CD-ROM subsystem, so this removes physical disc support from ddio_lnx/lnxcdrom.cpp. If that's a serious problem in 2024, we can talk about what to do about it; likely we can get away with something quick and dirty if this is even necessary anymore.
This has gone as far as "it compiles and works on my machine," so please test it! I'm marking this as a draft PR until GitHub Actions gives the thumbs up.