search

fishfolk / jumpy

Tactical 2D shooter in fishy pixels style. Made with Rust-lang 🦀 and Bevy 🪶
https://fishfolk.org/games/jumpy/
Other
1.65k stars 121 forks source link

feat: Cannon re-implement #965

Closed DRuppFv closed 5 months ago

DRuppFv commented 5 months ago

This PR closes #576.

I started this as draft because I think the team feedback is important before I head to writing the cannon itself.

There are many .yaml fields that are used only when holding the kickbomb, and I wonder if I should delete these from the cannonball (as it will only work at the lit state) or just leave it there because the game would just crash if the cannonball is used as non-lit.

I also used .clone() while dealing with SVec<> and animations in kick_bomb.rs because I think it was the only option, but using .clone() is quite inefficient so I wonder if there's another way of doing that.

MaxCWhitehead commented 5 months ago

There are many .yaml fields that are used only when holding the kickbomb, and I wonder if I should delete these from the cannonball (as it will only work at the lit state) or just leave it there because the game would just crash if the cannonball is used as non-lit.

Not sure exactly but good question. What do you mean by the game will crash if cannonball used as non-lit? Not suggesting this is untrue just not clear to me where exactly this would crash. Generally, it's best not to crash unless in a very unrecoverable situation or really want to draw attention to a problem that cannot be handled in a reasonable way. Otherwise logging warnings or errors to indicate an issue and trying to continue game is usually best course of action.

I also used .clone() while dealing with SVec<> and animations in kick_bomb.rs because I think it was the only option, but using .clone() is quite inefficient so I wonder if there's another way of doing that.

I wouldn't worry too much about the clone - should only be when setting up new cannon ball, and it's not a lot of data to clone. While not the perfect solution, but this is simple at least and doing something else is probably not super easy.

DRuppFv commented 5 months ago

What do you mean by the game will crash if cannonball used as non-lit? Not suggesting this is untrue just not clear to me where exactly this would crash. Generally, it's best not to crash unless in a very unrecoverable situation or really want to draw attention to a problem that cannot be handled in a reasonable way. Otherwise logging warnings or errors to indicate an issue and trying to continue game is usually best course of action.

Oh, I think there was a misunderstanding, by "crash" I just meant it would get an error - which I'm sure is 100% recoverable. Yeah I will delete the fields, it just felt weird writing cannonball using kick_bomb as a base while I can't use all kick_bomb functions.

I wouldn't worry too much about the clone - should only be when setting up new cannon ball, and it's not a lot of data to clone. While not the perfect solution, but this is simple at least and doing something else is probably not super easy.

I see, I am gonna keep it as it is. Thank you for the feedback.

MaxCWhitehead commented 5 months ago

The cannon is awesome. Is an exciting weapon, great work!