tonarino / bomberman-of-the-hill

13 stars 5 forks source link

Implement bomb chain reactions #24

Closed strohel closed 3 years ago

strohel commented 3 years ago

object_despawn_system -> objects_on_fire_system and move to bomb.rs

Per discussion on review.

Split bomb_explosion_system into 2 using BombExplodeEvent

No (intended) functional change.

Let bombs explode in chain reactions

The "chained" bombs only explode in next frame, but it feels quite natural in the game. It also creates an interesting sound effect.

Play bomb spawn sound only once per tick

Little leftover TODO.

Old Discussion This is my take on #23, but I didn't get much far: bombs only explode in the next tick, while we want to whole chain reaction to happen immediately. - The chain reaction would most naturally be implemented using recursive algorithm, but I could not come up how to combine it with unique `mut bomb_query` in `bomb_explosion_system()`. Another option would be a form of fixed-point algorithm: "check bombs until there no newly exploding one." Is it possible to tell Bevy "please run this system again in this tick"? - I was initially confused that we have both `enum Object::Bomb` (and `::Crate`) and `struct Bomb`. Shall we have just `struct Bomb` and `struct Crate` (i.e. separate components)? Maybe `Object` can stay for the WASM api, but cease to be used as a component. - `object_despawn_system()` should probably only only run once per high-level tick, and not every frame? (I get `Bomb 30v73 at TileLocation(1, 16) is on fire, zeroing fuse` logs a lot of times instead of once) - `object_despawn_system()` probably belongs to `bomb.rs` and could be ranamed. Shall be easy to do, or @skywhale perhaps there was a reason to put it into `GameMapPlugin` in the first place? Based on (targets) #21.
strohel commented 3 years ago

FWIW: 4 wanderers that place bombs one after another (in time) I use for testing: players.tar.gz

skywhale commented 3 years ago
  • object_despawn_system() probably belongs to bomb.rs and could be ranamed. Shall be easy to do, or @skywhale perhaps there was a reason to put it into GameMapPlugin in the first place?

No, I was still figuring out how to organize things, and naively put it in GameMapPlugin so the spawn / despawn logic are in the same file. Please move it you like; I agree that the bomb mod may be where it belongs.

PabloMansanet commented 3 years ago

I was initially confused that we have both enum Object::Bomb (and ::Crate) and struct Bomb. Shall we have just struct Bomb and struct Crate (i.e. separate components)? Maybe Object can stay for the WASM api, but cease to be used as a component.

This is an interesting question, and one I was wondering about myself. I feel like we'll need both an Object enum type and a concrete Bomb type just because of the usefulness of With and Without query modifiers. You're right that we could do away with the enum (and use it only in the Wasm interface) but we lose the simplicity of just being able to pass the component directly to the wasm modules; we'd have to reassemble things like fuse_remaining from other components before passing it to the player.

I don't think I have a strong opinion, but I'd say there's not much harm in having marker structs (Bomb) on top of stateful types or enums (Object) and it's a pattern I've seen in some Bevy games around, for what it's worth.