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

Feature/demo platformer 2d #58

Merged
merged 12 commits into from
Nov 18, 2019
Merged

Feature/demo platformer 2d #58

merged 12 commits into from
Nov 18, 2019

Conversation

cogwoz
Copy link
Contributor

@cogwoz cogwoz commented Nov 10, 2019

Hello!

I'm in the middle of refactoring the platformer demo. You can see a todo list and progress in #57

I'd like some advice on some things before cracking on again.

  1. For the Player and Enemy, should I proceed with a StateMachine as in the Hook demo?
  2. I'm having trouble with the moving platforms. To align the Player with them, sync_to_physics seems to be necessary but it's not usable with move_and_slide so I'm unsure how to do this properly.

Please also offer any other feedback. This is also a learning experience for me as I learn to stick to the guidelines 😄

Cheers!

@cogwoz cogwoz added the enhancement New feature or request label Nov 10, 2019
@cogwoz cogwoz self-assigned this Nov 10, 2019
Copy link
Contributor

@NathanLovato NathanLovato left a comment

Choose a reason for hiding this comment

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

Quick review on a handful of points. I don't know if you changed some code already but looking at it, some doesn't look like code from the official demos.

I don't think a lost/win overlay is necessary. Having just one level with the playground is enough. The goal of these demos isn't to have full games I think, but to show how a feature works, or how to implement a mechanic, or in this case the core loop of a specific type of game. Doing more will make the demos overlap and use a lot more production + maintenance time.

General notes:

  • We get most nodes using the onready keyword. The goal is to have all dependencies at the top of the scripts.
  • You don't need casting that way
  • Avoid shorthands in variable names, e.g. favor animation over anim

platformer-2d-rework/src/Objects/Coin.gd Outdated Show resolved Hide resolved
platformer-2d-rework/src/Objects/Coin.gd Outdated Show resolved Hide resolved
platformer-2d-rework/src/Objects/Coin.gd Outdated Show resolved Hide resolved
platformer-2d-rework/src/Actors/Enemy.gd Outdated Show resolved Hide resolved
platformer-2d-rework/src/Actors/Enemy.gd Outdated Show resolved Hide resolved
platformer-2d-rework/src/Actors/Player.gd Outdated Show resolved Hide resolved
platformer-2d-rework/src/Actors/Player.gd Outdated Show resolved Hide resolved
platformer-2d-rework/src/Actors/Player.gd Outdated Show resolved Hide resolved
cogwoz and others added 3 commits November 11, 2019 12:36
The issue was that the floor's velocity (platform here) is only transfered to
the character if move_and_slide_with_snap's stop_on_slope argument is `false`.

stop_on_slope seems to stop the character's movement if it has no horizontal
velocity and the floor's normal is < a threshold angle to the floor_normal
parameter, even if it's horizontal in our case.

To preserve stop_on_slope on slopes, I've introduced a Raycast2D that only
detects platforms.
@NathanLovato
Copy link
Contributor

Just fixed the moving platform issue. Here's the explanation:

The issue was that the floor's velocity (platform here) is only transferred to
the character if move_and_slide_with_snap's stop_on_slope argument is `false`.

stop_on_slope seems to stop the character's movement if it has no horizontal
velocity and the floor's normal is < a threshold angle to the floor_normal
parameter, even if it's horizontal in our case.

To preserve stop_on_slope on slopes, I've introduced a Raycast2D that only
detects platforms.

@NathanLovato
Copy link
Contributor

Regarding the code style/guidelines, would you like me to do e.g. part of a script, or part of the project to give you a headstart + example? The code has some no-nos to me at this point.

@cogwoz
Copy link
Contributor Author

cogwoz commented Nov 11, 2019

Thanks for that Nathan.

There are some parts of the code that are left over from the original demo that I haven't touched yet. I was planning on going over the scripts in more detail after everything was working using the guidelines but an example would be helpful if you have time.

@NathanLovato
Copy link
Contributor

There you go, I just rewrote the code for the Player script.
There's more refactoring to do to show good practices: the physics layers and masks are a mess, so the original code relied on things like having the bullet get its parent and making it an exception for collisions.
Anyway, the player is the most complicated bit here. from there, I think we could make some more gameplay improvements, but it'll be part of the polishing phase.
I'd port all the code to fit the guidelines at this point.

Regarding moving platforms, I'd also remove the code to use an animation player too, as that way, you can directly move the platforms in the editor and even use the new animation autokey feature to make the platforms move exactly where you want.

@cogwoz
Copy link
Contributor Author

cogwoz commented Nov 12, 2019

That's great thanks for that! I'll work on the rest of the scenes today.

@cogwoz cogwoz requested a review from NathanLovato November 17, 2019 15:15
@cogwoz
Copy link
Contributor Author

cogwoz commented Nov 17, 2019

The other scenes should be better now and I've tidied up the stage.

I wanted to use Path2D for the moving platforms as I thought moving the path in the editor would make intuitive sense but the paths stop working when the KinematicBody2D is set to sync_to_physics so I used an AnimationPlayer instead. Still sad about that one.

- Directly connect signals to target methods when possible
- Improve coin animations so the audio can't trigger twice in the editor and so
the coins' properties are reset every time you start the game
@NathanLovato
Copy link
Contributor

I just tweaked some tweaks. It's looking great to me! Do you still want to change anything or is it good to merge?

@cogwoz
Copy link
Contributor Author

cogwoz commented Nov 18, 2019

Great! If you're happy with everything then it's all good!

@NathanLovato NathanLovato merged commit 88e165c into master Nov 18, 2019
@NathanLovato NathanLovato deleted the feature/demo-platformer-2d branch November 18, 2019 13:13
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.

2 participants