Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split Player Camera from Debug Camera and Restore G3D Scene Editor Functionality #118

Merged
merged 10 commits into from
Jun 25, 2020

Conversation

bboudaoud-nv
Copy link
Collaborator

@bboudaoud-nv bboudaoud-nv commented Apr 20, 2020

We have been using activeCamera() for our player camera, which when defaulting to a debug camera removes the ability to use the developer window/F2 to move to a free-moving camera in the scene. Here we add in a playerCamera intended to split this functionality again.

All player-based actions (aside from waypoint editing) now use the player camera, leaving the debug/additional cameras free for future implementation.

Merging this closes #116.

@bboudaoud-nv
Copy link
Collaborator Author

Once complete, this should resolve issue #116. Still some work to do in getting a FirstPersonManipulator to control the debug camera.

@bboudaoud-nv
Copy link
Collaborator Author

Upon discussion w/ @morgan3d I think we may not be using the FirstPersonManipulator as intended. It seems we should consider making some changes here/that G3D may make some changes to make it more explicit the default FirstPersonManipulator provided by GApp should only be used with the debug camera.

This (likely) means we should be constructing our own manipulator to use in order to switch between mouse-as-cursor and mouse-as-camera control mode.

@jspjutNV
Copy link
Contributor

Oh, okay, so we should do a FirstPersonManipulator::create() call somewhere and keep that along side the default one provided by GApp? I assume there should also be changes made to the input handling to change the routing of input depending on which camera/manipulator is active.

@jspjutNV
Copy link
Contributor

It's been a while since we've seen an update on this pull request. Should we include this in the v20.06.01 release milestone, or is there still enough work to do here that we defer this one?

@bboudaoud-nv
Copy link
Collaborator Author

bboudaoud-nv commented Jun 10, 2020

This issue should be closed. I can look into creating a new FirstPersonManipulator to avoid overriding the debug camera one.

On second thought I may need some assistance with understanding how this should be done. I believe "just" creating a new FirstPersonManipulator won't do what we are looking for here.

@bboudaoud-nv bboudaoud-nv added this to the v20.06.01 milestone Jun 10, 2020
@jspjutNV jspjutNV modified the milestones: v20.06.01, v20.next Jun 12, 2020
@jspjutNV
Copy link
Contributor

This latest merge broke the testing. We should make sure there's no regression on testing before we consider this pull request ready.

@bboudaoud-nv
Copy link
Collaborator Author

I got the FPM moving the debug camera (we didn't copy over this line from the GApp::onSimulation() call into our FPSciApp::onSimulation() (which doesn't call GApp::onSimulation(), but instead reproduces a subset of the calls from within it).

This PR can now be reviewed.

@bboudaoud-nv bboudaoud-nv marked this pull request as ready for review June 22, 2020 14:54
@jspjutNV
Copy link
Contributor

The same 4 tests that are passing in master are passing again with the latest commit.

@jspjutNV
Copy link
Contributor

I believe this is ready to merge with the exception of the crash that is now documented in #177 . This bug probably isn't added by this pull request and instead is likely a bug that was always present but wasn't visible because the larger bug this pull request solves was hiding it.

@jspjutNV jspjutNV merged commit 1cec13d into master Jun 25, 2020
@jspjutNV jspjutNV deleted the PlayerCamera branch June 25, 2020 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The DebugCamera should be independently controllable
3 participants