-
-
Notifications
You must be signed in to change notification settings - Fork 492
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tux emits bubbles while being underwater #3014
Conversation
I think this is a pretty nice feature, but is it possible that the air_bubble-*.png files are missing? |
Oh you're right, I forgot to push the bubbles. |
Looks pretty nice so far. Some comments from my side:
|
I like the effect of them "popping" in the air. And for the bubbles not being emitted at the right place, I'm aware - but I don't know what to do about it - sometimes the sprite rotation gets registered improperly - At-least that's what I've observed. |
Some suggestions:
|
Thanks for the suggestions! |
No problem. I really like this feature, but I think there are two small tweaks to be done:
It would be great if you could have a look at these two things. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approve on account of code style. Haven't done any actual testing in-game, though.
@bruhmoent Why did you not use the .sprite file I send you? |
Also, I think the bubbles vertical speed could be increased by quite a bit |
I just used the images - I found the .sprite file to be unnecessary? Idk how does it matter really |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace all glm::vec2
s with Vector
s
Despite what I mentioned. It looks great! Thanks for your contribution!
src/object/player.cpp
Outdated
glm::vec2 beak_position; | ||
|
||
// Determine direction based on the radians | ||
if (m_swimming_angle > static_cast<float>(M_PI_2) && m_swimming_angle < 3.0f * static_cast<float>(M_PI_2)) // Facing left |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use math::PI_2
if (m_swimming_angle > static_cast<float>(M_PI_2) && m_swimming_angle < 3.0f * static_cast<float>(M_PI_2)) // Facing left | |
if (m_swimming_angle > static_cast<float>(math::PI_2)) && m_swimming_angle < 3.0f * static_cast<float>(math::PI_2)) // Facing left |
src/object/player.cpp
Outdated
m_bubble_particles[1] = Surface::from_file("images/particles/air_bubble-2.png"); | ||
m_bubble_particles[2] = Surface::from_file("images/particles/air_bubble-3.png"); | ||
m_bubble_particles[3] = Surface::from_file("images/particles/air_bubble-4.png"); | ||
m_bubble_timer.start(3.0f + (rand() % 2)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For usage of rng in graphics effects, use graphicsRandom
m_bubble_timer.start(3.0f + (rand() % 2)); | |
m_bubble_timer.start(3.0f + graphicsRandom.randf(2)); |
src/object/player.cpp
Outdated
SurfacePtr bubble_surface = m_bubble_particles[random_index]; | ||
|
||
glm::vec2 bubble_pos; | ||
if (m_swimming_angle > static_cast<float>(M_PI_2) && m_swimming_angle < 3.0f * static_cast<float>(M_PI_2)) // Facing left |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (m_swimming_angle > static_cast<float>(M_PI_2) && m_swimming_angle < 3.0f * static_cast<float>(M_PI_2)) // Facing left | |
if (m_swimming_angle > static_cast<float>(math::PI_2) && m_swimming_angle < 3.0f * static_cast<float>(math::PI_2)) // Facing left |
src/object/player.cpp
Outdated
m_bubble_particles[0] = Surface::from_file("images/particles/air_bubble-1.png"); | ||
m_bubble_particles[1] = Surface::from_file("images/particles/air_bubble-2.png"); | ||
m_bubble_particles[2] = Surface::from_file("images/particles/air_bubble-3.png"); | ||
m_bubble_particles[3] = Surface::from_file("images/particles/air_bubble-4.png"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently rusty sent you a sprite file? Use this in junction with the get_actions method
Sorry, but I have to slightly update my suggestion regarding how to determine the direction. From the swim sprite logic, I saw that the following code is used: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good, will test later
src/object/player.cpp
Outdated
Vector beak_position; | ||
|
||
// Determine direction based on the radians | ||
if (m_swimming_angle > static_cast<float>(math::PI_2) && m_swimming_angle < 3.0f * static_cast<float>(math::PI_2)) // Facing left |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please change the check to std::abs(m_swimming_angle) <= math::PI_2
? This is used here in Player.cpp already, so the code would be consistent. Remember, that this condition checks for swimming right, so your conditional code must be swapped.
src/object/player.cpp
Outdated
SurfacePtr bubble_surface = surfaces.at(random_index); | ||
|
||
Vector bubble_pos(0.f, 0.f); | ||
if (m_swimming_angle > static_cast<float>(math::PI_2) && m_swimming_angle < 3.0f * static_cast<float>(math::PI_2)) // Facing left |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please change the check to std::abs(m_swimming_angle) <= math::PI_2
? This is used here in Player.cpp already, so the code would be consistent. Remember, that this condition checks for swimming right, so your conditional code must be swapped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is just this one include, that is obsolete again. Rest looks fine, thanks for adapting the left/right check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks fine to me.
While this looks nice and cartoony, I (random passerby) should warn that cartoons are often wrong, and actual penguins probably don't breathe out underwater. (Nor should any species that can't breathe in underwater, except if very close to the surface :-) Instead of emitting periodic bubbles from their beaks, penguins might produce them through:
Therefore: I would suggest emitting the bubbles more irregularly in space and time; from the main body instead of from the beak; and maybe progressively reduce the bubble rate the longer Tux stays underwater. |
SuperTux is not aimed to replicate realistic behaviours. This is just a little visual effect. This is out of scope for this PR - and this might be adjusted in the future. |
Agreed. While I think foundations in reality are nice to have, in the end art and gameplay are what matter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can't confirm if the item in m_active_bubbles
gets popped when the bubble goes offscreen, but make sure it does.
Almost there!
The bubbles are still not animated despite the sprite file having 2 action of animation. One for bigger bubbles and one for smaller ones. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may be able to get away string_view instead of arrays of strings
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work
This pr adds a visual effect - when Tux is underwater, he will emit bubble particles that will follow a sine pattern until they reach the water surface.
Make sure to test.