RandomSpawner should preserve the TOSSED flag

Is there something that doesn't work right in the latest GZDoom? Post about it here.

Moderator: GZDoom Developers

Forum rules
Please construct and post a simple demo whenever possible for all bug reports. Please provide links to everything.

If you can include a wad demonstrating the problem, please do so. Bug reports that include fully-constructed demos have a much better chance of being investigated in a timely manner than those that don't.

Please make a new topic for every bug. Don't combine multiple bugs into a single topic. Thanks!
argv
Posts: 184
Joined: Tue Aug 30, 2016 4:47 pm

RandomSpawner should preserve the TOSSED flag

Post by 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_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: RandomSpawner should preserve the TOSSED flag

Post by _mental_ »

Post a runnable sample please.
User avatar
phantombeta
Posts: 2084
Joined: Thu May 02, 2013 1:27 am
Operating System Version (Optional): Windows 10
Graphics Processor: nVidia with Vulkan support
Location: Brazil

Re: RandomSpawner should preserve the TOSSED flag

Post by 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_
 
 
Posts: 3812
Joined: Sun Aug 07, 2011 4:32 am

Re: RandomSpawner should preserve the TOSSED flag

Post by _mental_ »

phantombeta wrote: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
Posts: 184
Joined: Tue Aug 30, 2016 4:47 pm

Re: RandomSpawner should preserve the TOSSED flag

Post by 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.
Attachments
tossed-demo.pk3
(1002 Bytes) Downloaded 78 times
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: RandomSpawner should preserve the TOSSED flag

Post by Graf Zahl »

argv wrote: 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
Posts: 184
Joined: Tue Aug 30, 2016 4:47 pm

Re: RandomSpawner should preserve the TOSSED flag

Post by 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?
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: RandomSpawner should preserve the TOSSED flag

Post by 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
Posts: 184
Joined: Tue Aug 30, 2016 4:47 pm

Re: RandomSpawner should preserve the TOSSED flag

Post by 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. :bang:

Why in the world is this the case?! What would break if this behavior were removed?
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: RandomSpawner should preserve the TOSSED flag

Post by Graf Zahl »

I have no idea. I didn't write that code.
argv
Posts: 184
Joined: Tue Aug 30, 2016 4:47 pm

Re: RandomSpawner should preserve the TOSSED flag

Post by argv »

Are you open to removing it, then? See if anyone complains about their mod breaking?
User avatar
Graf Zahl
Lead GZDoom+Raze Developer
Lead GZDoom+Raze Developer
Posts: 49056
Joined: Sat Jul 19, 2003 10:19 am
Location: Germany

Re: RandomSpawner should preserve the TOSSED flag

Post by Graf Zahl »

Far too dangerous.
argv
Posts: 184
Joined: Tue Aug 30, 2016 4:47 pm

Re: RandomSpawner should preserve the TOSSED flag

Post by 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?
User avatar
Caligari87
Admin
Posts: 6174
Joined: Thu Feb 26, 2004 3:02 pm
Preferred Pronouns: He/Him
Contact:

Re: RandomSpawner should preserve the TOSSED flag

Post by 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.

8-)
argv
Posts: 184
Joined: Tue Aug 30, 2016 4:47 pm

Re: RandomSpawner should preserve the TOSSED flag

Post by 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.
}
Post Reply

Return to “Bugs [GZDoom]”