Skip to content

Combo demo#24

Open
NathanLovato wants to merge 20 commits into
mainfrom
combo-demo
Open

Combo demo#24
NathanLovato wants to merge 20 commits into
mainfrom
combo-demo

Conversation

@NathanLovato

Copy link
Copy Markdown
Contributor

No description provided.

@NathanLovato NathanLovato left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The main issue I have with this code is how it relies on individual variables to handle combos. It is functional but does not teach our students something very useful for them.

We should at least show an approach that is somewhat scalable or applicable to the kinds of games that people want to make.

Comment thread combo-demo/player/Player.gd Outdated
Comment on lines +14 to +15
var _previous_light_combo := "light_combo_3"
var _previous_heavy_combo := "heavy_combo_2"

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This isn't great. I'd rather have something a bit more of a system that makes editing combos efficient. An array of attacks you used as part of a combo would be preferable

Comment thread combo-demo/player/Player.gd Outdated
Comment on lines +90 to +91
func set_accept_next_attack() -> void:
_accept_next_attack = true

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If you're going to have a setter function, you might as well make the property public, use setget, and give the setter function a default argument of true.

func set_accept_next_attack(value := true) -> void:
	accept_next_attack = value

Otherwise you're making a function that works differently from most functions named set_*

Comment thread combo-demo/player/Player.gd Outdated
Comment on lines +21 to +58
func _unhandled_input(event: InputEvent) -> void:
if event.is_action_pressed("light_attack") and _accept_next_attack:
_previous_heavy_combo = "heavy_combo_2"

_state = States.LIGHT_ATTACK
_accept_next_attack = false

_animation_player.stop()
_animation_player.play("idle")

match _previous_light_combo:
"light_combo_1":
_animation_player.play("light_combo_2")
"light_combo_2":
_animation_player.play("light_combo_3")
"light_combo_3":
_animation_player.play("light_combo_1")

_previous_light_combo = _animation_player.current_animation
return

if event.is_action_pressed("heavy_attack") and _accept_next_attack:
_previous_light_combo = "light_combo_3"

_state = States.HEAVY_ATTACK
_accept_next_attack = false

_animation_player.stop()
_animation_player.play("idle")

match _previous_heavy_combo:
"heavy_combo_1":
_animation_player.play("heavy_combo_2")
"heavy_combo_2":
_animation_player.play("heavy_combo_1")

_previous_heavy_combo = _animation_player.current_animation
return

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is the code I think we should change the most about this demo because it's very hard-coded.

It is completely fine in the context of a game where all you're going to have is one or two combos. But as we make these demos for educational purposes, we have to design them with what our students want to learn in mind.

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.

2 participants