ZDoom / gzdoom

GZDoom is a feature centric port for all Doom engine games, based on ZDoom, adding an OpenGL renderer and powerful scripting capabilities
http://zdoom.org
GNU General Public License v3.0
2.47k stars 539 forks source link

[BUG] Items cannot obtain correct owner.height when being picked up from world #2579

Open jekyllgrim opened 4 months ago

jekyllgrim commented 4 months ago

GZDoom version

gd9f03863b

Which game are you running with GZDoom?

Doom 2

What Operating System are you using?

Windows 10

Please describe your specific OS version

Windows 10 Pro

Relevant hardware info

No response

Have you checked that no other similar issue already exists?

A clear and concise description of what the bug is.

The issue: If you read owner.height in a Powerup's InitEffect() (upd: or from any other Inventory function executed at the moment when an item is received), it'll be incorrectly reported to be 80 (even if the actual height is different, such as the default 56) if that powerup was received from a PowerupGiver picked up from world. Giving that Powerup OR that PowerupGiver directly (e.g. via the give cheat) does not cause this issue.

The pointer is obtained correctly (checked with Console.Printf).

This was tested on fresh portable version of GZDoom with empty config.

Steps to reproduce the behaviour.

Explain how to reproduce

  1. Run GZDoom with the attached file. powerupOwnerHeightBug.zip

  2. Type summon PowerupGiverTest in the console.

  3. Pick up the spawned powerup giver (it'll have the BAL1A0 sprite).

  4. Observe the print in the console where it says owner.height is 80.

  5. Wait until the powerup runs out (it takes 5 seconds).

  6. Type give PowerupGiverTest OR give PowerupHeigthTest in the console.

  7. Observe the print where it says owner.height is 56.

image

Your configuration

https://gist.github.com/jekyllgrim/a545567fa17f7cfb69b7b1dd6c813a79.js

Provide a Log

No response

jekyllgrim commented 4 months ago

UPD: same happens if AttachToOwner is used instead of InitEffect. The current workaround is to have a custom "init" run from DoEffect instead, because that gets proper values.

UPD2: not related to Powerup/PowerupGiver. This happens in all items on pickup; a powerup was just a good illustration.

Boondorl commented 4 months ago

For posterity, the answer: when doing the collision check, the checker lazily adds MaxStepHeight to the Actor's height. This should be fixed properly by using a checkHeight argument for PIT_CheckThing. This primarily happens because Actors (mainly players), when looking for things to step up, need to check they're not going to bonk their head on another Actor if they're going to step up something. The extra headspace will achieve this but gives an incorrect height value for the purpose of every check should it be desired to use the Actor's true height during it,

coelckers commented 4 months ago

That height increase causes other problems as well, it's one of those poorly conceived legacy ZDoom things that should have been handled differently.