search

Alzter / TuxBuilder

A Godot re-implementation of SuperTux
GNU General Public License v3.0
212 stars 28 forks source link

Added proper explosion to Mr.Bomb #48

Closed Nolkaloid closed 2 years ago

Nolkaloid commented 5 years ago

Improved Mr.Bombs animations and changed the Control wrapper to a Node2D https://cdn.discordapp.com/attachments/396022213100437504/632560306748391455/bomb.mp4

skyace65 commented 5 years ago

Ok, a few things. You need to undo the change of the Control node to a Node2D. It's how squishing is done, I know it might not be an ideal solution but that's the way it works. And you need to explain why every change in a PR was done, why was rect_scale changed to scale in a few scripts?

Also, undo the Sawblade.tscn and Coin.tscn changes, the file changes in those aren't actually meaningful. Godot files keep track of what frame an animation was on when a scene was last opened. It's not actually something that needs to be committed in a PR.

I'll get Alzter to look over the bomb changes later.

Nolkaloid commented 4 years ago

@skyace65 Using a control node for squashing is wrong, the scale is not smooth this way and you have less control over the look of it. That why rect_scale was changed in the scripts because Node2D uses scale instead of rect_scale. For the coin and the sawblade I just wasn't careful when committing in GitKraken.

EDIT: I understand that the Control node is used to have the scaling origin at the bottom, and by replacing it with the Node2D the origin is in the center. So my proposition would be to unstage this change from this PR and make a other with a reinvented squashing system using proper nodes. Continuing to use the Control node may lead to complications later. :)

skyace65 commented 4 years ago

@Nobelix That sounds good to me.