From d338ca3ec17f65a7ea256fd576ef8afcdfdc479a Mon Sep 17 00:00:00 2001
From: Christoph Oelckers <coelckers@users.noreply.github.com>
Date: Fri, 13 Jan 2017 13:51:47 +0100
Subject: [PATCH] - scriptified the sector actions.

---
 src/CMakeLists.txt                            |   1 -
 src/am_map.cpp                                |   8 +-
 src/d_main.cpp                                |   2 +-
 src/dobject.cpp                               |   4 +-
 src/dobjtype.cpp                              |   4 +-
 src/dobjtype.h                                |   2 +-
 src/g_shared/a_sectoraction.cpp               | 117 ------------------
 src/p_sectors.cpp                             |  21 +++-
 src/r_defs.h                                  |   3 +-
 src/s_advsound.cpp                            |  42 -------
 src/scripting/thingdef.cpp                    |   2 +
 src/virtual.h                                 |  10 ++
 wadsrc/static/zscript/shared/sectoraction.txt | 112 +++++++++++++++--
 13 files changed, 141 insertions(+), 187 deletions(-)
 delete mode 100644 src/g_shared/a_sectoraction.cpp

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 70e4715b1..7826ec6ab 100644
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -1166,7 +1166,6 @@ set (PCH_SOURCES
 	g_shared/a_movingcamera.cpp
 	g_shared/a_quake.cpp
 	g_shared/a_randomspawner.cpp
-	g_shared/a_sectoraction.cpp
 	g_shared/a_skies.cpp
 	g_shared/a_soundenvironment.cpp
 	g_shared/a_soundsequence.cpp
diff --git a/src/am_map.cpp b/src/am_map.cpp
index a6ccb92cf..70d439d9d 100644
--- a/src/am_map.cpp
+++ b/src/am_map.cpp
@@ -2228,11 +2228,13 @@ bool AM_Check3DFloors(line_t *line)
 // If needUseActivated is true, the special must be activated by use.
 bool AM_checkSectorActions (sector_t *sector, bool (*function)(int, int *), int *specialptr, int **argsptr, bool needUseActivated)
 {
-	for (ASectorAction* action = sector->SecActTarget; action; action = barrier_cast<ASectorAction *>(action->tracer))
+	// This code really stands in the way of a more generic and flexible implementation of sector actions because it makes far too many assumptions
+	// about their internal workings. Well, it can't be helped. Let's just hope that nobody abuses the special and the health field in a way that breaks this.
+	for (AActor* action = sector->SecActTarget; action; action = action->tracer)
 	{
-		if ((action->IsActivatedByUse() || false == needUseActivated)
+		if (((action->health & (SECSPAC_Use | SECSPAC_UseWall)) || false == needUseActivated)
 			&& (*function)(action->special, action->args)
-			&& action->CanTrigger (players[consoleplayer].mo))
+			&& !(action->flags & MF_FRIENDLY))
 		{
 			*specialptr = action->special;
 			*argsptr = action->args;
diff --git a/src/d_main.cpp b/src/d_main.cpp
index 1fd89c6bf..887923260 100644
--- a/src/d_main.cpp
+++ b/src/d_main.cpp
@@ -2726,7 +2726,7 @@ void D_DoomMain (void)
 
 		restart++;
 		PClass::bShutdown = false;
-		PClass::bShuttingDown = false;
+		PClass::bVMOperational = false;
 	}
 	while (1);
 }
diff --git a/src/dobject.cpp b/src/dobject.cpp
index ead9848be..30c3c1710 100644
--- a/src/dobject.cpp
+++ b/src/dobject.cpp
@@ -354,7 +354,7 @@ DObject::~DObject ()
 void DObject:: Destroy ()
 {
 	// We cannot call the VM during shutdown because all the needed data has been or is in the process of being deleted.
-	if (!PClass::bShuttingDown)
+	if (PClass::bVMOperational)
 	{
 		IFVIRTUAL(DObject, OnDestroy)
 		{
@@ -495,7 +495,7 @@ size_t DObject::StaticPointerSubstitution (DObject *old, DObject *notOld, bool s
 #define SECTOR_CHECK(f,t) \
 if (sec.f.p == static_cast<t *>(old)) { sec.f = static_cast<t *>(notOld); changed++; }
 		SECTOR_CHECK( SoundTarget, AActor );
-		SECTOR_CHECK( SecActTarget, ASectorAction );
+		SECTOR_CHECK( SecActTarget, AActor );
 		SECTOR_CHECK( floordata, DSectorEffect );
 		SECTOR_CHECK( ceilingdata, DSectorEffect );
 		SECTOR_CHECK( lightingdata, DSectorEffect );
diff --git a/src/dobjtype.cpp b/src/dobjtype.cpp
index cc5dfdb4e..bde74f649 100644
--- a/src/dobjtype.cpp
+++ b/src/dobjtype.cpp
@@ -72,7 +72,7 @@ FTypeTable TypeTable;
 PSymbolTable GlobalSymbols;
 TArray<PClass *> PClass::AllClasses;
 bool PClass::bShutdown;
-bool PClass::bShuttingDown;
+bool PClass::bVMOperational;
 
 PErrorType *TypeError;
 PErrorType *TypeAuto;
@@ -2962,7 +2962,7 @@ void PClass::StaticShutdown ()
 
 	// From this point onward no scripts may be called anymore because the data needed by the VM is getting deleted now.
 	// This flags DObject::Destroy not to call any scripted OnDestroy methods anymore.
-	bShuttingDown = true;
+	bVMOperational = false;
 
 	// Unless something went wrong, anything left here should be class and type objects only, which do not own any scripts.
 	TypeTable.Clear();
diff --git a/src/dobjtype.h b/src/dobjtype.h
index 3035a6eb2..6f33483fc 100644
--- a/src/dobjtype.h
+++ b/src/dobjtype.h
@@ -883,7 +883,7 @@ public:
 	static TArray<PClass *> AllClasses;
 
 	static bool bShutdown;
-	static bool bShuttingDown;
+	static bool bVMOperational;
 };
 
 class PClassType : public PClass
diff --git a/src/g_shared/a_sectoraction.cpp b/src/g_shared/a_sectoraction.cpp
deleted file mode 100644
index 6cc285afa..000000000
--- a/src/g_shared/a_sectoraction.cpp
+++ /dev/null
@@ -1,117 +0,0 @@
-/*
-** a_sectoraction.cpp
-** Actors that hold specials to be executed upon conditions in a sector
-**
-**---------------------------------------------------------------------------
-** Copyright 1998-2006 Randy Heit
-** All rights reserved.
-**
-** Redistribution and use in source and binary forms, with or without
-** modification, are permitted provided that the following conditions
-** are met:
-**
-** 1. Redistributions of source code must retain the above copyright
-**    notice, this list of conditions and the following disclaimer.
-** 2. Redistributions in binary form must reproduce the above copyright
-**    notice, this list of conditions and the following disclaimer in the
-**    documentation and/or other materials provided with the distribution.
-** 3. The name of the author may not be used to endorse or promote products
-**    derived from this software without specific prior written permission.
-**
-** THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
-** IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
-** OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
-** IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
-** INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
-** NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-** DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-** THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-** (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
-** THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-**---------------------------------------------------------------------------
-**
-*/
-
-#include "r_defs.h"
-#include "p_lnspec.h"
-
-// The base class for sector actions ----------------------------------------
-
-IMPLEMENT_CLASS(ASectorAction, false, false)
-
-bool ASectorAction::IsActivatedByUse() const
-{
-	return !!(health & (SECSPAC_Use|SECSPAC_UseWall));
-}
-
-void ASectorAction::OnDestroy ()
-{
-	if (Sector != nullptr)
-	{
-		// Remove ourself from this sector's list of actions
-		AActor *probe = Sector->SecActTarget;
-		union
-		{
-			AActor **act;
-			ASectorAction **secact;
-		} prev;
-		prev.secact = &Sector->SecActTarget;
-
-		while (probe && probe != this)
-		{
-			prev.act = &probe->tracer;
-			probe = probe->tracer;
-		}
-		if (probe != nullptr)
-		{
-			*prev.act = probe->tracer;
-		}
-		Sector = nullptr;
-	}
-
-	Super::OnDestroy();
-}
-
-void ASectorAction::BeginPlay ()
-{
-	Super::BeginPlay ();
-
-	// Add ourself to this sector's list of actions
-	tracer = Sector->SecActTarget;
-	Sector->SecActTarget = this;
-}
-
-void ASectorAction::Activate (AActor *source)
-{
-	flags2 &= ~MF2_DORMANT;	// Projectiles cannot trigger
-}
-
-void ASectorAction::Deactivate (AActor *source)
-{
-	flags2 |= MF2_DORMANT;	// Projectiles can trigger
-}
-
-bool ASectorAction::DoTriggerAction (AActor *triggerer, int activationType)
-{
-	return (activationType & health) ? CheckTrigger(triggerer) : false;
-}
-
-bool ASectorAction::CanTrigger (AActor *triggerer) const
-{
-	return special &&
-		((triggerer->player && !(flags & MF_FRIENDLY)) ||
-		((flags & MF_AMBUSH) && (triggerer->flags2 & MF2_MCROSS)) ||
-		((flags2 & MF2_DORMANT) && (triggerer->flags2 & MF2_PCROSS)));
-}
-
-bool ASectorAction::CheckTrigger (AActor *triggerer) const
-{
-	if (CanTrigger(triggerer))
-	{
-		bool res = !!P_ExecuteSpecial(special, NULL, triggerer, false, args[0], args[1],
-			args[2], args[3], args[4]);
-		return res;
-	}
-	return false;
-}
-
diff --git a/src/p_sectors.cpp b/src/p_sectors.cpp
index b8b2fdfa0..e64296da7 100644
--- a/src/p_sectors.cpp
+++ b/src/p_sectors.cpp
@@ -36,6 +36,7 @@
 #include "r_sky.h"
 #include "r_data/colormaps.h"
 #include "g_levellocals.h"
+#include "virtual.h"
 
 
 // [RH]
@@ -1461,16 +1462,24 @@ DEFINE_ACTION_FUNCTION(_Sector, NextLowestFloorAt)
 	 {
 		 AActor *next = act->tracer;
 
-		 bool didit = act->DoTriggerAction(thing, activation);
-		 if (didit)
+		 IFVIRTUALPTRNAME(act, "SectorAction", TriggerAction)
 		 {
-			 if (act->flags4 & MF4_STANDSTILL)
+			 VMValue params[3] = { act, thing, activation };
+			 VMReturn ret;
+			 int didit;
+			 ret.IntAt(&didit);
+			 GlobalVMStack.Call(func, params, 3, &ret, 1, nullptr);
+
+			 if (didit)
 			 {
-				 act->Destroy();
+				 if (act->flags4 & MF4_STANDSTILL)
+				 {
+					 act->Destroy();
+				 }
 			 }
+			 act = next;
+			 res |= !!didit;
 		 }
-		 act = static_cast<ASectorAction*>(next);
-		 res |= didit;
 	 }
 	 return res;
  }
diff --git a/src/r_defs.h b/src/r_defs.h
index e3eed8135..916ff569d 100644
--- a/src/r_defs.h
+++ b/src/r_defs.h
@@ -287,7 +287,6 @@ public:
 	void Activate (AActor *source);
 	void Deactivate (AActor *source);
 	bool CanTrigger (AActor *triggerer) const;
-	bool IsActivatedByUse() const;
 	virtual bool DoTriggerAction(AActor *triggerer, int activationType);
 protected:
 	bool CheckTrigger(AActor *triggerer) const;
@@ -1068,7 +1067,7 @@ public:
 	// flexible in a Bloody way. SecActTarget forms a list of actors
 	// joined by their tracer fields. When a potential sector action
 	// occurs, SecActTarget's TriggerAction method is called.
-	TObjPtr<ASectorAction> SecActTarget;
+	TObjPtr<AActor> SecActTarget;
 
 	// [RH] The portal or skybox to render for this sector.
 	unsigned Portals[2];
diff --git a/src/s_advsound.cpp b/src/s_advsound.cpp
index 052abf04e..085f1869e 100644
--- a/src/s_advsound.cpp
+++ b/src/s_advsound.cpp
@@ -2457,45 +2457,3 @@ void S_ParseMusInfo()
 }
 
 
-//==========================================================================
-//
-// Music changer. Uses the sector action class to do its job
-//
-//==========================================================================
-
-class AMusicChanger : public ASectorAction
-{
-	DECLARE_CLASS (AMusicChanger, ASectorAction)
-public:
-	virtual bool DoTriggerAction (AActor *triggerer, int activationType);
-	virtual void PostBeginPlay();
-};
-
-IMPLEMENT_CLASS(AMusicChanger, false, false)
-
-bool AMusicChanger::DoTriggerAction (AActor *triggerer, int activationType)
-{
-	if (activationType & SECSPAC_Enter && triggerer->player != NULL)
-	{
-		if (triggerer->player->MUSINFOactor != this)
-		{
-			triggerer->player->MUSINFOactor = this;
-			triggerer->player->MUSINFOtics = 30;
-		}
-	}
-	return false;
-}
- 
-void AMusicChanger::PostBeginPlay()
-{
-	// The music changer should consider itself activated if the player
-	// spawns in its sector as well as if it enters the sector during a P_TryMove.
-	Super::PostBeginPlay();
-	for (int i = 0; i < MAXPLAYERS; ++i)
-	{
-		if (playeringame[i] && players[i].mo && players[i].mo->Sector == this->Sector)
-		{
-			DoTriggerAction(players[i].mo, SECSPAC_Enter);
-		}
-	}
-}
diff --git a/src/scripting/thingdef.cpp b/src/scripting/thingdef.cpp
index ef26d8a50..38e58d6f0 100644
--- a/src/scripting/thingdef.cpp
+++ b/src/scripting/thingdef.cpp
@@ -443,6 +443,8 @@ void LoadActors()
 	timer.Unclock();
 	if (!batchrun) Printf("script parsing took %.2f ms\n", timer.TimeMS());
 
+	// Now we may call the scripted OnDestroy method.
+	PClass::bVMOperational = true;
 	// Since these are defined in DECORATE now the table has to be initialized here.
 	for (int i = 0; i < 31; i++)
 	{
diff --git a/src/virtual.h b/src/virtual.h
index 844c51d38..f0479e663 100644
--- a/src/virtual.h
+++ b/src/virtual.h
@@ -18,3 +18,13 @@ inline unsigned GetVirtualIndex(PClass *cls, const char *funcname)
 	if (func != nullptr)
 
 #define IFVIRTUAL(cls, funcname) IFVIRTUALPTR(this, cls, funcname)
+
+#define IFVIRTUALPTRNAME(self, cls, funcname) \
+	static unsigned VIndex = ~0u; \
+	if (VIndex == ~0u) { \
+		VIndex = GetVirtualIndex(PClass::FindActor(cls), #funcname); \
+		assert(VIndex != ~0u); \
+	} \
+	auto clss = self->GetClass(); \
+	VMFunction *func = clss->Virtuals.Size() > VIndex? clss->Virtuals[VIndex] : nullptr;  \
+	if (func != nullptr)
diff --git a/wadsrc/static/zscript/shared/sectoraction.txt b/wadsrc/static/zscript/shared/sectoraction.txt
index b1d41a69f..32302959a 100644
--- a/wadsrc/static/zscript/shared/sectoraction.txt
+++ b/wadsrc/static/zscript/shared/sectoraction.txt
@@ -1,7 +1,7 @@
 
-class SectorAction : Actor native
+class SectorAction : Actor
 {
-	// this class uses health to define the activation type.
+	// self class uses health to define the activation type.
 	enum EActivation
 	{
 		SECSPAC_Enter		= 1,
@@ -24,9 +24,70 @@ class SectorAction : Actor native
 		+NOGRAVITY
 		+DONTSPLASH
 	}
+	
+	override void OnDestroy ()
+	{
+		if (CurSector != null)
+		{
+			// Remove ourself from self CurSector's list of actions
+			if (CurSector.SecActTarget == self)
+			{
+				CurSector.SecActTarget = SectorAction(tracer);
+			}
+			else
+			{
+				Actor probe = CurSector.SecActTarget;
+				while (probe.tracer != self && probe.tracer != null)
+				{
+					probe = probe.tracer;
+				}
+				if (probe.tracer == self)
+				{
+					probe.tracer = tracer;
+				}
+			}
+		}
+		Super.OnDestroy();
+	}
+
+	override void BeginPlay ()
+	{
+		Super.BeginPlay ();
+
+		// Add ourself to self CurSector's list of actions
+		tracer = CurSector.SecActTarget;
+		CurSector.SecActTarget = self;
+	}
+
+	override void Activate (Actor source)
+	{
+		bDormant = false;	// Projectiles cannot trigger
+	}
+
+	override void Deactivate (Actor source)
+	{
+		bDormant = true;	// Projectiles can trigger
+	}
+
+	virtual bool TriggerAction (Actor triggerer, int activationType)
+	{
+		if ((activationType & health) && CanTrigger(triggerer))
+		{
+			return triggerer.A_CallSpecial(special, args[0], args[1], args[2], args[3], args[4]);
+		}
+		return false;
+	}
+
+	virtual bool CanTrigger (Actor triggerer)
+	{
+		return special &&
+			((triggerer.player && !bFriendly) ||
+			(bAmbush && triggerer.bActivateMCross) ||
+			(bDormant && triggerer.bActivatePCross));
+	}
 }
 
-// Triggered when entering sector -------------------------------------------
+// Triggered when entering CurSector -------------------------------------------
 
 class SecActEnter : SectorAction
 {
@@ -36,7 +97,7 @@ class SecActEnter : SectorAction
 	}
 }
 
-// Triggered when leaving sector --------------------------------------------
+// Triggered when leaving CurSector --------------------------------------------
 
 class SecActExit : SectorAction
 {
@@ -46,7 +107,7 @@ class SecActExit : SectorAction
 	}
 }
 
-// Triggered when hitting sector's floor ------------------------------------
+// Triggered when hitting CurSector's floor ------------------------------------
 
 class SecActHitFloor : SectorAction
 {
@@ -56,7 +117,7 @@ class SecActHitFloor : SectorAction
 	}
 }
 
-// Triggered when hitting sector's ceiling ----------------------------------
+// Triggered when hitting CurSector's ceiling ----------------------------------
 
 class SecActHitCeil : SectorAction
 {
@@ -66,7 +127,7 @@ class SecActHitCeil : SectorAction
 	}
 }
 
-// Triggered when using inside sector ---------------------------------------
+// Triggered when using inside CurSector ---------------------------------------
 
 class SecActUse : SectorAction
 {
@@ -76,7 +137,7 @@ class SecActUse : SectorAction
 	}
 }
 
-// Triggered when using a sector's wall -------------------------------------
+// Triggered when using a CurSector's wall -------------------------------------
 
 class SecActUseWall : SectorAction
 {
@@ -136,9 +197,40 @@ class SecActHitFakeFloor : SectorAction
 	}
 }
 
-// Music changer ----------------------------------
+//==========================================================================
+//
+// Music changer. Uses the sector action class to do its job
+//
+//==========================================================================
 
-class MusicChanger : SectorAction native
+class MusicChanger : SectorAction
 {
+	override bool TriggerAction (Actor triggerer, int activationType)
+	{
+		if (activationType & SECSPAC_Enter && triggerer.player != null)
+		{
+			if (triggerer.player.MUSINFOactor != self)
+			{
+				triggerer.player.MUSINFOactor = self;
+				triggerer.player.MUSINFOtics = 30;
+			}
+		}
+		return false;
+	}
+	 
+	override void PostBeginPlay()
+	{
+		// The music changer should consider itself activated if the player
+		// spawns in its sector as well as if it enters the sector during a P_TryMove.
+		Super.PostBeginPlay();
+		for (int i = 0; i < MAXPLAYERS; ++i)
+		{
+			if (playeringame[i] && players[i].mo && players[i].mo.CurSector == self.CurSector)
+			{
+				TriggerAction(players[i].mo, SECSPAC_Enter);
+			}
+		}
+	}
 }
 
+