SDK Question API compatibility strategy?

DarkWanderer

Active member
Orbiter Contributor
Donator
Joined
Apr 27, 2008
Messages
213
Reaction score
83
Points
43
Location
Moscow
What is the strategy for API compatibility in OpenOrbiter? Is the objective to maintain compatibility with Orbiter 2016 as much as possible, or is it possible to make breaking* changes?

Specific issues I'm looking to address are:
  • return types of some functions - int vs intptr_t, int vs size_t etc.
  • const-correctness - some functions take char* where they should take const char*
Is the strategy to preserve binary compatbility (plugins compatible as-is), code compatibility (plugins work after recompiling) or to not preserve backward compatibility at all (perhaps after some point)?
 

GLS

Well-known member
Orbiter Contributor
Addon Developer
Joined
Mar 22, 2008
Messages
5,878
Reaction score
2,870
Points
188
Website
github.com
IMO, unless there is something wrong that causes CTDs or similar, I'd "group" those changes in the x64 release, and reduce compatilibity issues for the final x86 release. This way it could save devs a recompilation for the final x86 version.
 

martins

Orbiter Founder
Orbiter Founder
Joined
Mar 31, 2008
Messages
2,448
Reaction score
462
Points
83
Website
orbit.medphys.ucl.ac.uk
Yes, essentially that. I plan to have a final x86 release that incorporates bug fixes and the handful of feature enhancements since Orbiter 2016 and retains backward compatibility as much as possible. That should happen sooner rather than later. After that, the plan is to drop the 32-bit build, drop the built-in graphics version (and all remaining dependencies to DX7), and break backward compatibility.
 

jarmonik

Well-known member
Orbiter Contributor
Addon Developer
Beta Tester
Joined
Mar 28, 2008
Messages
2,651
Reaction score
785
Points
128
Specific issues I'm looking to address are:
  • return types of some functions - int vs intptr_t, int vs size_t etc.
  • const-correctness - some functions take char* where they should take const char*

Const-correctness is kinda cool. But I have noticed that when people try to be const-correct they do the exact opposite of that. So, I would be careful with "const" declarations. If uncertain then it's better not to use "const" especially in a function declarations like this one: "virtual bool clbkBlt (SURFHANDLE tgt, DWORD tgtx, DWORD tgty, SURFHANDLE src, DWORD flag = 0) const" which is problematic when you need to call non-const member functions from a "const" one. Easily leading to chain effect needing everything to be "const" and "mutable". There are probably equally many "const" to be removed as to be added. IMHO.
 

dbeachy1

O-F Administrator
Administrator
Orbiter Contributor
Addon Developer
Donator
Beta Tester
Joined
Jan 14, 2008
Messages
9,214
Reaction score
1,560
Points
203
Location
VA
Website
alteaaerospace.com
Preferred Pronouns
he/him
Const-correctness is kinda cool. But I have noticed that when people try to be const-correct they do the exact opposite of that. So, I would be careful with "const" declarations. If uncertain then it's better not to use "const" especially in a function declarations like this one: "virtual bool clbkBlt (SURFHANDLE tgt, DWORD tgtx, DWORD tgty, SURFHANDLE src, DWORD flag = 0) const" which is problematic when you need to call non-const member functions from a "const" one. Easily leading to chain effect needing everything to be "const" and "mutable". There are probably equally many "const" to be removed as to be added. IMHO.

I respectfully disagree -- while I agree using 'const' can result in extra changes during development (e.g., the chain effect mentioned above), using const has helped me catch potential bugs as I develop, and it keeps me from accidentally modifying an object when I did not intend to. :) However, I do agree that if the developer is not sure that a public API function or parameter may not always be 'const' in the future, it's better to declare it as non-const so it won't break add-ons that call it in the future. Conversely, though, sometimes it's clear that const could (and IMO should) be used, such as:

Code:
OAPIFUNC void oapiWriteLog (char *line);

For methods like that, it's clear that the missing 'const' is just a bug, because a logging function will not (or at least should not) change the string passed to it, particularly since log messages are often just static strings (e.g., oapiWriteLog("Hello, world!")). So when add-ons need to invoke that method, they have to either cast away the constness of the string they pass (which is an ugly hack), or create a temporary non-const string on the stack and copy the message to that before sending that non-const string to oapiWriteLog (which is arguably even uglier, plus slower). In summary, working around incorrect constness in APIs when you are using 'const' in your add-on's code is a pain. And it's still perfectly fine for add-ons that do no use 'const' to pass non-const objects to API methods that take const pointers to those objects; i.e., APIs that use const do not impose any additional restrictions on add-ons that do not use const. The reverse is not true, however, because add-ons that do use 'const' have to constantly (no pun intended LOL!) work around API calls that do not use const when they could.

All this being said, oapiWriteLog is just a simple example, and other examples are not so clear-cut. For clear-cut public API methods, at least, I'm in favor of using correct constness for the reasons above. And obviously, changing an API's signature, including making a parameter pointer const, is a breaking change, which is why it can't be changed until the upcoming OpenOrbiter 64-bit version, which will have other breaking changes as well.

This is all just my perspective, though, and other perspectives are just as valid, of course. ?
 
Last edited:

kuddel

Donator
Donator
Joined
Apr 1, 2008
Messages
2,064
Reaction score
507
Points
113
I think when martin was talking about const-correctness, he means the method declaration to be const (signalling that a call to that method will not change the inner state of an object) like:
C++:
class Foo {
    // ...
    int getValue() const;
    void setValue(int value);
    // ...
};
I am all for const-correctness wherever it can be done, but sometimes it might be too much of a change (as martin said).

The const in parameter signatures however are usually a bit less of an issue - and should therefore be added wherever reasonable.
Pointers/References that are guaranteed not to be changed by the call should of course be marked 'const', like dbeachy1's example:
C++:
OAPIFUNC void oapiWriteLog (const char *line);
 

DarkWanderer

Active member
Orbiter Contributor
Donator
Joined
Apr 27, 2008
Messages
213
Reaction score
83
Points
43
Location
Moscow
virtual bool clbkBlt (SURFHANDLE tgt, DWORD tgtx, DWORD tgty, SURFHANDLE src, DWORD flag = 0) const
Out of curiosity, why would this method require non-const operations in your use case? From the signature, it should only change state of objects passed to it

(no critique, trying to understand the reasons)
 

jarmonik

Well-known member
Orbiter Contributor
Addon Developer
Beta Tester
Joined
Mar 28, 2008
Messages
2,651
Reaction score
785
Points
128
Out of curiosity, why would this method require non-const operations in your use case? From the signature, it should only change state of objects passed to it

(no critique, trying to understand the reasons)
If a "color key" is used then it will reroute the call to Sketchpad interface which is acquired by a non-const function and it needs to swap render targets and configure the render pipeline which means "object" state is changed by the call. Right now the problem is solved by adding a const version of GetSketchpad and making some variables "mutable". Which is a step towards everything "const" and "mutable" direction.

What good does the "const" do except that it can be called from an other "const" function ? Why would it need to be const ?
 
Last edited:

GLS

Well-known member
Orbiter Contributor
Addon Developer
Joined
Mar 22, 2008
Messages
5,878
Reaction score
2,870
Points
188
Website
github.com
I think the idea with const shouldn't be "let's change things to be const because...", but should be "is this changed? no? make it const!". It should be a check of the logic and not modify it.
 

kuddel

Donator
Donator
Joined
Apr 1, 2008
Messages
2,064
Reaction score
507
Points
113
const-correctness has two main purposes:
1. inform the user of a method that a call does not change the state of the object (nice to have)
2. let the compiler do more tricks in optimizing the callers code (very nice to have ;) )
 

kalral

New member
Joined
Nov 22, 2020
Messages
27
Reaction score
14
Points
3
Location
Germany
Can we also switch to using new C++ version like C++14 or C++17?
C++17, we can have string_view instead of char*.
I know nothing is stopping me from doing
C++:
CMAKE_CXX_STANDARD 17
but I am thinking more of updating the code itself.
I know that there is a talk about the Contribution Guidelines and Coding Style, would also be helpful if we have those in the git repo.
 
Top