Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added WorldHitscanFired and WorldHitscanPreFired #2432

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jekyllgrim
Copy link
Contributor

@jekyllgrim jekyllgrim commented Mar 4, 2024

This adds WorldHitscanFired and WorldHitscanPreFired events to P_LineAttack and P_RailAttack. The former is called after a hitscan has been fired and provides information about it; the latter before it and allows stopping it. The idea is to allow mod authors universally detect hitscans, and potentially replace them with projectiles, which hasn't been possible so far.

THE PROBLEM is that in its current form, despite compiling properly, it leads to a mysterious crash on startup, presumably tripping up the file system. This is caused by WorldHitscanPrefired specifically; the other features were tested and work fine, but as soon as I added WorldHitscanPrefired, the crashes started happening. Unfortunately, I cannot nail down the issue myself.

UPD: I can't believe my inattention; the whole problem was that WorldHitscanPreFired was defined without a return value in zscript, and apparently that causes a CTD on startup rather than a build error.

Test file. This replaces hitscans with plasma balls and reports on received values.
WorldHitscan.zip

@madame-rachelle
Copy link
Collaborator

Yeah it's hard to pin down the issue with this code since it trips up something else entirely within the new file system.

@MajorCooke
Copy link
Contributor

Report it as a bug. If anything, Graf can take a look at it.

@jekyllgrim
Copy link
Contributor Author

Report it as a bug. If anything, Graf can take a look at it.

That's what I'm doing, more or less? I can't really report it as a bug for the current version of GZDoom because this issue occurs only after my personal changes on the cpp side.

@jekyllgrim jekyllgrim marked this pull request as draft March 13, 2024 18:43
@jekyllgrim jekyllgrim marked this pull request as ready for review March 15, 2024 14:51
@jekyllgrim jekyllgrim marked this pull request as draft March 15, 2024 14:51
@jekyllgrim
Copy link
Contributor Author

@coelckers Looks like it's not something I can solve on my end. Would you be able to debug this? I'm not sure how to progress, because it's not a draft anymore, all the stuff I wanted to add is there, but it's also not really "ready" because it causes a crash in its current form.

@jekyllgrim jekyllgrim changed the title Added bISPUFF, WorldHitscanFired, WorldHitscanPreFired [draft] Added bISPUFF, WorldHitscanFired, WorldHitscanPreFired [compiles but crashes] Mar 29, 2024
@@ -311,6 +311,8 @@ class DStaticEventHandler : public DObject // make it a part of normal GC proces
void WorldThingRevived(AActor* actor);
void WorldThingDamaged(AActor* actor, AActor* inflictor, AActor* source, int damage, FName mod, int flags, DAngle angle);
void WorldThingDestroyed(AActor* actor);
bool WorldHitscanPreFired(AActor* actor, DAngle angle, double distance, DAngle pitch, int damage, FName damageType, PClassActor *pufftype, int flags, double sz, double offsetforward, double offsetside);
void WorldHitscanFired(AActor* actor, DVector3 AttackPos, DVector3 DamagePosition, AActor* Inflictor, int flags);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure to pass vectors by reference value (const DVector3&) to avoid recreating them

src/events.h Outdated
double AttackOffsetForward;
double AttackOffsetSide;
double AttackZ;
PClassActor* AttackPuffType;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doubles and pointers need default initialization values, otherwise they'll be corrupted

@jekyllgrim
Copy link
Contributor Author

This is finally fixed and tested. No performance impact that I could see.

@jekyllgrim jekyllgrim changed the title Added bISPUFF, WorldHitscanFired, WorldHitscanPreFired [compiles but crashes] Added WorldHitscanFired and WorldHitscanPreFired Oct 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants