RandomSpawner should preserve the TOSSED flag

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

Moderator: 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.

RandomSpawner should preserve the TOSSED flag

Postby argv » Sun Jul 08, 2018 12:20 am

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.
argv
 
Joined: 30 Aug 2016

Re: RandomSpawner should preserve the TOSSED flag

Postby _mental_ » Sun Jul 08, 2018 12:23 am

Post a runnable sample please.
_mental_
 
 
 
Joined: 07 Aug 2011

Re: RandomSpawner should preserve the TOSSED flag

Postby phantombeta » Sun Jul 08, 2018 12:34 am

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.
User avatar
phantombeta
In the meadow of sinful thoughts, every flower's a perfect one
 
Joined: 02 May 2013
Location: The United Soviet Socialist Dictatorship of Hueland
Discord: phantombeta#2461
Twitch ID: phantombeta_

Re: RandomSpawner should preserve the TOSSED flag

Postby _mental_ » Sun Jul 08, 2018 1:13 am

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.
_mental_
 
 
 
Joined: 07 Aug 2011

Re: RandomSpawner should preserve the TOSSED flag

Postby argv » Mon Jul 09, 2018 3:38 am

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 7 times
argv
 
Joined: 30 Aug 2016

Re: RandomSpawner should preserve the TOSSED flag

Postby Graf Zahl » Mon Jul 09, 2018 3:55 am

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.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: RandomSpawner should preserve the TOSSED flag

Postby argv » Wed Jul 11, 2018 3:00 am

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?
argv
 
Joined: 30 Aug 2016

Re: RandomSpawner should preserve the TOSSED flag

Postby Graf Zahl » Wed Jul 11, 2018 3:54 am

Normally that should be MF_DROPPED but obviously that one's been hijacked to death by ZDoom's inventory system, making it mostly useless.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: RandomSpawner should preserve the TOSSED flag

Postby argv » Wed Jul 11, 2018 5:12 pm

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?
argv
 
Joined: 30 Aug 2016

Re: RandomSpawner should preserve the TOSSED flag

Postby Graf Zahl » Wed Jul 11, 2018 5:48 pm

I have no idea. I didn't write that code.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: RandomSpawner should preserve the TOSSED flag

Postby argv » Thu Jul 12, 2018 12:03 am

Are you open to removing it, then? See if anyone complains about their mod breaking?
argv
 
Joined: 30 Aug 2016

Re: RandomSpawner should preserve the TOSSED flag

Postby Graf Zahl » Thu Jul 12, 2018 1:28 am

Far too dangerous.
User avatar
Graf Zahl
Lead GZDoom Developer
 
Joined: 19 Jul 2003
Location: Germany

Re: RandomSpawner should preserve the TOSSED flag

Postby argv » Thu Jul 12, 2018 7:28 pm

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?
argv
 
Joined: 30 Aug 2016

Re: RandomSpawner should preserve the TOSSED flag

Postby Caligari87 » Thu Jul 12, 2018 7:59 pm

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-)
User avatar
Caligari87
I'm just here for the community
User Accounts Assistant
 
Joined: 26 Feb 2004
Location: Salt Lake City, Utah, USA
Discord: Caligari87#3089

Re: RandomSpawner should preserve the TOSSED flag

Postby argv » Thu Jul 12, 2018 8:29 pm

If you want to ignore items that are in an actor's inventory, use an expression like this:
Code: Select allExpand view
Actor actor = …;
if (!(actor is "Inventory" && Inventory(Actor).Owner)) {
    // This block only runs if the actor is not an item in someone's inventory.
}
argv
 
Joined: 30 Aug 2016


Return to Bugs

Who is online

Users browsing this forum: No registered users and 0 guests