-changes for API changes with regards latest SilverLining #1224
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thx - do you happen to know if these changes backwards compatible? |
|
They are not backward compatible. With older versions, you would have to
remove the last parameter passed in.
…On Sun, Oct 14, 2018, 12:28 PM Glenn ***@***.***> wrote:
Thx - do you happen to know if these changes backwards compatible?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1224 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJxd58I_5daOh7T-pTwWdrkbFr142lpIks5uk2ZCgaJpZM4XZViu>
.
|
|
At least in SilverLining 5.009 I agree that it's not compatible; and unfortunately there doesn't seem to be an easy way to do a version check with an ifdef, that I can see. That's unfortunate. |
|
The api changes add default parameters. They would be compatible if it was
not geocentric mode, which it is in the for osgearth.
Not sure whether SL version is maintained somehow in SL code. I copied
Frank on this email.
On Oct 15, 2018 5:52 AM, "Daniel Emminizer" <notifications@github.com> wrote:
At least in SilverLining 5.009 I agree that it's not compatible; and
unfortunately there doesn't seem to be an easy way to do a version check
with an ifdef, that I can see. That's unfortunate.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1224 (comment)>,
or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AJxd59QGsHIrA1Cdld1jo79VkyeN3V--ks5ulFrNgaJpZM4XZViu>
.
|
|
There is a Version.h header in SilverLining's public headers, where
SILVERLINING_MAJOR_VERSION and SILVERLINING_MINOR_VERSION are defined.
Thank you,
Frank Kane
Founder
Sundog Software, LLC
http://www.sundog-soft.com/
http://www.linkedin.com/pub/frank-kane/17/434/764
On Mon, Oct 15, 2018 at 5:56 AM Poojan Prabhu <poojanprabhu@gmail.com>
wrote:
… The api changes add default parameters. They would be compatible if it was
not geocentric mode, which it is in the for osgearth.
Not sure whether SL version is maintained somehow in SL code. I copied
Frank on this email.
On Oct 15, 2018 5:52 AM, "Daniel Emminizer" ***@***.***>
wrote:
At least in SilverLining 5.009 I agree that it's not compatible; and
unfortunately there doesn't seem to be an easy way to do a version check
with an ifdef, that I can see. That's unfortunate.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1224 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJxd59QGsHIrA1Cdld1jo79VkyeN3V--ks5ulFrNgaJpZM4XZViu>
.
|
|
Thanks. It looks like that header was added after 5.009. We're due for an update on the SilverLining version we use, this will just accelerate that. |
|
Dan, can you double check that? I seem to have version 5.005 (or 006), and I do have a Version.h file. |
|
Double checked. It's not in my Program Files's SilverLining SDK (Full source) directory. I checked Public Headers there and it's not there. I do see a Version.h in my Linux build. I did an uninstall and a reinstall. I confirmed I'm on 5.009. The Full source install does not include a Version.h. I checked Public Headers directory, which has 25 headers, none including Version.h. A full grep -r through the extracted exe in Program Files does not include the text I'll pass along a note to their team to indicate the problem. From my side I'm fine with this change here, if you don't mind trying to conditionally compile the calls to I'm guessing this is a bug with the Windows installer. I can limp along by copying the one from Linux over since they're the same version. Edit: The file is there in the 5.31 installer. Must have just been a bug for 5.009. As I indicated I'm happy to just copy over the Linux one, no blockers here on Version.h. Because the file's there in the latest download I did not bother their team with the (fixed) issue. |
|
I copied Frank on this reply.
…On Tue, Oct 16, 2018, 6:19 AM Daniel Emminizer ***@***.***> wrote:
Double checked. It's not in my Program Files's SilverLining SDK (Full
source) directory. I checked Public Headers there and it's not there. I do
see a Version.h in my Linux build. I did an uninstall and a reinstall. I
confirmed I'm on 5.009. The Full source install does not include a
Version.h. I checked Public Headers directory, which has 25 headers, none
including Version.h. A full grep -r through the extracted exe in Program
Files does not include the text SILVERLINING_MAJOR_VERSION anywhere.
I'll pass along a note to their team to indicate the problem. From my side
I'm fine with this change here, if you don't mind trying to conditionally
compile the calls to DrawObjects() and GetEnvironmentMap() with
preprocessor conditionals? We're happy to update our SilverLining version,
it's just a matter of getting the time to do so since we have to rebuild
without DX support.
I'm guessing this is a bug with the Windows installer. I can limp along by
copying the one from Linux over since they're the same version.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1224 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJxd5zsgKWwrA1cEgGnANJ8kvvyIK61Oks5ulbKbgaJpZM4XZViu>
.
|
|
It's certainly possible that there was some sort of build or installer
glitch at that time that we never knew about. If so I apologize for the
oversight.
However, I can confirm that Version.h is being generated reliably now. The
solution of just copying over the Version.h from the Linux distribution
seems fine to me. But, a conditional compile of the API changes that
depends not only on the value of our version #define's but also their
existence would do the job too. If they don't exist at all, it's safe to
say you're using a SilverLining version from before the API changes.
Thank you,
Frank Kane
Founder
Sundog Software, LLC
http://www.sundog-soft.com/
http://www.linkedin.com/pub/frank-kane/17/434/764
On Tue, Oct 16, 2018 at 7:03 AM Poojan Prabhu <poojanprabhu@gmail.com>
wrote:
… I copied Frank on this reply.
On Tue, Oct 16, 2018, 6:19 AM Daniel Emminizer ***@***.***>
wrote:
> Double checked. It's not in my Program Files's SilverLining SDK (Full
> source) directory. I checked Public Headers there and it's not there. I do
> see a Version.h in my Linux build. I did an uninstall and a reinstall. I
> confirmed I'm on 5.009. The Full source install does not include a
> Version.h. I checked Public Headers directory, which has 25 headers, none
> including Version.h. A full grep -r through the extracted exe in Program
> Files does not include the text SILVERLINING_MAJOR_VERSION anywhere.
>
> I'll pass along a note to their team to indicate the problem. From my
> side I'm fine with this change here, if you don't mind trying to
> conditionally compile the calls to DrawObjects() and GetEnvironmentMap()
> with preprocessor conditionals? We're happy to update our SilverLining
> version, it's just a matter of getting the time to do so since we have to
> rebuild without DX support.
>
> I'm guessing this is a bug with the Windows installer. I can limp along
> by copying the one from Linux over since they're the same version.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <#1224 (comment)>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AJxd5zsgKWwrA1cEgGnANJ8kvvyIK61Oks5ulbKbgaJpZM4XZViu>
> .
>
|
|
Implemented with version checking in 6a647ee |
|
Copying Frank
…On Tue, Oct 16, 2018, 3:33 PM Glenn ***@***.***> wrote:
Closed #1224 <#1224>.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1224 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AJxd5384CkAHsLCDnlYQLrENJOScfYNgks5uljRvgaJpZM4XZViu>
.
|
|
Copying Frank
…On Tue, Oct 16, 2018, 3:33 PM Glenn ***@***.***> wrote:
Implemented with version checking in 6a647ee
<6a647ee>
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#1224 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AJxd50qPx_rSyjFx9fAv-LAs2y88jVaVks5uljRwgaJpZM4XZViu>
.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi Glenn,
Here's a patch against the latest SilverLining that take into account the API changes mentioned here:
https://sundog-soft.com/2018/10/important-api-updates-in-silverlining-5-30/
Its minor, but important