Skip to content

Configurable Chase Camera#188

Merged
jason-watkins merged 10 commits intonasa:developfrom
flyingk:camera_PR
Nov 15, 2019
Merged

Configurable Chase Camera#188
jason-watkins merged 10 commits intonasa:developfrom
flyingk:camera_PR

Conversation

@flyingk
Copy link
Copy Markdown
Contributor

@flyingk flyingk commented Nov 6, 2019

This PR extends the previous runway camera contribution by adding an option to position the chase camera relative to the aircraft such that it moves with it and points at the specified direction in local aircraft axes.

This allows to quickly switch pre-programmed camera angles of the flying aircraft.

A new file was added to group the camera callbacks.

@jason-watkins
Copy link
Copy Markdown
Contributor

It looks like you're trying to include the C client header in one of the plugin source files... Was that intentional for some reason?

Looks good otherwise. I'll do a detailed review once all of the builds are working.

@flyingk
Copy link
Copy Markdown
Contributor Author

flyingk commented Nov 13, 2019

I am using the enum VIEW_TYPE, so to ensure consistency I included the client header. How would you propose to do it to ensure compatibility? Obviously, the enum can be copied into the plugin header, but then changes have to be applied in both locations.

@jason-watkins
Copy link
Copy Markdown
Contributor

Ah, I see. I'd prefer not to do that for two reasons. One, I just don't like the cross dependency from a build perspective. Second, I like the plugin code to stay C++, and I hate C-style enums in C++ code.

I don't think there's any real problem with copying the enum over to the plugin header (and making it an enum class in the process), especially since we at least nominally have to keep all of the clients up to date so reusing the C client enum doesn't actually solve the issue of consistency completely.

@flyingk
Copy link
Copy Markdown
Contributor Author

flyingk commented Nov 14, 2019

Here is the msg definition for the extended camera for the wiki:
xplane_camera_msg.xlsx

Copy link
Copy Markdown
Contributor

@jason-watkins jason-watkins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of style nitpicks. Looks ready to go as far as functionality goes.

@flyingk
Copy link
Copy Markdown
Contributor Author

flyingk commented Nov 15, 2019

this all works fine on Mac, so #75 can be closed as well.

@jason-watkins jason-watkins merged commit c0abcf7 into nasa:develop Nov 15, 2019
@jason-watkins
Copy link
Copy Markdown
Contributor

Looks good. Thanks as always for the contribution!

@flyingk flyingk deleted the camera_PR branch November 16, 2019 09:40
@jason-watkins jason-watkins mentioned this pull request Jun 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants