Skip to content

Saving worldmap position on unstable tiles#3766

Open
joanasaraiva10 wants to merge 1 commit intoSuperTux:masterfrom
joanasaraiva10:fix-saved-paths
Open

Saving worldmap position on unstable tiles#3766
joanasaraiva10 wants to merge 1 commit intoSuperTux:masterfrom
joanasaraiva10:fix-saved-paths

Conversation

@joanasaraiva10
Copy link
Copy Markdown

When leaving to the main menu, the game saves Tux's current worldmap position even if he is mid-path or on a non-stable tile. Upon reloading, Tux is restored to that invalid position. Save Tux on the last valid "stable" tile (such as a stop or level tile) instead of the current one. Also store the corresponding back direction to keep movement consistent after loading. In ghost mode, avoid persisting invalid tiles by falling back to the last valid level tile. This ensures Tux always respawns on a valid, navigable position.

Closes #3728

@joanasaraiva10
Copy link
Copy Markdown
Author

@tobbi Hi! I was wondering if you could check my PR as a few weeks have passed since I opened it. I would also like to know what can I do for the 2 failing checks to pass.

@tobbi
Copy link
Copy Markdown
Member

tobbi commented Apr 29, 2026

Can you please rebase against master?

When leaving to the main menu, the game saves Tux's current
worldmap position even if he is mid-path or on a non-stable tile.
Upon reloading, Tux is restored to that invalid position.
Save Tux on the last valid "stable" tile (such as a stop or level
tile) instead of the current one. Also store the corresponding
back direction to keep movement consistent after loading.
In ghost mode, avoid persisting invalid tiles by falling back to
the last valid level tile. This ensures Tux always respawns on a
valid, navigable position.
Comment thread src/worldmap/tux.cpp
m_worldmap->set_passive_message({}, 0.0f);
}

if (const auto level = worldmap_sector->at_object<LevelTile>(); level)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[...]; level[...]

Pardon?

In general, I would avoid declaring variables in if statements to improve clarity. Even with that in mind, it doesn't even seem like it is necessary in this case.

Suggested change
if (const auto level = worldmap_sector->at_object<LevelTile>(); level)
if (worldmap_sector->at_object<LevelTile>())

Comment thread src/worldmap/tux.cpp
Comment on lines +311 to +327
if (const auto level = worldmap_sector->at_object<LevelTile>(); level)
{
m_last_level_tile_pos = m_tile_pos;
m_last_level_back_direction = m_back_direction;
m_last_stable_tile_pos = m_tile_pos;
m_last_stable_back_direction = m_back_direction;
}
else if (!m_ghost_mode)
{
m_last_stable_tile_pos = m_tile_pos;
m_last_stable_back_direction = m_back_direction;
}
else
{
m_last_stable_tile_pos = m_last_level_tile_pos;
m_last_stable_back_direction = m_last_level_back_direction;
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, I believe this could be written in a more concise manner.

Suggested change
if (const auto level = worldmap_sector->at_object<LevelTile>(); level)
{
m_last_level_tile_pos = m_tile_pos;
m_last_level_back_direction = m_back_direction;
m_last_stable_tile_pos = m_tile_pos;
m_last_stable_back_direction = m_back_direction;
}
else if (!m_ghost_mode)
{
m_last_stable_tile_pos = m_tile_pos;
m_last_stable_back_direction = m_back_direction;
}
else
{
m_last_stable_tile_pos = m_last_level_tile_pos;
m_last_stable_back_direction = m_last_level_back_direction;
}
if (worldmap_sector->at_object<LevelTile>() != nullptr)
{
m_last_level_tile_pos = m_tile_pos;
m_last_level_back_direction = m_back_direction;
}
m_last_stable_tile_pos = m_ghost_mode ? m_last_level_tile_pos : m_tile_pos;
m_last_stable_back_direction = m_ghost_mode ? m_last_level_back_direction : m_back_direction;

Please make sure to double-check this logic.

log_warning << "Player position not set, respawning." << std::endl;
sector.move_to_spawnpoint(DEFAULT_SPAWNPOINT_NAME);
m_position_was_reset = true;
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excuse me. I don't understand why this is here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Saving worldmap position in the middle of a path

3 participants