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

Explosives - Add "ace_explosives_setup" CBA Event #9197

Merged
merged 4 commits into from Jun 20, 2023

Conversation

ThatArcanist
Copy link
Contributor

@ThatArcanist ThatArcanist commented May 10, 2023

When merged this pull request will:

  • Add an event to the "fnc_setupExplosive" for when an explosive is successfully placed down.

Currently, the "ace_explosives_place" event only triggers when a "trigger" is added to the explosive. This adds an event that passes in the placed explosive's vehicle, the mag class and the unit that placed the explosive.

Personally, the current event should be renamed and this one should be named "place" however, to avoid fallout with other mods that depend on this one, I've simply created a new one to keep the two separate and not cause other mods to need to be updated.

IMPORTANT

  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Development Guidelines are read, understood and applied.
  • Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

@BrettMayson
Copy link
Member

I have added back the IMPORTANT section of the PR template that was removed. Please complete the 3 items in that section.

@ThatArcanist
Copy link
Contributor Author

ThatArcanist commented May 15, 2023

@BrettMayson The event "ace_explosives_place" is undocumented, would you prefer I left this out of documentation or created a section for the two events (ace_explosives_place and ace_explosives_setup) in the explosives framework documentation

@BrettMayson
Copy link
Member

I would document ace_explosives_place if it is missing

@Drofseh
Copy link
Contributor

Drofseh commented May 15, 2023

Title of this PR should be something like Explosives - Add {changes}

@ThatArcanist ThatArcanist changed the title Improve: Create "ace_explosives_setup" CBA Event Explosives - Add "ace_explosives_setup" CBA Event May 15, 2023
@jonpas jonpas added this to the 3.16.0 milestone May 24, 2023
@jonpas jonpas added the kind/enhancement Release Notes: **IMPROVED:** label May 24, 2023
@LinkIsGrim
Copy link
Contributor

LinkIsGrim commented Jun 20, 2023

Descriptions were fine, just event naming that was backwards.
Disregard.

@ThatArcanist
Copy link
Contributor Author

ThatArcanist commented Jun 20, 2023

@LinkIsGrim Could you be more specific? Both the descriptions (You didn't mention it, but I also noticed it was wrong) and naming was fixed, if there's another issue, I can't see i t

@BrettMayson BrettMayson merged commit 864d2e9 into acemod:master Jun 20, 2023
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants