I hacked in a spectator-like follow mode #115
Conversation
There was a problem hiding this comment.
Funnily enough, I was just about to look into working on this feature myself, before I found your PR. 😂
I've given it a quick test and had a quick glance over the code. It's working very well as an initial proof of concept.
Other than code cleanup/refactoring, I personally would take a different approach to the keybinding. IMO it makes sense to re-use vanilla's "Pick" keybind (middle mouse) rather than adding a new freecam-specific one.
This would mean "follow" is always activated from within freecam mode, and we'd have to think about how to exit follow mode; perhaps the freecam keybind or using the "pick" bind would exit?
Even if we don't go with my approach, I still think the "follow" and "followSet" binds should be combined somehow.
Hopefully @hashalite is happy to include a follow/spectate mode. If I get chance I may look into working on this myself too, although I have other stuff I want to look at too, so who knows.
| followBind = KeyBindingHelper.registerKeyBinding(new KeyBinding( | ||
| "key.freecam.follow", InputUtil.Type.KEYSYM, GLFW.GLFW_KEY_UNKNOWN, "category.freecam.freecam")); | ||
| setFollowBind = KeyBindingHelper.registerKeyBinding(new KeyBinding( | ||
| "key.freecam.setFollow", InputUtil.Type.KEYSYM, GLFW.GLFW_KEY_UNKNOWN, "category.freecam.freecam")); |
There was a problem hiding this comment.
Personally, I'm in favor of hooking into vanilla's Pick Block action instead of adding our own keybind, however if we do add a custom bind I'd rather it be one single bind than two separate ones.
Also, the keybind translation key should be added to the lang file.
| private static boolean freecamEnabled = false; | ||
| private static boolean tripodEnabled = false; | ||
| private static boolean followEnabled = false; |
There was a problem hiding this comment.
The isEnabled method should also check this. Currently various mixins are broken, e.g. "Show Player".
| return follow; | ||
| } | ||
|
|
||
| public static void toggleFollow() { |
There was a problem hiding this comment.
toggleFollow might benefit from being split into enableFollow and disableFollow methods?
| if (followEnabled) { | ||
| freeCamera.applyPosition(new FreecamPosition(followMe)); | ||
| followEnabled = false; | ||
| } |
There was a problem hiding this comment.
This should use FreecamPosition.getSwimmingPosition() instead of new FreecamPosition(). (Currently activating freecam from follow mode will position the camera at the feet instead of at the head of followMe.)
It may be cleaner to remove the public FreeCamera(int id) constructor entirely and instead do something like:
freeCamera = new FreeCamera(-420, FreecamPosition.getSwimmingPosition(
followEnabled ? followMe : MC.player
));There was a problem hiding this comment.
Also, calling onEnable() conditionally and then manually disabling followEnabled without any sorta disableFollow method feels a bit messy.
|
I like what's been done here, thank you! I agree with what @MattSturgeon has said. I think the two keybinds should be replaced with the default pick block bind. I think it feels more intuitive and it uses less space in the controls menu. followEnabled definitely needs to be checked by isEnabled, too. The other style-related changes would also be appreciated. |
Is this kind of thing something that you'd consider to merge? I'd be willing to put in some work to make it nicer, this is just a simple hack that works for me™️