Attack hitbox and startup frame refactor

[odecay]

Closes both
#53 #54

Collisions have been refactored to take place in one system only, the filtering we need is already done via BodyLayers.
There are now two main types of attacks.

1. Melee attacks: these are spawned as a child of the fighter who starts them, with an AttackFrames component which consists of the startup active and recovery frames of the attack. These correspond to the frame window in which a hitbox is spawned, which will collide with other fighters as defined by its collision layers. Melee attacks are not despawned when they collide with something, instead they stay active for the duration of their Active Frames. There are systems which activate the hitbox at the start of the Active Frames and despawn the attack at the end of the Active Frames/Start of the Recovery Frames. These attacks also lock the fighter into the Attacking state for the duration of their attack animation.

2. Projectile attacks: these are spawned un-parented, and with a ProjectileLifetime component. They are despawned on collision, or upon reaching the end of their lifetime. Currently only the player has these, and they do not lock the player into an attack animation, although I think ultimately both of these things will change, players and enemies will both want projectile attacks and they will be tied to either character specific special attacks or Ranged Weapon animations.

Things to consider for this PR:

• Currently there may be issues with the exactness of frame timing due to commands being used to activate hitboxes and the need for commands to be separated into stages to apply. This should probably be addressed after Solve commands flushing (1-frame-bug related) issues #99, but I am confident that these frame based definitions for attacks are the correct design. I tried the approach of changing adding the hitbox component at attack spawn time with a 0 sized shape, and mutating to the correct shape on Active frame start to avoid command application, but this did not behave as expected.
• I may think further about the startup, active, and recovery attributes of the AttackFrames component. I want to stick with these names but I see the way they are referenced currently could be confusing, currently they are the index relative to the animation on which the startup, active, and recovery portions of the attack end. I also considered representing them as ranges, but it seemed like an index was simpler.
• I have refactored some of the work done in Weapons refactoring #123 and that lead to some renames from what was added by @64kramsystem. I think I will take another pass tomorrow to check but I am quite confident that the conceptual model of Melee and Projectile attacks is cleaner for our purposes here. (Some of what that rework took into account was due to the way flop worked before but it is now unified with other Melee attacks)
• I have left some unnecessary comments which I will come back and clean up before merge.

Further improvements

• Define attack startup frames, hitbox size and placement via transform relative to parent in fighter yaml files.
• Write up further design goals and spec for attacks.
  (I intend to do both of these)

[zicklag]

I did a quick, light review and it looks good to me.

I like that we've moving things into a more generic Projectile/Melee attack systems with less item specific systems. 👍

[64kramsystem]

Looks good to me 😄

[64kramsystem]

Possibly! It depends on what you have in mind 😄

Maybe adding a system that matches the current on-screen animations? I haven't checked the system, in this case, they'd need to be connected to a state.

[odecay]

Not sure I have anything in mind yet, just noting that it feels a bit awkward that it lives here, I think this might make sense to either trigger based off attack actionlike state or when we split out the movement intent from attacks also use a similar strategy to split out the sound triggering intent of attacks.

[64kramsystem]

Difficult to say 🤔 The reason is that this is already a decoupled logic - FighterStateEffectsPlayback does not play any effect itself, but it adds a component, in turn this component is picked up by a separate system (fighter_sound_effect).

Having said that, I definitely find ugly the surrounding conditionals. They can be offloaded to the fighter_sound_effect system, by passing the Handle<FighterMeta>, so that the audio system can find out the asset by itself. On the other hand, this design leads to one system is saying to another "here's the metadata; maybe you'll play a sound, maybe you won't". This can be fine, it's a matter of taste.

Let me know if you have any preference, since I think all the approaches are equally valid.

[odecay]

bors +r

[bors]

Did you mean "r+"?

[odecay]

Did you mean "r+"?

Did I ? 😆

[odecay]

bors merge

[bors]

Build succeeded:

• Check Rust formatting
• Clippy correctness checks

[zicklag]

Haha, check it out, bors actually merged both of our pull requests into a single merge commit because we tried to merge it at the same time. That's cool. :)