Skip to content

Commit

Permalink
Temporary fix for segfault (#1697)
Browse files Browse the repository at this point in the history
Sometimes, objects being deleted from a sector (e.g. exploding bombs) remained referenced in CollisionObjects and were accessed despite not existing.

The fix works but is not really nice. It will be improved as part of a larger work on code quality.

Closes #1695
  • Loading branch information
mbernard2 authored Mar 11, 2021
1 parent 56efa80 commit b26ce45
Show file tree
Hide file tree
Showing 5 changed files with 23 additions and 0 deletions.
6 changes: 6 additions & 0 deletions src/collision/collision_object.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,12 @@ CollisionObject::collision_moving_object_bottom(CollisionObject& other)
}
}

void
CollisionObject::notify_object_removal(CollisionObject* other)
{
m_objects_hit_bottom.erase(other);
}

void
CollisionObject::clear_bottom_collision_list()
{
Expand Down
2 changes: 2 additions & 0 deletions src/collision/collision_object.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ class CollisionObject
/** called when this object, if (moving) static, has collided on its top with a moving object */
void collision_moving_object_bottom(CollisionObject& other);

void notify_object_removal(CollisionObject* other);

void set_ground_movement_manager(const std::shared_ptr<CollisionGroundMovementManager>& movement_manager)
{
m_ground_movement_manager = movement_manager;
Expand Down
8 changes: 8 additions & 0 deletions src/collision/collision_system.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ CollisionSystem::remove(CollisionObject* object)
m_objects.erase(
std::find(m_objects.begin(), m_objects.end(),
object));

// FIXME: this is a patch. A better way of fixing this is coming.
for (auto* collision_object : m_objects) {
collision_object->notify_object_removal(object);
}
for (auto* tilemap : m_sector.get_solid_tilemaps()) {
tilemap->notify_object_removal(object);
}
}

void
Expand Down
6 changes: 6 additions & 0 deletions src/object/tilemap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -535,6 +535,12 @@ TileMap::hits_object_bottom(CollisionObject& object)
m_objects_hit_bottom.insert(&object);
}

void
TileMap::notify_object_removal(CollisionObject* other)
{
m_objects_hit_bottom.erase(other);
}

void
TileMap::set_solid(bool solid)
{
Expand Down
1 change: 1 addition & 0 deletions src/object/tilemap.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ class TileMap final :
/** Called by the collision mechanism to indicate that this tilemap has been hit on
the top, i.e. has hit a moving object on the bottom of its collision rectangle. */
void hits_object_bottom(CollisionObject& object);
void notify_object_removal(CollisionObject* other);

int get_layer() const { return m_z_pos; }
void set_layer(int layer_) { m_z_pos = layer_; }
Expand Down

0 comments on commit b26ce45

Please sign in to comment.