From c98cf4cd2907eb3396bfc363c1e88ed99fac4b85 Mon Sep 17 00:00:00 2001
From: bones_was_here <bones_was_here@xonotic.au>
Date: Tue, 4 Jul 2023 22:28:00 +1000
Subject: [PATCH] items: merge _StartItem() and StartItem(), warn instead of
 crashing if defaultrespawntime not set

Null ptr deref is a pain to debug.
---
 qcsrc/server/items/items.qc      | 31 ++++++++++++++++---------------
 qcsrc/server/items/items.qh      |  9 ++++-----
 qcsrc/server/weapons/spawning.qc |  4 +---
 3 files changed, 21 insertions(+), 23 deletions(-)

diff --git a/qcsrc/server/items/items.qc b/qcsrc/server/items/items.qc
index e0013b5ce..e06c6f074 100644
--- a/qcsrc/server/items/items.qc
+++ b/qcsrc/server/items/items.qc
@@ -961,10 +961,14 @@ void item_use(entity this, entity actor, entity trigger)
 	gettouch(this)(this, actor);
 }
 
-// if defaultrespawntime is 0 get respawntime from the item definition
-// if defaultrespawntimejitter is 0 get respawntimejitter from the item definition
-void _StartItem(entity this, entity def, float defaultrespawntime, float defaultrespawntimejitter)
+void StartItem(entity this, entity def)
 {
+	if (def.m_spawnfunc_hookreplace)
+		def = def.m_spawnfunc_hookreplace(def, this);
+	this.itemdef = def;
+	if (def.m_canonical_spawnfunc != "") // FIXME why do weapons set itemdef to an entity that doesn't have this?
+		this.classname = def.m_canonical_spawnfunc;
+
 	string itemname = def.m_name;
 	float(entity player, entity item) pickupevalfunc = def.m_pickupevalfunc;
 	float pickupbasevalue = def.m_botvalue;
@@ -1059,8 +1063,15 @@ void _StartItem(entity this, entity def, float defaultrespawntime, float default
 		// must be done before Item_Reset() and after MUTATORBLOCKED check (blocked items may have null func ptrs)
 		if(!this.respawntime) // both need to be set
 		{
-			this.respawntime = defaultrespawntime ? defaultrespawntime : def.m_respawntime();
-			this.respawntimejitter = defaultrespawntimejitter ? defaultrespawntimejitter : def.m_respawntimejitter();
+			if (def.m_respawntime)
+				this.respawntime = def.m_respawntime();
+			else
+				LOG_WARNF("Default respawntime for a %s is unavailable from its itemdef", this.classname);
+
+			if (def.m_respawntimejitter)
+				this.respawntimejitter = def.m_respawntimejitter();
+			else
+				LOG_WARNF("Default respawntimejitter for a %s is unavailable from its itemdef", this.classname);
 		}
 
 		if(this.angles != '0 0 0')
@@ -1196,16 +1207,6 @@ void _StartItem(entity this, entity def, float defaultrespawntime, float default
 	setItemGroup(this);
 }
 
-void StartItem(entity this, GameItem def)
-{
-	def = def.m_spawnfunc_hookreplace(def, this);
-
-	this.classname = def.m_canonical_spawnfunc;
-
-	this.itemdef = def;
-	_StartItem(this, this.itemdef, 0, 0);
-}
-
 #define IS_SMALL(def) ((def.instanceOfHealth && def == ITEM_HealthSmall) || (def.instanceOfArmor && def == ITEM_ArmorSmall))
 int group_count = 1;
 
diff --git a/qcsrc/server/items/items.qh b/qcsrc/server/items/items.qh
index a38765c27..7bfb97353 100644
--- a/qcsrc/server/items/items.qh
+++ b/qcsrc/server/items/items.qh
@@ -11,11 +11,8 @@ int autocvar_g_pickup_items;
 bool autocvar_g_nodepthtestitems;
 #define autocvar_g_weapon_stay cvar("g_weapon_stay")
 
-void StartItem(entity this, entity a);
-.int item_group;
-.int item_group_count;
-
 float autocvar_sv_simple_items;
+
 bool ItemSend(entity this, entity to, int sf);
 
 const float ITEM_RESPAWN_TICKS = 10;
@@ -92,8 +89,10 @@ float healtharmor_pickupevalfunc(entity player, entity item);
 
 .bool is_item;
 .entity itemdef;
-void _StartItem(entity this, entity def, float defaultrespawntime, float defaultrespawntimejitter);
+void StartItem(entity this, entity def);
 
+.int item_group;
+.int item_group_count;
 void setItemGroup(entity this);
 void setItemGroupCount();
 
diff --git a/qcsrc/server/weapons/spawning.qc b/qcsrc/server/weapons/spawning.qc
index 3dd040b97..e2a28f254 100644
--- a/qcsrc/server/weapons/spawning.qc
+++ b/qcsrc/server/weapons/spawning.qc
@@ -155,9 +155,7 @@ void weapon_defaultspawnfunc(entity this, Weapon wpn)
 	else
 		this.glowmod = colormapPaletteColor(this.owner.clientcolors & 0x0F, true);
 
-	GameItem def = wpn.m_pickup;
-	this.itemdef = def;
-	_StartItem(this, this.itemdef, this.respawntime, this.respawntimejitter);
+	StartItem(this, wpn.m_pickup);
 
 	#if 0 // WEAPONTODO
 	if (this.modelindex) { // don't precache if this was removed
-- 
2.39.2