Skip to content

unify(dx8wrapper): Migrate dx8wrapper.cpp and h to the core WW3D2 folder #1401

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

Closed

Conversation

ryan-de-boer
Copy link

The core dx8wrapper is now the ZH one, and generals code has been changed to work with it.

@ryan-de-boer ryan-de-boer changed the title Unify dx8wrapper.cpp and h in the core WW3D2 folder unify(dx8wrapper) .cpp and h in the core WW3D2 folder Aug 1, 2025
@ryan-de-boer ryan-de-boer changed the title unify(dx8wrapper) .cpp and h in the core WW3D2 folder unify(dx8wrapper): .cpp and h in the core WW3D2 folder Aug 1, 2025
@ryan-de-boer ryan-de-boer changed the title unify(dx8wrapper): .cpp and h in the core WW3D2 folder unify(dx8wrapper): Migrate dx8wrapper.cpp and h to the core WW3D2 folder Aug 1, 2025
@Caball009 Caball009 added Gen Relates to Generals ZH Relates to Zero Hour Unify Unifies code between Generals and Zero Hour labels Aug 1, 2025
@Mauller
Copy link

Mauller commented Aug 2, 2025

This is actually really difficult to follow because of how it has been performed.

Because of the number of files and nature of this part of the code, it would have been better to first unify the differences between the files from the two games. Then look at extracting commonality and moving to core.

Right now entire chunks are moved about making it difficult to read the changes that were possibly made or overlooked between the two versions.

@xezon
Copy link

xezon commented Aug 2, 2025

This is a good initiative. We need to get ww3d2 unified. Would it be possible to breakdown what features and fixes Generals inherited?

Unification changes going to main branch definitely need at least 2 commits:

  • First for merging code
  • Second for moving files to Core

I would not mind merging code in small steps if that makes reviewing easier, given the complexity of the diff. Would that be feasible to do with dx8wrapper?

@ryan-de-boer
Copy link
Author

Thanks for your comment xezon. I'll try to make shorter pull requests and try to make reviewing easier. Not even moving files until the two duplicate dx8wrapper.cpp/h files are identical and pull request approved first.

@xezon
Copy link

xezon commented Aug 3, 2025

That sounds lovely.

As for moving files to Core, I have created a helper script here that makes it easy: #1398

@xezon
Copy link

xezon commented Aug 8, 2025

Do we want to close this pull in the meantime?

@ryan-de-boer
Copy link
Author

Closing this pull request, as I have started again with this smaller pull request:
#1423

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Gen Relates to Generals Unify Unifies code between Generals and Zero Hour ZH Relates to Zero Hour
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants