search

horde3d / Horde3D

Horde3D is a small 3D rendering and animation engine. It is written in an effort to create an engine being as lightweight and conceptually clean as possible.
http://horde3d.org/
1.55k stars 308 forks source link

Develop #223

Closed gwald closed 10 months ago

gwald commented 10 months ago

removed global's replaced with #defines

horde3d commented 10 months ago

Why do you think defines are better than globals?

algts commented 10 months ago

Can you please provide a short sample where "static H3DNode causes multiple definitions"? Thanks. The only problem with defines is that we lose type information which may lead to problems in some cases.

gwald commented 10 months ago

These two global are defined as consts, I don't know about C++ but in C they cause multiple definition, I would think it's the same in C++.

I'm not a C++ programmer but in C, static would instantiate these two variables for each compiled file .c they are included into, it wouldn't make them global.

I dont know why they have to be globals and not just #define or enums? If they have to be global wouldn't they be in the engine with functions to set them?

@algts This is just the C binding, I dont think type information would matter? maybe in C++, I dont know. Also I cant find any reference to either H3DRootNode or H3DUTMaxStatMode in the new C binding.

horde3d commented 10 months ago

I see, there is indeed a difference between C and C++ here. But we could also use enums instead of defines. In general I personally don't like defines since they can be redefined. But I'm also fine with the define statement. I leave it up to algts

algts commented 10 months ago

Well, I think its ok if it only changes the C binding (defines are normal in C world). I'll pull it now.

gwald commented 10 months ago

If you guys are changing these two conts in the C++ bindings to enums, then the C binding can also be an enum, it just has to be fully qualified in the enum, ie:

typedef enum H3DDeviceCapabilities
    {
        H3D_DeviceCapabilities_GeometryShaders = 200,
        H3D_DeviceCapabilities_TessellationShaders,
        H3D_DeviceCapabilities_ComputeShaders,
        H3D_DeviceCapabilities_TextureFloatRenderable,
        H3D_DeviceCapabilities_TextureCompressionDXT,
        H3D_DeviceCapabilities_TextureCompressionETC2,
        H3D_DeviceCapabilities_TextureCompressionBPTC,
        H3D_DeviceCapabilities_TextureCompressionASTC
    }H3DDeviceCapabilities;

If not, then define's in C is perfectly fine IMO anyway.