search

Prof-Butts / xwa_ddraw_d3d11

Direct3D 11 implementation of DDraw.dll for XWA with VR support and New Shaders
MIT License
6 stars 4 forks source link

Code refactor #33

Closed morallo closed 3 years ago

morallo commented 3 years ago

@Prof-Butts I have started moving stuff around and pushed to a WIP branch.

Still didn't fix all the imports, declarations etc. so it's not ready to merge, but if you can take a look to see if you are ok with the structure or you can suggest where to put things, I create this draft PR.

TO FIX

TO DO

Prof-Butts commented 3 years ago

From what I can see, there are no functional changes here, only a (major) refactor. If that's the case, then this looks good to me.

Sorry for creating such a mess, and thank you for doing this much-needed refactor!

There was no way for you to know this, but some of that code is dead, and some SteamVR functions are also used in the DirectSBS path (so they should be in the common VR file). But those are minor details that can be fixed later. For this PR I think we should focus on the refactor without changing any functionality.

Looks like there are still a few things that need to be finalized in this PR, but once you're ready let me know and I'll merge it.

morallo commented 3 years ago

Thanks for the feedback. If you see something misplaced, feel free to put a comment in the line itself with the code review features from GitHub!

Yesterday I split a lot more things, including DC, AC, bloom, materials, reticle...

Now I have a ton of compilation errors that I'm fixing slowly 😅 It's mostly putting the declarations of functions and globals in the right header files, and the corresponding includes.

I'll try to keep pushing my WIP.

Prof-Butts commented 3 years ago

Well, it doesn't have to be the full refactor in one go. We can do it in smaller steps too, making sure that master compiles and is stable at each step. For instance, the reticle code is particularly complicated, we can do that later.

BTW, I just noticed that some of the commits in your PR date back to July. I assume you refreshed your working copy from master recently -- just wanted to make sure.

Thanks again!

morallo commented 3 years ago

So, the refactor is in a buildable state, @Prof-Butts please check if it makes sense for you and if you want to merge now and start working with the new file organization.

There are still many things that should be taken away from Jeremy's class files, but it's a start.