Fix Tux being permanently safe after mix of using door and scripting Tux safe #3187
Conversation
src/object/player.hpp
Outdated
| /** | ||
| * @description Flag indicating that Tux shall not blink while being safe | ||
| */ | ||
| bool m_avoid_blinking_while_safe; | ||
|
|
There was a problem hiding this comment.
Avoid blinking while safe?? Man... This file needs a rewrite so bad...
There was a problem hiding this comment.
Giving variables a good name can be very challenging. ;-) I‘d be grateful for any suggestion.
src/object/player.cpp
Outdated
| @@ -2571,7 +2572,7 @@ Player::check_bounds() | |||
| /* fallen out of the level? */ | |||
| if ((get_pos().y > Sector::get().get_height()) | |||
| && !m_ghost_mode | |||
| && !(m_is_intentionally_safe && m_safe_timer.started())) { | |||
| && !(m_avoid_blinking_while_safe && m_safe_timer.started())) { | |||
There was a problem hiding this comment.
This use of m_avoid_blinking_while_safe does not match the variable description.
One strategy that might work here is to avoid such name/purpose problems is to avoid "derived" names, properties and state as much as possible. For example, instead of having a m_avoid_blinking_while_safe variable, store a variable expressing the cause, from which one determine whether the player should blink, or whether it should be allowed to fall out of bounds.
enum TemporarySafetyCause {
Damage, // has blink effect
Transition, // stay alive even if out of bounds
};
TemporarySafetyCause m_safe_timer_reason;
Another approach is to replace m_safe_timer with two distinct timers for the different causes; e.g. use m_post_damage_timer, started only in Player::kill, and m_temp_safety_timer, started only in make_temporarily_safe . Doing this would make it possible to have the player blink for the specified time after being damaged, even when going through a door halfway through.
There was a problem hiding this comment.
Another approach is to replace
m_safe_timerwith two distinct timers for the different causes; e.g. usem_post_damage_timer, started only in Player::kill, andm_temp_safety_timer, started only inmake_temporarily_safe. Doing this would make it possible to have the player blink for the specified time after being damaged, even when going through a door halfway through.
This totally makes sense in my opinion, so I modified the PR accordingly.
…Tux safe * Use a separate timer for temporary safety instead of reusing m_is_intentionally_safe * Split timers for post damage safety and temporary safety Fixes SuperTux#3186
Brockengespenst
force-pushed
the
fix_invincibility_after_door_and_scripting
branch
from
49d73db to
b45af28
Compare
|
I do not see any problems with the current version of this PR, and have tested it in a few scenarios (going through doors, applying damage, sector changes, etc.) without finding any issues. |
|
Nevermind, I can't review this, because there is no functional windows build. I'm going to assume it works though and i think 2 reviewers is enough. For context, i made a review in the wrong place. Sorry! |
Fixes #3186