search

godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.69k stars 528 forks source link

Use /MTd and /MDd flags on Windows dev builds #1469

Closed xissburg closed 4 months ago

xissburg commented 4 months ago

Select the debug runtime library for dev builds (i.e. when passing dev_build=yes to scons).

I had to do this change to build my extension successfully in debug mode because I link with other libraries which are built with this flag which is the default choice for debug builds. Otherwise, I get linker errors of the kind "error LNK2038: mismatch detected for 'RuntimeLibrary': value 'MT_StaticRelease' doesn't match value 'MTd_StaticDebug' in <file>.obj"

AThousandShips commented 4 months ago

This is controlled by the debug_crt option in the engine, and not dev_build, I think we should match here

bruvzg commented 4 months ago

It's definitely should be separate debug_crt, using debug CRT is rarely useful and should not be a default for debug builds.

Also, /MTd is never used by the engine (due to broken thread_local at least in some versions), but it's irrelevant what's CRT is used in the engine (GDExtension API is pure C and is not passing CRT objects), so it's should be OK to support /MTd as well.

Engine use something like:

    if env["debug_crt"]:
        # Always use dynamic runtime, static debug CRT breaks thread_local.
        env.AppendUnique(CCFLAGS=["/MDd"])
    else:
        if env["use_static_cpp"]:
            env.AppendUnique(CCFLAGS=["/MT"])
        else:
            env.AppendUnique(CCFLAGS=["/MD"])
xissburg commented 4 months ago

Thank you for clarifying. I will then recompile my libraries with /MT or /MD instead.

dsnopek commented 4 months ago

@xissburg Are you going to update this PR to match the engine (per the code @bruvzg shows above)? Or, are you simply going to switch to not using /MDd and so this PR can be closed?

xissburg commented 4 months ago

@dsnopek I'll close since this does not seem to be an issue to nobody else since it hasn't been brought up before apparently. I'll adopt the same practice used by the engine.