From 607d7a1d99df6db967b5d38e3e7b0c792db86bca Mon Sep 17 00:00:00 2001 From: danij Date: Wed, 25 Sep 2013 23:39:50 +0100 Subject: [PATCH] World|Map: Allow linking mobjs to a sector of a degenerate BSP leaf 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... --- doomsday/client/include/world/bspleaf.h | 21 ++++++++++++--------- doomsday/client/include/world/sector.h | 5 ++++- doomsday/client/src/world/bspleaf.cpp | 2 +- doomsday/client/src/world/map.cpp | 24 +++++++++--------------- 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/doomsday/client/include/world/bspleaf.h b/doomsday/client/include/world/bspleaf.h index 21bf65d336..1b74700f9e 100644 --- a/doomsday/client/include/world/bspleaf.h +++ b/doomsday/client/include/world/bspleaf.h @@ -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 §or() const { return cluster().sector(); } + inline Sector §or() { return parent().as(); } + + /// @copydoc sector() + inline Sector const §or() const { return parent().as(); } /** - * 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()? §or() : 0; } + + /// @copydoc sectorPtr() + inline Sector const *sectorPtr() const { return hasParent()? §or() : 0; } /** * Remove the given @a polyobj from the set of those linked to the BSP leaf. diff --git a/doomsday/client/include/world/sector.h b/doomsday/client/include/world/sector.h index fae7625411..e6fbbd8175 100644 --- a/doomsday/client/include/world/sector.h +++ b/doomsday/client/include/world/sector.h @@ -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. */ diff --git a/doomsday/client/src/world/bspleaf.cpp b/doomsday/client/src/world/bspleaf.cpp index aaef950794..85be59be6e 100644 --- a/doomsday/client/src/world/bspleaf.cpp +++ b/doomsday/client/src/world/bspleaf.cpp @@ -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, §orAdr, 0); break; } default: diff --git a/doomsday/client/src/world/map.cpp b/doomsday/client/src/world/map.cpp index 65765da592..4f2d08f5f2 100644 --- a/doomsday/client/src/world/map.cpp +++ b/doomsday/client/src/world/map.cpp @@ -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; @@ -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; } } }