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

Add spectator handling of grenades #6012

Merged
merged 9 commits into from
Jan 22, 2018
Merged

Add spectator handling of grenades #6012

merged 9 commits into from
Jan 22, 2018

Conversation

AACO
Copy link
Contributor

@AACO AACO commented Jan 3, 2018

When merged this pull request will:

  • Add spectator handling of grenades (vanilla and advanced)

This is needed since grenades and other large/slow projectiles are actually handled globally instead of locally, and the projectile returned to the handler is null.

Note: we can probably drop special handling of advanced throwing grenades if we change the BI's fired event handler to use CBA's (since https://github.com/acemod/ACE3/blob/master/addons/advanced_throwing/functions/fnc_throwFiredXEH.sqf should handle it)

@jonpas
Copy link
Member

jonpas commented Jan 3, 2018

Note: we can probably drop special handling of advanced throwing grenades if we change the BI's fired event handler to use CBA's

Yes, XEH should be used, that's the whole reason of us throwing it.

@AACO
Copy link
Contributor Author

AACO commented Jan 3, 2018

@jonpas do you have a good starting point for that? It looks like my options in CBA are:

  1. add a BIS event handler to the object, which from what I can see wont do what's needed here
    or
  2. Add a class based XEH that would have to be handled every fire, which seems very clunky for the usage here.

Am I missing something, or is there no way to handle adding / removing object based extended events?

@PabstMirror
Copy link
Contributor

xeh doesn't really have a clean way to remove EH,
so I think the current system is the best we can do

@kymckay
Copy link
Member

kymckay commented Jan 5, 2018

So stupid that the fired EH doesn't always pass in the projectile to begin with.

My initial concern was how well this scales, but since it's only running when the projectile is null I think it's fine. Could do worse than nearestObject, plus this code is only running for entities within the distance icons are rendered.

As for the advanced throwing part, I'm not sure that will work because that function is using the _thisEventHandler variable. I think I'd personally just add the event in post init (so it's always there) and have it run:

{
    if (!isNil QGVAR(entitiesToDraw) && {_unit in GVAR(entitiesToDraw)}) then {
        if (count GVAR(grenadesToDraw) > MAX_GRENADES) then { GVAR(grenadesToDraw) deleteAt 0; };
        GVAR(grenadesToDraw) pushBack _projectile;
    };
}

Doesn't really add much overhead since advanced throwing isn't happening often, plus it won't do anything unless spectator is active.

@kymckay
Copy link
Member

kymckay commented Jan 5, 2018

Of course, you could still and and remove the EH if you'd prefer 👍

@AACO
Copy link
Contributor Author

AACO commented Jan 5, 2018

As for the advanced throwing part, I'm not sure that will work because that function is using the _thisEventHandler variable.

The advanced throwing event handler should get removed before the conditions trigger the fired event removal, with that said having dedicated grenade handling code for advanced throwing might not be the worst thing ever, my only concern is an update to grenade handling code has to be tracked in two areas. (For example if you wanted to separate grenade icons by grenade types, you'd have to update the advanced throwing and the fired event handler code). Not the worst thing in the world.

I think I'd personally just add the event in post init (so it's always there)

I'm personally not a fan of adding handlers until they're needed, so I do want to keep the add/remove flow in

@jonpas
Copy link
Member

jonpas commented Jan 5, 2018

I'm personally not a fan of adding handlers until they're needed, so I do want to keep the add/remove flow in

I am not sure about this, but adding/removing a handler might be more expensive overall than adding it on start and then just checking a simple boolean.

@kymckay
Copy link
Member

kymckay commented Jan 5, 2018

The advanced throwing event handler should get removed before the conditions trigger the fired event removal

Keep in mind that units could be removed from the viewable list which were previously in it (via public API or even mid-mission settings change). So this is only true for when spectator is closed, I think dedicated code for advanced throwing is needed.

my only concern is an update to grenade handling code has to be tracked in two areas

We could make a local event to add projectiles into the system, that way it could also be called independently in other components too.

@AACO
Copy link
Contributor Author

AACO commented Jan 5, 2018

@SilentSpike something like that? (also why does circle ci keep failing?)

@jonpas jonpas requested a review from kymckay January 22, 2018 22:07
@jonpas jonpas added the kind/enhancement Release Notes: **IMPROVED:** label Jan 22, 2018
@jonpas jonpas added this to the 3.13.0 milestone Jan 22, 2018
@jonpas
Copy link
Member

jonpas commented Jan 22, 2018

also why does circle ci keep failing?

Just not setup yet from where you branched off, don't worry about it.

Copy link
Member

@kymckay kymckay left a comment

Choose a reason for hiding this comment

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

All looks good 👍

@jonpas jonpas merged commit 8ba81c5 into acemod:master Jan 22, 2018
(_this select 0) setVariable [QGVAR(highlightTime), time + FIRE_HIGHLIGHT_TIME];

// add grenade to tracking
[QGVAR(addToGrenadeTracking), [_this select 6]] CBA_fnc_localEvent;
Copy link
Contributor

Choose a reason for hiding this comment

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

call is missing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch

@PabstMirror PabstMirror modified the milestones: 3.13.0, 3.12.2 Apr 2, 2018
BaerMitUmlaut pushed a commit that referenced this pull request Aug 5, 2019
* Handle non-local vanilla grenade tosses

* handle advanced throwing grenades

* Track through events

* Add missing calls
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:** status/review-pending
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants