RandomSpawner should preserve the TOSSED flag

[argv]

RandomSpawner currently doesn't preserve the Inventory flag TOSSED. It should; setting up a RandomSpawner as a monster DropItem is perfectly reasonable, but while items dropped directly get the TOSSED flag, items dropped indirectly via a RandomSpawner don't.

My intended use case is an action that pulls loot dropped by monsters toward the player character. Selecting eligible items by their TOSSED status seems like the obvious choice, but that won't work if the monster drops a random item.

[_mental_]

Post a runnable sample please.

[phantombeta]

There's no need for a runnable sample for such a simple issue. One can easily perform a search for "TOSSED" in randomspawner.txt to see that, indeed, there's no code for handling the TOSSED flag.
Oddly enough, the DROPPED flag is handled, but not TOSSED.

[_mental_]

There's no need for a runnable sample for such a simple issue.

Could you please let me (or someone else who will fix this) decide do we need an example or not?
Personally I would like to spend time fixing issues rather than to waste it crafting test samples.
Even if I will do so, there is still a chance that something will be missed from these samples.
I cannot read other people’s minds to express their thoughts and intentions precisely.

[argv]

No problem. Here's a runnable sample of the issue. Once you load it up, there are two things to try:

• summon DoomedImp. This will summon an imp which immediately dies. It drops a RandomSpawner, which becomes either a Clip or Shell. When you pick up the resulting item, it reports whether its TOSSED flag is set.
• summon DoomedPinky. This will summon a pinky which immediately dies. It directly drops a Shell, which also reports whether its TOSSED flag is set.

You'll see that items dropped by the DoomedPinky do have the TOSSED flag set (because no RandomSpawner), while items dropped by the DoomedImp don't (because RandomSpawner).

The big problem I see here is that TOSSED is defined on the Inventory class, but RandomSpawner does not inherit from Inventory, so it doesn't have a TOSSED flag.

Is it perhaps feasible to move this flag to the Actor class instead? It is, after all, perfectly valid to name non-Inventory classes as DropItems, and to call A_DropItem with a non-Inventory class. If this is done, the SpecialDropAction method should probably also be moved to the Actor class.

[Graf Zahl]

Is it perhaps feasible to move this flag to the Actor class instead? It is, after all, perfectly valid to name non-Inventory classes as DropItems, and to call A_DropItem with a non-Inventory class. If this is done, the SpecialDropAction method should probably also be moved to the Actor class.

Absolutely not!
Doom already suffers from far too many problems caused by overgeneralizing the Actor class. In a sane environment there should also be a "Monster" class to inherit from instead of having flags, and there should be a "Projectile" class to serve as a base for projectiles of all kinds and abstract their special behavior. The engine is full of gross inefficiencies because every actor can be everything, thanks to Dehacked.

Just because RandomSpawner has some implementation issues we shouldn't completely obliterate the existing abstraction of the Inventory class.

[argv]

Well, the TOSSED flag is more about where an actor came from than what it is or can do, but I see your point. The Actor class has a ton of flags—223 of them, if I'm not mistaken.

Do you have any other idea of how P_DropItem can inform a newly-spawned, non-inventory actor that it was dropped by another actor?

[Graf Zahl]

Normally that should be MF_DROPPED but obviously that one's been hijacked to death by ZDoom's inventory system, making it mostly useless.

[argv]

I see. Inventory.BeginPlay sets the dropped flag on all inventory items. Actors spawned by ACS or action specials set it. A bunch of Strife pickups are always dropped, even when they're map things.

Why in the world is this the case?! What would break if this behavior were removed?

[Graf Zahl]

I have no idea. I didn't write that code.

[argv]

Are you open to removing it, then? See if anyone complains about their mod breaking?

[Graf Zahl]

Far too dangerous.

[argv]

Can't argue with that.

What about adding a virtual “you've been dropped, and this is the actor that dropped you” method to the Actor class? Would that also unacceptably increase overhead, like a new flag or field would?

[Caligari87]

Well, just tossing my hat in the ring to say my mod would certainly break if it were removed. I have a WorldThingSpawned eventhandler, and apparently it twigs to things in the player's inventory (including spawning stuff at (0,0) if I try to use e.thing.pos), so I have it abort if the thing has the DROPPED flag.

[argv]

If you want to ignore items that are in an actor's inventory, use an expression like this:

Code: Select all

Actor actor = …;
if (!(actor is "Inventory" && Inventory(Actor).Owner)) {
    // This block only runs if the actor is not an item in someone's inventory.
}