evilrat666 / directx-d

[DISCONTINUED] DirectX bindings for D
MIT License
20 stars 16 forks source link

Add version(Windows) #10

Closed GrimMaple closed 4 years ago

GrimMaple commented 4 years ago

If used on other platforms, mostly in cross-platform solutions, the package fails to build for obvious reasons. I have added version(Windows): to all the files to prevent it from trying to compile on anything but Windows, because it's pointless anyway. Also, dub doesn't have platform-specific dependencies, so it has to be done.

GrimMaple commented 4 years ago

@evilrat666 sorry for the ping, but this PR is quite essential to make this work in cross-platorm apps

evilrat666 commented 4 years ago

Hi. No problem, but you should use proper tool for the task.

dub ignores platform specifications for dependencies directly(that means you've likely already tried doing "dependencies-windows"), however it lets you do this using configurations, just add a new named configuration and customize relevant parameters, i.e. in your dub.json add you can add the following lines

    "configurations": [
        {
            "name": "windowsonly",
            "targetType": "executable",
            "platforms": ["windows"],
            "dependencies": {
                "directx": "~master"
            }
        },
    ],

this platforms "windows" will do the trick.

See this[1] and [2] for more info

[1] https://dub.pm/package-format-json.html#configurations [2] https://forum.dlang.org/post/n3hv0j$enb$1@digitalmars.com

GrimMaple commented 4 years ago

Are you suggesting that running dub build --config=windowsonly is better and more convenient than running dub ? This change doesn't break anything. Besides, other libs have that too: for example, other direct-x bindings https://github.com/auroragraphics/directx/blob/master/source/aurora/directx/d3d/d3dcommon.d have version(Windows):, and I would've used that if it had all the DirectX, not just d3d11 and d3d12. Moreover, even core.sys.windows.* has this. I don't see why it shouldn't be added here.

evilrat666 commented 4 years ago

Have you actually tried it? It should auto pick appropriate config without specifying which one to use.

Main reason why I would like to avoid this is that there is one possible use case for keeping it that way, is making wrapper implementation over another graphics API like ANGLE or MoltenVK does. Or maybe wine. Or even another (highly unlikely) case if MS decided to make DirectX universal API for all platforms.

GrimMaple commented 4 years ago

You are avoding a fix that doesn't break anything over a non-existent featuer that will most likely never exist in the first place. If someone is going to cross-compile for windows to generate an exe file they'll have Windows in their version too.

evilrat666 commented 4 years ago

This is a dependency management problem and I proposed a solution that deals with that and only with that. Without breaking anyone's projects. Can you confirm that the proposed solution is actually works, or fails? If it doesn't works or doesn't solves the problem, then sure, I'll merge this PR.

GrimMaple commented 4 years ago

I disagree with the proposed solution and will not test that as I have no intention of using it ¯_(ツ)_/¯

evilrat666 commented 4 years ago

After investigation on Linux I've just merged your commit and issued new release. This should solve your problem.

GrimMaple commented 4 years ago

Thank you!