-
Notifications
You must be signed in to change notification settings - Fork 1
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
Multiplayer #10
Multiplayer #10
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yay! Kudos for adding to the docs as well :) I have one nitpick about input map stuff, and I don't love how the Controls text is, but I realize there isn't much we can do here without a bunch of extra work.
hud.tscn
Outdated
Player left: WASD | ||
Player right: Arrow keys |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the "player left" / "player right" scheme here, but it's awkward in the default case where we have a single player node. (Am I left or right? I have no idea!). I'd be interested if anyone has an alternative scheme that works better, but with a similar bit of clarity over "player 1" / "player 2"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, Player 1 / Player 2 is better.
scripts/player.gd
Outdated
"jump": "ui_up", | ||
"left": "ui_left", | ||
"right": "ui_right", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we're defining player 2 actions in the input map, we should define player 1 actions in the input map as well instead of using the built in ui actions. At the moment I can go to project settings and customize those player 2 controls (for example adding controller buttons), but I can't do the same for player 1 controls.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, let's do that. I was only copying Pong for consistency, but I can update both. The good thing about builtin actions like "ui_up" is that they are already mapped for joysticks. I will see if I can do the same for player 1 / player 2.
Use custom actions for player 1 instead of the builtin actions. Copy the current builtins that has not only keyboard, but also joystick support. Add actions for player 2. Use WASD keys for keyboard, and the same for joystick as player 1, but for device number 1. #8
This property tells which player controls the character. It can be player one, two or both. Add a constant dict holding the mapping from player to input actions. Add helper functions to Action.just_pressed(), Action.just_released(), Action.get_axis() that consider when both players control this character. #8
Using the current label in the center. Nothing fancy.
Add a global group "players". Add the player scene to it. Change the enemy collision and falling platform collision to use that player instead of the node name. #8
Thanks @dylanmccall . Ready for review again!
I realize that it may be confusing to have a Player property in a Player scene instance. The scene came first, but it really should be renamed to "PlayerCharacter", "Avatar" or something. If this gets merged, I will do the same thing in Moddable Pong. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! LGTM!
Notes:
#8