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

Xbox NXDK platform #4838

Merged
merged 8 commits into from
Jul 9, 2022
Merged

Xbox NXDK platform #4838

merged 8 commits into from
Jul 9, 2022

Conversation

glebm
Copy link
Collaborator

@glebm glebm commented Jul 1, 2022

DevilutionX now builds with https://github.com/XboxDev/nxdk.

xbe_logo

Reported 26 FPS on a real Xbox in town.

There are some outstanding PRs to fix a few issues (listed below), so we build from my fork for now.

Note: Requires clang that comes with Ubuntu 22.04 (20.04 won't work).

Issues:

@glebm glebm mentioned this pull request Jul 1, 2022
@glebm glebm force-pushed the xbox-nxdk branch 5 times, most recently from a19084e to cba8b50 Compare July 1, 2022 10:06
@AJenbo
Copy link
Member

AJenbo commented Jul 1, 2022

Wow, it would be exciting to finally have an OG Xbox port under our wings :)

Looks like building fmt is currently the biggest blocker.

@AJenbo
Copy link
Member

AJenbo commented Jul 1, 2022

@brentdc-nz you might be interested in this :)

@glebm glebm force-pushed the xbox-nxdk branch 10 times, most recently from a57403f to 10273c5 Compare July 3, 2022 22:32
@glebm glebm marked this pull request as ready for review July 3, 2022 22:32
@glebm glebm enabled auto-merge (rebase) July 3, 2022 22:45
@AJenbo AJenbo force-pushed the xbox-nxdk branch 2 times, most recently from ee2d9b9 to 55e6bb6 Compare July 4, 2022 01:28
@glebm glebm force-pushed the xbox-nxdk branch 5 times, most recently from 07ed0ba to c0fcb17 Compare July 4, 2022 10:35
@glebm glebm force-pushed the xbox-nxdk branch 5 times, most recently from 28a7f70 to 78f6efa Compare July 6, 2022 09:54
@glebm
Copy link
Collaborator Author

glebm commented Jul 6, 2022

@Halofreak1990 I've noticed your port of Devilution (not DevilutionX) to the OG Xbox at https://github.com/Halofreak1990/devilution-xbox. This is a port of DevilutionX to the OG Xbox using NXDK that you might be interested in.

@Halofreak1990
Copy link

@Halofreak1990 I've noticed your port of Devilution (not DevilutionX) to the OG Xbox at https://github.com/Halofreak1990/devilution-xbox. This is a port of DevilutionX to the OG Xbox using NXDK that you might be interested in.

Could be interesting indeed. The reason I went with the Devilution codebase for my port and tried emulating all the Windows stuff, was because DevilutionX depends on SDL2, among other things, and I do not have access to a port of that for the OG Xbox, and I'm not comfortable (like, not at all) with SDL to even try attempting swapping to the SDL 1.x port that does exist. There's also the bit with the modern C++ stuff that Visual Studio 2003--which, up until NXDK came around, was required to build OG Xbox games--just doesn't take.

And then I got stuck on trying to wrap my head around an implementation of the Win32 dialog API, though I did manage to include the resources in the executable and access them. I just didn't get around to rendering anything.

@glebm
Copy link
Collaborator Author

glebm commented Jul 6, 2022

@Halofreak1990 DevilutionX does support SDL1 (-DUSE_SDL1=ON) but luckily we can use SDL2 with NXDK. I've had a few issues porting via NXDK as well but thankfully NXDK folks were very helpful on GitHub and their Discord.

@glebm glebm force-pushed the xbox-nxdk branch 2 times, most recently from 4e1a295 to a65a977 Compare July 7, 2022 01:50
glebm and others added 8 commits July 7, 2022 08:44
ISO is not useful for us because the user still has to put diabdat.mpq
on the hard drive. At this point, might as well put DevilutionX on the
hard drive as well.

Changes the CI to produce a zip file with our assets and the `default.xbe`
executable.
1. Automount D drive
2. Mount E drive
3. Set preference and config paths to `E:\UDATA\devilutionx`

Co-authored-by: Ryzee119 <[email protected]>
Looks like vsync doesn't work on NXDK at the moment.

Defaults it to false and hides it from the menu.

It is still possible to manually enable it via `diablo.ini` for testing.
Comment on lines +248 to +249
if (windowSize.width >= xmode.width && windowSize.height == xmode.height)
break;
Copy link
Member

Choose a reason for hiding this comment

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

This logic could be better, but lets wait until XNDK gets the SDL calls implemented.

@glebm glebm merged commit 15bb2f4 into diasurgical:master Jul 9, 2022
@AJenbo
Copy link
Member

AJenbo commented Jul 9, 2022

@glebm will you create an issue to track the remaning issues?

@AJenbo
Copy link
Member

AJenbo commented Jul 9, 2022

Also I guess we should look at packaging, but it seams it would be pretty trivial?

@glebm glebm deleted the xbox-nxdk branch July 9, 2022 19:53
@AJenbo
Copy link
Member

AJenbo commented Jul 16, 2022

looks like everything is handled upstream. should we update things?

@glebm
Copy link
Collaborator Author

glebm commented Jul 16, 2022

Still need XboxDev/nxdk#606, and then I guess we'll need to fork libfmt into diasurgical

Also I guess we should look at packaging, but it seams it would be pretty trivial?

The package is just a zip file that we already produce

@glebm
Copy link
Collaborator Author

glebm commented Jul 16, 2022

Switched to our libfmt fork in #5011

- name: Install APT packages
run: sudo apt-get update && sudo apt-get install -y clang llvm lld bison flex cmake git gettext

- name: Clone NXDK Repo

Choose a reason for hiding this comment

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

nit: The preferred spelling of nxdk is "nxdk" (all lowercase, even as first word in sentence).
Initially there were some disagreements about that spelling, but we settled on the all-lowercase variant a couple of years ago (if I remember correctly, also to differentiate it from the MS XDK). All-caps NXDK is only used in code for defines and such.

There were other instances in this PR which use the all-caps spelling, too.

Slightly related: While MS didn't agree on a stylization around 2001, the XboxDev community prefers to use the now-preferred MS spelling of "Xbox" (capital "X", lower-case "box") for the console name. If there might be confusion with other consoles, "original Xbox" is used ("original" in lowercase unless at start of sentence or similar).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, noted!

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

Successfully merging this pull request may close these issues.

4 participants