+CHECKMULTIPLAYERSPAWN (or perhaps something else)

[TerminusEst13]

This isn't so much a "glitch", but it is an annoying little thing that I think there should be a way to work around.
For mods which replace the weapon pickups with CustomInventory items to give a minor amount of scripting for their pickups, this effectively renders sv_noweaponspawn useless in netplay. Whether or not the flag is disabled or enabled, the Multiplayer weapon spawns set by the map are always triggered. For example: Doom 2 map01, which gives the BFG, SSG, and rocket launcher right away.
For earlier maps, like Doom 2/Heretic/etc, this is kind of an annoying thing since it effectively gives players a full loadout upon entering map02, rendering the rest of the early mapset trivial. The only option available is sv_coop_spactorspawn, which also removes extra Multiplayer monster spawns from the equation.

Is there any way to address this? Perhaps a +CHECKMULTIPLAYERSPAWN flag for CustomInventory that checks to see if the spawn spot has a Multiplayer flag attached? Or maybe something else?

[Graf Zahl]

I think the only reasonable thing here is to add an actor flag that triggers the weapon spawning behavior instead of hardcoding it to the weapon classes.

I have never been a fan of making such behavior so rigidly tied to some very limited range of actors, because this example clearly shows that it's ultimately self-defeating to code such limitations.

[Tesseract]

Actually, I've discovered this bug few months ago while testing related code submission (now quietly rotting in development limbo,sadly) - in my case it was the fact that random spawner was not respecting multiplayer only weapon spawning. I examined the issue and found solution - check original replaced actor instead of replacee,like this:

Code: Select all

// [RH] don't spawn extra weapons in coop if so desired
if (multiplayer && !deathmatch && (dmflags & DF_NO_COOP_WEAPON_SPAWN))
{
	if (i->IsDescendantOf (RUNTIME_CLASS(AWeapon)))
	{
		if ((mthing->flags & (MTF_DEATHMATCH|MTF_SINGLE)) == MTF_DEATHMATCH)
			return NULL;
	}
}

>>>

Code: Select all

// [RH] don't spawn extra weapons in coop if so desired
if (multiplayer && !deathmatch && (dmflags & DF_NO_COOP_WEAPON_SPAWN))
{
	if (i->ActorInfo->GetReplacee()->Class->IsDescendantOf (RUNTIME_CLASS(AWeapon)))
	{
		if ((mthing->flags & (MTF_DEATHMATCH|MTF_SINGLE)) == MTF_DEATHMATCH)
			return NULL;
	}
}

I was about to post it, when I realized it's only tip of the iceberg, there are many cases where problems could manifest, so complete solution must be well-thought and consistent.
In short, IsDescendantOf() check is not always sufficient for determining if given spawning filter should apply. For weapons, there are randomspawners, custom/fakeinventory or even pure actors with ACS magic, all not derived from AWeapon. Same goes for health and monsters. Considering that replace system is so permissive that you can replace stimpacks with cyberdemons, question arise about how spawn filters should work - based on replaced object,or replacee? (Of course, one can argue that replacing health with monster is stupid in the first place, but some magic wasp that flies around, attack player and can be picked up for health is reasonable idea.)

There are several dmflags-based spawn filters and all should be reevaluated - I had other priorities before, but now that someone else noticed this issue, let's discuss it.

"No monsters" probably should catch all monsters, no matter what they replace, but items, big items, health, armor and powerups are up to debate.

Example,in case the problem is not clear:
Should "Allow armor" affect actors only derived from AArmor? Then what about all armor replacements that replaces actual armor pickup but aren't like that? CustomInventory,Health,everything is possible really... If check is based on what original actor was, it catches all these cases ... but it misses case when some other actor type is replaced with something derived from AArmor. Combining both tests? But when some crazy person replaces armor with imps, should this filter still apply?

As you can see they're not trivial questions, that's why I wasn't eager to deal with this before.

[XutaWoo]

Ideally I think all actor classifications like that should be determined not by inheritance, like most of Inventory, but by actor flags, like monsters and big powerups.

[Graf Zahl]

I was about to post it, when I realized it's only tip of the iceberg, there are many cases where problems could manifest, so complete solution must be well-thought and consistent.
In short, IsDescendantOf() check is not always sufficient for determining if given spawning filter should apply. For weapons, there are randomspawners, custom/fakeinventory or even pure actors with ACS magic, all not derived from AWeapon. Same goes for health and monsters. Considering that replace system is so permissive that you can replace stimpacks with cyberdemons, question arise about how spawn filters should work - based on replaced object,or replacee? (Of course, one can argue that replacing health with monster is stupid in the first place, but some magic wasp that flies around, attack player and can be picked up for health is reasonable idea.)

Simple: Don't even think about it. It's the actor actually being spawned that should determine the rules. If we go through replacements and stuff it'll all end up a mess.
It's a mess anyway with users hacking the actors to pieces and then still expecting everything to work in a sane fashion. That's never going to happen.

I can uncouple the weapon spawning rules from the weapon class, but that's as far as I am willing to go with this. The current check is broken anyway because it forgot to check for the WeaponPiece and WeaponGiver class. To be honest, it was merely added to work around the limitations of the original map format, it never factored in users replacing stuff via DECORATE.

[Graf Zahl]

OK, done. I added a new flag, called WEAPONSPAWN that can be placed on the replacing actors. Please note that this will have no effect on DF_WEAPONS_STAY, because CustomInventory items by their very nature cannot deal with that!

[Tesseract]

I agree it's a mess, but I really think this hotfix is not sufficient.

What if someone will soon create thread complaining that his CustomInventory health replacement is not respected with "Allow health" flag?
I think this case is entirely equivalent - and have good uses, I already made small mod replacing pickups with ones that heal slowly over time (with ACS).
Should new flag be created for all these? In any way, the global solution should be consistent.

Please reconsider the issue and tell me if this matter is really closed or the discussion should continue, perhaps in fresh feature suggestion.

[Graf Zahl]

What I added now was merely replacing a check for a specific actor class with a flag, and on top of that, the logic was faulty to begin with (it forgot to check for weapon pieces.) so it needed to be fixed anyway.

The other stuff you mention wouldn't even fall under the suggested 'CHECKMULTIPLAYERSPAWN' logic.
Looking at the code (it's f*cking awful) it may make sense to add a HEALTHSPAWN and ARMORSPAWN flag, too, to weed out the hideous special cases in those handlers, but that's definitely where I'd stop.

[Tesseract]

I think we're reached good conclusion here, may I ask for these flag additions right here? That will close this whole matter for good. It's probably the best solution,if not backward compatible.

One last thing, I noticed that DF_NO_ITEMS branch is commented out so whole "Allow powerups" option is dead code.

Code: Select all

if (dmflags & DF_NO_ITEMS)
{
//			if (i->IsDescendantOf (RUNTIME_CLASS(AArtifact)))
//				return;
}

I advice one of the following:
1. deprecate the flag and remove the option from the menu, possibly remove whole code branch as well
2. uncomment the code and change the check from AArtifact to new ARTIFACTSPAWN flag for consistency

[Ed the Bat]

I guess this means I can stop inheriting my spawner actor from WeaponGiver, now (that's how I was getting it to honor the mutiplayer-only status).

[NeuralStunner]

The current check is broken anyway because it forgot to check for the WeaponPiece and WeaponGiver class.

I guess this means I can stop inheriting my spawner actor from WeaponGiver, now (that's how I was getting it to honor the mutiplayer-only status).

Funny how these things turn out, isn't it?

This will definitely help for item replacers or Boom-compatible projects. Though for anything with new maps new you should probably be using Hexen format at the least, and that offers unambiguous game mode flags. (Just something for folks to keep in mind.)

[Ed the Bat]

I honestly could've sworn it was working for me, last time I checked. Granted, it's been a little while since I checked, as I only have one co-op ally and he never wants to play except when the damn planets align...

But either way, this new flag will be a simpler way to do it, so I have no complaints.