Fix vkfw module (and clean up)#26
Conversation
2-3. Even more so if the two files are not identical. So it's not even possible to use basic diff to make sure two stay in sync. |
|
Well, the error is exporting modules inside of an Also I think you could realistically drop header support, since this library is the more "modern" approach to Vulkan anyway, and C++20 did come out ~6 years ago and many people say that C++20 is a good baseline for gamedev nowadays. You could also remove a LOT of macros like But the choice is up to you, I'm probably missing something anyway. |
I faintly remember talking about something like that in the past. For some context, my only interaction with the modules was a while ago. And they were still severely underbaked. I wonder if this is a matter of hoisting just the
I don't think there's much we can do about this one. Luckily it's just a warning.
This one, on the other hand, sound easy to fix: we can just move it down. It's not like its exact position matters much. If it triggers, it triggers.
Which means we are cutting out all projects started before it became a thing. Considering that libraries like this are more likely to be used for hobby projects, I'd imagine it would break quite a few - not everyone has time to be constantly updating theirs. If anything, I'd avoid breaking any existing code unless we have a really strong reason to! Also, while I agree that C++20 is a decent baseline for new projects, modules are definitely not there yet:
Yes. If I were to start writing this thing today, I'd not use a macro for that. And yes, I agree it's ugly, but I can't remember a single time it caused any trouble. 'Let sleeping dogs lie,' as they say. IMO, it's worth keeping, even if it's just for that one (hopefully not imaginary) person who still uses C++11. To summarize, here's my general stance:
So, that out of the way, are you willing to try and massage the PR in a way that would allow us to achieve what you want without doubling the codebase size? Or, otherwise, prove that it cannot be done (then we'll have to think about the least dangerous way to duplicate the sources)? If you feel like that's too much of an undertaking, that's fine. I'm willing to provide whatever help with it I can give (as long as it doesn't cost me too much time :D). Be it answering any questions you might have or investigating any blockers you run into. |
|
Specifically, I'm thinking about something like this: Can you try something like that first? It probably needs more massaging, but hopefully it does the trick. I didn't merge it then because I didn't have an easy way of testing modules yet (gcc-16 it was targeting still needed to be built from source, if I recall correctly). But, if it works for you, maybe I can try again. Surely the situation is better now that it was a year ago when I wrote that. |
|
Closing this, and I will make a new pull request when I'm done. |
Changes
vkfw.cppmno longer includesvkfw.hpp. This means the code is duplicated, but I think this is the way forward, since it both fixes a lot of errors and warnings, and also this bumps up the minimum C++ standard forvkfw.cppmto C++20, which could allow a lot of cleanup.Checked on MSVC. I'd be grateful if someone checks on GCC/Clang, but I don't see a way this could go wrong anyway.