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.

Sign up for GitHub

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

Jump to bottom

Explosives - Add exclusion from dynamic defuse action #8171

Merged
merged 21 commits into from
Oct 11, 2021
Merged

Explosives - Add exclusion from dynamic defuse action #8171

merged 21 commits into from
Oct 11, 2021

Conversation

Walthzer
Copy link
Contributor

When merged this pull request will

Add the functionality to exclude a mine from the "forced" defuse action the explosives module creates in interactEH

Additions in existing files

  • Initialize GVAR(excludedMines) in XEH_postInit
  • Prep functions in XEH_PREP
  • Allow dynamic exclusion of mines from the defuse actions added in interactEH

New Files

As it works with a GVAR should I add an event to allow easier MP compatibility or is this more of a write-your-own-thing for the developer that wants to call the functions?

BaerMitUmlaut
BaerMitUmlaut previously requested changes Mar 16, 2021
View reviewed changes
addons/explosives/functions/fnc_excludeMine.sqf Outdated Show resolved Hide resolved
addons/explosives/functions/fnc_stopExcludingMine.sqf Outdated Show resolved Hide resolved
Walthzer and others added 2 commits March 16, 2021 11:42
@Walthzer @BaerMitUmlaut
Update addons/explosives/functions/fnc_excludeMine.sqf
e010ed3 
Uhm, strange. Its clearly allMines, yet Allmines didn't only build it also worked properly. Undocumented variable?

Co-authored-by: BaerMitUmlaut <BaerMitUmlaut@users.noreply.github.com>
@Walthzer @BaerMitUmlaut
Update addons/explosives/functions/fnc_stopExcludingMine.sqf
f567aba 
Co-authored-by: BaerMitUmlaut <BaerMitUmlaut@users.noreply.github.com>
@Walthzer Walthzer requested a review from BaerMitUmlaut March 16, 2021 10:49
veteran29
veteran29 requested changes Mar 16, 2021
View reviewed changes
Copy link
Member

@veteran29 veteran29 left a comment

Choose a reason for hiding this comment

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

Does setVariable work on mines? If so why not just do something like _mine setVariable [QGVAR(excluded), true, true].

That way seems cleaner to me and there's no problem with MP synchronization.

@veteran29 veteran29 added this to the 3.13.7 milestone Mar 16, 2021
@Walthzer
Copy link
Contributor Author

Walthzer commented Mar 16, 2021 •
edited
Loading

@veteran29 Nope, sadly setVariable doesn't work on mines. BaerMitUmlaut also suggested this when we discussed it on slack. Would have been the cleanest, but nope not under BI's watch :)

@Walthzer Walthzer mentioned this pull request Mar 16, 2021
Closed
@Walthzer Walthzer requested a review from veteran29 March 16, 2021 14:40
jonpas
jonpas requested changes Apr 20, 2021
View reviewed changes
Copy link
Member

@jonpas jonpas left a comment •
edited
Loading

Choose a reason for hiding this comment

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

As it works with a GVAR should I add an event to allow easier MP compatibility or is this more of a write-your-own-thing for the developer that wants to call the functions?

Maybe event implementation would be better indeed. That way people can just call it with globalEvent if they want it global, or local/targetEvent otherwise (eg. certain people shouldn't be able to defuse).

addons/explosives/functions/fnc_excludeMine.sqf Outdated Show resolved Hide resolved
addons/explosives/functions/fnc_excludeMine.sqf Outdated Show resolved Hide resolved
addons/explosives/functions/fnc_stopExcludingMine.sqf Outdated Show resolved Hide resolved
addons/explosives/functions/fnc_stopExcludingMine.sqf Outdated Show resolved Hide resolved
addons/explosives/XEH_PREP.hpp Outdated Show resolved Hide resolved
Walthzer and others added 5 commits April 20, 2021 13:45
@Walthzer @jonpas
Header Fixes
d8aec56 
Fix information in the function headers

Co-authored-by: jonpas <jonpas33@gmail.com>
@Walthzer
Remove Individual Functions
fcde48b
@Walthzer
Compacter Functions
25f43f2
@Walthzer
Event
7ec7feb
@Walthzer
Merge branch 'allow-defusalAction-exclusion' of https://github.com/Wa…
11c951f 
…lthzer/ACE3 into allow-defusalAction-exclusion
@Walthzer
Copy link
Contributor Author

I have merged the functions and made an event available to call it. Currently the event is named "ace_allowDefuse" a nice accessible name, is this right or should I add the "explosives" prefix?

@Walthzer Walthzer requested a review from jonpas April 20, 2021 14:05
jonpas
jonpas requested changes Apr 20, 2021
View reviewed changes
Copy link
Member

@jonpas jonpas left a comment

Choose a reason for hiding this comment

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

Event name is good.

addons/explosives/XEH_PREP.hpp Outdated Show resolved Hide resolved
addons/explosives/XEH_postInit.sqf Outdated Show resolved Hide resolved
addons/explosives/functions/fnc_allowDefuse.sqf Outdated Show resolved Hide resolved
addons/explosives/functions/fnc_allowDefuse.sqf Outdated Show resolved Hide resolved
addons/explosives/functions/fnc_allowDefuse.sqf Outdated Show resolved Hide resolved
addons/explosives/functions/fnc_allowDefuse.sqf Outdated Show resolved Hide resolved
addons/explosives/functions/fnc_allowDefuse.sqf Outdated Show resolved Hide resolved
addons/explosives/functions/fnc_isAllowedDefuse.sqf Outdated Show resolved Hide resolved
Walthzer and others added 2 commits April 20, 2021 16:59
@Walthzer @jonpas
Jonpass' Review Fixes
196060f 
Co-authored-by: jonpas <jonpas33@gmail.com>
@Walthzer
Fix exitWith mistake
38fe92e
@Walthzer @jonpas
Refractor of allowDefuse
ced426c 
Co-authored-by: jonpas <jonpas33@gmail.com>
@jonpas
Copy link
Member

jonpas commented Apr 20, 2021

Add the new event here as well please.

@Walthzer @jonpas
Update addons/explosives/functions/fnc_allowDefuse.sqf
85a674e 
Co-authored-by: jonpas <jonpas33@gmail.com>
@Walthzer
Copy link
Contributor Author

Add the new event here as well please.

In the "ace_explosives" header I'll assume? Although those looks like Called events not callable's.

@jonpas
Copy link
Member

jonpas commented Apr 20, 2021

Yeah. It's documentation for both, you can reference one of the other sections for a Callable one.

@Walthzer Walthzer requested a review from jonpas April 20, 2021 20:59
@jonpas
Copy link
Member

jonpas commented Oct 10, 2021

Good to merge from my side, it's been a while though so leaving to @PabstMirror to press the button.

@PabstMirror PabstMirror merged commit fad7f84 into acemod:master Oct 11, 2021
@jonpas jonpas added kind/feature Release Notes: **ADDED:** and removed kind/added feature labels Oct 14, 2021
AndreasBrostrom pushed a commit to AndreasBrostrom/ACE3 that referenced this pull request Dec 3, 2021
@Walthzer @BaerMitUmlaut
@jonpas @PabstMirror @AndreasBrostrom
Explosives - Add exclusion from dynamic defuse action (acemod#8171)
810b2dd 
* Add GVAR(noDefusalAction)

* add changes

* Prep functions

* Update addons/explosives/functions/fnc_excludeMine.sqf

Uhm, strange. Its clearly allMines, yet Allmines didn't only build it also worked properly. Undocumented variable?

Co-authored-by: BaerMitUmlaut <BaerMitUmlaut@users.noreply.github.com>

* Update addons/explosives/functions/fnc_stopExcludingMine.sqf

Co-authored-by: BaerMitUmlaut <BaerMitUmlaut@users.noreply.github.com>

* Header Fixes

Fix information in the function headers

Co-authored-by: jonpas <jonpas33@gmail.com>

* Remove Individual Functions

* Compacter Functions

* Event

* remove tab (facepalm)

* Jonpass' Review Fixes

Co-authored-by: jonpas <jonpas33@gmail.com>

* Fix exitWith mistake

* Refractor of allowDefuse

Co-authored-by: jonpas <jonpas33@gmail.com>

* Update addons/explosives/functions/fnc_allowDefuse.sqf

Co-authored-by: jonpas <jonpas33@gmail.com>

* Update Documentation

* Rephrase documentation

* Another rephrase

* Relabel Locality

* Update addons/explosives/functions/fnc_allowDefuse.sqf

Co-authored-by: BaerMitUmlaut <BaerMitUmlaut@users.noreply.github.com>
Co-authored-by: jonpas <jonpas33@gmail.com>
Co-authored-by: PabstMirror <pabstmirror@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PabstMirror PabstMirror PabstMirror approved these changes

@jonpas jonpas jonpas approved these changes

@veteran29 veteran29 veteran29 approved these changes

@BaerMitUmlaut BaerMitUmlaut Awaiting requested review from BaerMitUmlaut

Assignees
No one assigned
Labels
kind/feature Release Notes: **ADDED:**
Projects
None yet
Milestone
3.14.0
Development

Successfully merging this pull request may close these issues.
5 participants
@Walthzer @jonpas @veteran29 @PabstMirror @BaerMitUmlaut