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

spectate: Remove spawned check for spectator #29

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

spectate: Remove spawned check for spectator #29

wants to merge 1 commit into from

Conversation

dracc
Copy link
Owner

@dracc dracc commented Jan 31, 2019

Closes #14

Spectator lag/camera choppiness/dumbstuff is not in our reach as it's part of VC-MP itself. This PR removes the requirement for the spectator to be in a non-playing state.
Controls are, just like before, F1 to toggle spectator mode and Left/Right to browse the players.

@JayFoxRox
Copy link
Collaborator

Spectator lag/camera choppiness/dumbstuff is not in our reach as it's part of VC-MP itself.

I could imagine that the Spectate Target can also be set to Vehicles. I assume a lot of the lag happens because the player object is not updated often enough when in vehicles.


Did you verify that the actual player is not controllable while in spectate mode? If I stood next to water on my right, and I held right to switch the person I'm spectating, would I fall into the water?

}
// if(player.Spawned) {
// return;
// }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Leaving dead code is bad.

We should have some can_always_spectate <- true; (near the top) which disables this check (and is self-describing).
Alternatively, if we decide we never want to restrict spectating this way: remove this code altogether.

But commenting out unused code is bad practice.

@dracc
Copy link
Owner Author

dracc commented Jan 31, 2019

I have yet to test this change as I'm currently at work.
Spectating the players vehicle instead ought to work as the SpectateTarget takes an object as the argument. Question is, should all vehicles (and no players) be follow-able? Or should camera switch focus if the spectated player is in a vehicle? What should happen when the player enters/exits a vehicle?

@JayFoxRox
Copy link
Collaborator

Right, that'd need more testing and handlers (for always switching the target to the players active vehicle). It would be an independent change from this PR, but something to look at.
We should refactor the entire spectator script anyway as the code is very overcomplicated.

This change is still valid, but it shouldn't break game mechanics.
In singleplayer, I'm also aware of the game occasionally freezing players or slowing down their cars (braking them in the air), if the camera is far away. So if that happens while spectating someone, then we shouldn't merge this.
Collision models might also unload when switching to another player far away, so the player might fall through the ground. We need to test these things.

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.

None yet

2 participants