Add Battlers and Combat logic #218
Conversation
|
Thank you very much for your work! I checked and recognized some code patterns, cool! Regarding the two differences:
I'm taking note to download and review this thoroughly this week. Do you have a Twitter account? I'll post about this, and I would like to give you a shout out. |
|
Hey, thanks! Looking forward to the feedback. The BattlerActions as Node or Resource have tradeoffs, I'm not sure which I prefer at the moment. I guess that will become more apparent as concrete actions are added. I'm picturing a "projectile spawner" that will allow animating projectiles, impacts, and other VFX, but I'm not sure yet if that's the best option. Hard to tell until we get there.
Pretty sure it's something like @_food_please. Thanks again and, as always, please don't feel the need to rush through it. |
src/combat/active_turn_queue.gd
Outdated
| player_turn_finished.connect(func _on_player_turn_finished() -> void: | ||
| if _queued_player_battlers.is_empty(): | ||
| _is_player_playing = false | ||
| else: | ||
| _play_turn(_queued_player_battlers.pop_front()) | ||
| ) | ||
|
|
||
| for battler: Battler in _battlers: | ||
| battler.ready_to_act.connect(func on_battler_ready_to_act() -> void: | ||
| if battler.is_player_controlled() and _is_player_playing: | ||
| _queued_player_battlers.append(battler) | ||
| else: | ||
| _play_turn(battler) | ||
| ) | ||
| battler.health_depleted.connect(func on_battler_health_depleted() -> void: | ||
| if not _deactivate_if_side_downed(_party_members, false): | ||
| _deactivate_if_side_downed(_enemies, true) | ||
| ) |
There was a problem hiding this comment.
This is the main area where I made changes in this first refactor commit. You can simplify the code and group it by using first class functions. Back in Godot 3, you had to separate the signals and the functions called from the signals. If you needed to pass extra data to the signal callback, you had to bind arguments when connecting the signals.
You can still do that in Godot 4 and it's completely fine, but you can also use first-class functions to create closures and capture the data you need. The thing I like most about this is that now the signal connection and the callback function are not on either end of the script, but they are grouped in one place. This makes it easier to understand the code and see what is connected to what.
|
I started reviewing and refactoring little bits to prepare for a merge. You may expect quite a few code changes, as I'm going to be moving things around, but most changes will likely be superficial (small cleanup like e.g. renaming vars to be pseudo-private or not). I'll leave comments on the more interesting changes, if any. |
|
@food-please do you want me to merge now or is there more you want to add to this PR? |
|
All good overall! Like the rest of contributions, I think it'd be best to merge now and see if things should be modified later once the demo/project is fully fledged. |
I think this is a good point to merge the changes. The next pass will probably handle UI/player input and should be sufficiently different. Unless you see any glaring issues with design or implementation I'm happy to merge what we have now. I'm open to refactoring down the road.
That's great! I'm not fully happy with some of the paradigms that I had included here and I always benefit from learning from the revisions. If I were to re-approach the BattlerAnim class, I think that I would probably have Battler call its methods directly rather than respond to signals. Perhaps the existing approach will make more sense once the UI changes are in. |
|
Merged! Thank you very much for your contribution and continued participation in this project. Regarding battler animations, you could expose more functions but also because animations may have any length you will likely always need a few signals in there. It's the kind of thing I wouldn't necessarily work on now because you have to have all the systems in place and an actually animated character to really nail the code. There are other things like the stats with modifiers that, looking at the code now, I would like to either simplify (remove code or make it more specific) or solidify (type more strongly and keep track of modifiers more easily). But before that, you want to have the full demo to make the most informed decisions possible as to what makes sense to keep in the final project. |
Another PR looking to address the Combat issue.
This removes placeholder assets in the combat state and replaces them with Battlers and a handful of nodes needed to get them on the screen.
This should also add logic to the combat, transitioning cleanly to and from combat depending on which team wins the combat. Other elements that are included are an active turn queue (which orders how Battlers act in case of overlap) and BattlerActions which animate/apply effects/are chosen by battlers whenever it is their turn. Consequently, combat now doesn't end on keypress but rather according to which 'team' won the combat.
With regards to implementation most cues were taken from the 2D Secrets course. Several of the commits have a lesson name attached, as laid out on gdquest.com. Of course, this project has translated these to Godot 4.X.
I've axed the formulas class, since I don't see that as being immediately useful. It may work its way back in, if needed.
Of particular note also is the structure of actions. In the 2D Secrets course, there were ActionData, Actions, and then derived ActionData (e.g. AttackActionData) and derived Actions (e.g. AttackAction). This meant that you had to a) duplicate your efforts in maintaining code and b) match the correct data to the correct action type at runtime. Perhaps this wasn't a big issue in practice, but I found this really hard to wrap my head around.
In this iteration of the project, actions are simply a Resource. This has a couple of benefits:
I'd love to have your feedback on this design, since it is a deviation from what was there in the course. All action and battler implementations have been put into the new /battler folder, though their BattlerAnimations co-exist with the assets in the /Assets folder (is there a best-practices issue here? I'm open to moving things around). The main issue I can see is that it's tricky to lay out/test Action animations, but I can't think of a super easy way to set that up anyways since it's all variable depending on where Battlers are positioned on the combat scene.
This PR is ready for review and the feedback/editing cycle.