Skip to content

Commit

Permalink
World|Map: Allow linking mobjs to a sector of a degenerate BSP leaf
Browse files Browse the repository at this point in the history
There appears to be some confusion as to what it means for a mobj to
be "sector linked". Specifically, whether a mobj in a sector list is
"within" the bounds of that sector according to line geometry of the
map, or, if only in a BSP leaf attributed to that sector.

Essentially DOOM confuses the definition of a subsector, exchanging
"BSP half-space" with "convex subspace" depending on the context.
The name "subsector" probably didn't help to resolve it either.

For compatibility purposes, when linking mobjs to sectors we should
apply the BSP half-space definition for now.

Todo for later: Most likely there are more mixups and/or incorrect
assumptions in any code which uses the sector mobj lists...
  • Loading branch information
danij-deng committed Sep 25, 2013
1 parent 78f135d commit 607d7a1
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 26 deletions.
21 changes: 12 additions & 9 deletions doomsday/client/include/world/bspleaf.h
Expand Up @@ -198,20 +198,23 @@ class BspLeaf : public de::MapElement
}

/**
* Convenient method of returning the sector of the cluster attributed to
* the BSP leaf.
*
* @see hasSector(), cluster()
* Convenient method of returning the parent sector of the BSP leaf.
*/
inline Sector &sector() const { return cluster().sector(); }
inline Sector &sector() { return parent().as<Sector>(); }

/// @copydoc sector()
inline Sector const &sector() const { return parent().as<Sector>(); }

/**
* Convenient method returning a pointer to the sector of the cluster
* attributed to the BSP leaf. If not attributed then @c 0 is returned.
* Convenient method returning a pointer to the sector attributed to the
* BSP leaf. If not attributed then @c 0 is returned.
*
* @see hasSector(), sector()
* @see sector()
*/
inline Sector *sectorPtr() const { return hasCluster()? &cluster().sector() : 0; }
inline Sector *sectorPtr() { return hasParent()? &sector() : 0; }

/// @copydoc sectorPtr()
inline Sector const *sectorPtr() const { return hasParent()? &sector() : 0; }

/**
* Remove the given @a polyobj from the set of those linked to the BSP leaf.
Expand Down
5 changes: 4 additions & 1 deletion doomsday/client/include/world/sector.h
Expand Up @@ -532,7 +532,10 @@ class Sector : public de::MapElement
void unlink(struct mobj_s *mobj);

/**
* Link the mobj to the head of the list of mobjs "in" the sector.
* Link the mobj to the head of the list of mobjs "in" the sector. Note that
* mobjs in this list may not actually be inside the sector. This is because
* the sector is determined by interpreting the BSP leaf as a half-space and
* not a closed convex subspace (@ref Map::link()).
*
* @param mobj Mobj to be linked.
*/
Expand Down
2 changes: 1 addition & 1 deletion doomsday/client/src/world/bspleaf.cpp
Expand Up @@ -694,7 +694,7 @@ int BspLeaf::property(DmuArgs &args) const
switch(args.prop)
{
case DMU_SECTOR: {
Sector *sectorAdr = sectorPtr();
Sector const *sectorAdr = sectorPtr();
args.setValue(DMT_BSPLEAF_SECTOR, &sectorAdr, 0);
break; }
default:
Expand Down
24 changes: 9 additions & 15 deletions doomsday/client/src/world/map.cpp
Expand Up @@ -1963,11 +1963,9 @@ void Map::link(mobj_t &mo, byte flags)
BspLeaf &bspLeafAtOrigin = bspLeafAt_FixedPrecision(mo.origin);

// Link into the sector?
/// @todo fixme: Never link to a degenerate BSP leaf!
if(flags & DDLINK_SECTOR)
{
d->unlinkMobjFromSectors(mo);
DENG_ASSERT(bspLeafAtOrigin.hasCluster());
bspLeafAtOrigin.sector().link(&mo);
}
mo.bspLeaf = &bspLeafAtOrigin;
Expand All @@ -1990,25 +1988,21 @@ void Map::link(mobj_t &mo, byte flags)
// entered or exited the void.
if(mo.dPlayer && mo.dPlayer->mo)
{
ddplayer_t *player = mo.dPlayer;
mo.dPlayer->inVoid = true;

player->inVoid = true;
if(!player->mo->bspLeaf ||
!player->mo->bspLeaf->polyContains(player->mo->origin))
if(!Mobj_BspLeafAtOrigin(mo).polyContains(mo.origin))
return;

if(SectorCluster *cluster = player->mo->bspLeaf->clusterPtr())
{
SectorCluster &cluster = Mobj_Cluster(mo);
#ifdef __CLIENT__
if(player->mo->origin[VZ] < cluster->visCeiling().heightSmoothed() + 4 &&
player->mo->origin[VZ] >= cluster->visFloor().heightSmoothed())
if(mo.origin[VZ] < cluster.visCeiling().heightSmoothed() + 4 &&
mo.origin[VZ] >= cluster.visFloor().heightSmoothed())
#else
if(player->mo->origin[VZ] < cluster->ceiling().height() + 4 &&
player->mo->origin[VZ] >= cluster->floor().height())
if(mo.origin[VZ] < cluster.ceiling().height() + 4 &&
mo.origin[VZ] >= cluster.floor().height())
#endif
{
player->inVoid = false;
}
{
mo.dPlayer->inVoid = false;
}
}
}
Expand Down

0 comments on commit 607d7a1

Please sign in to comment.