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

Gunbag - Add weapon swapping #7713

Merged
merged 21 commits into from
Jun 19, 2020
Merged

Gunbag - Add weapon swapping #7713

merged 21 commits into from
Jun 19, 2020

Conversation

mjc4wilton
Copy link
Contributor

@mjc4wilton mjc4wilton commented May 16, 2020

Adds the capability to swap the currently held primary weapon and the weapon currently stored in the gunbag. Has a 1.5x (7.5 seconds) time to complete compared to just adding or removing a weapon from the gunbag without a primary for balance purposes.

When merged this pull request will:

adds capability to swap a currently held primary weapon and the weapon current stored in the gunbag. Has a 1.5x time to complete compared to just adding or removing a weapon from the gunbag.
@jokoho48
Copy link
Member

this PR looks 1 to 1 like #6212 but with changed authors in headers and no keybindings.
removing someone else as author is not the gentlemen way

@commy2
Copy link
Contributor

commy2 commented May 17, 2020

If it is the exact same thing as #6212, then this one can be closed.

@commy2
Copy link
Contributor

commy2 commented May 17, 2020

Btw, I don't want either of them. And it is not me not liking the code, but the feature.

@jonpas
Copy link
Member

jonpas commented May 17, 2020

Then don't use it. We went over this before.

@commy2
Copy link
Contributor

commy2 commented May 17, 2020

Sure, but

jokoho48 requested your review on this pull request.

on the other one. I am not going to approve something I don't like.

Copy link
Member

@jokoho48 jokoho48 left a comment

Choose a reason for hiding this comment

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

In addition to the changes, I would recommend adding a keybinding.
I would also not put automatically generated translations in there and keep it open for translators todo. The current one for German is fairly bad.

addons/gunbag/functions/fnc_swapWeapon.sqf Outdated Show resolved Hide resolved
addons/gunbag/functions/fnc_swapWeaponCallback.sqf Outdated Show resolved Hide resolved
addons/gunbag/functions/fnc_swapWeaponCallback.sqf Outdated Show resolved Hide resolved
@jokoho48 jokoho48 changed the title gunbag - Improve | add weapon swapping Gunbag - Add weapon swapping May 17, 2020
mjc4wilton and others added 3 commits May 17, 2020 10:53
Update authors field to add credit to the original author of much of the changed code

Co-authored-by: Joko <hoffman.jonas95@gmail.com>
Update the virtual load in a more efficient way.

Co-authored-by: Joko <hoffman.jonas95@gmail.com>
Properly attribute author of majority of original code

Co-authored-by: Joko <hoffman.jonas95@gmail.com>
@mjc4wilton
Copy link
Contributor Author

A few points:

  • I wrote this for our private unit's modpack as we occasionally run a sniper element in a QRF scenario when we organize ourselves into multiple platoons. I submitted this pull request primarily so we do not need to manually override the pbo whenever we update ACE and because IMO its a feature that should be in already.
  • I did not know about Add Switching of Weapon from gunbag. #6212. If this code is identical to that one its one heck of a coincidence.
  • I do not often script stuff on larger projects with multiple people. Apologies for attribution issues. Should be fixed now. I'm in no way trying to plagiarize other peoples' work because thats a low move.
  • I personally do not believe this action needs a keybind. Currently there are no keybinds for the other gunbag actions as far as I am aware, I am not sure why this needs one. I feel that adding a keybind will go against a lot of the discussion about balance in issue Cant switch gunbag weapon with active primary weapon #4119, which is why I modified the time it takes for this action to be 1.5x that of just removing / adding a weapon without another primary.
  • Apologies for horrendous translations. I can easily remove them and just leave English if that is preferred.

@mjc4wilton
Copy link
Contributor Author

I just updated the Stringtable. I went around my unit and found people to translate to most languages. German, French, Russian, and Czech have all been translated by natives of their respective languages. I then went through the remaining Japanese, Polish, Korean, Italian, Chinese (Simplified), and Chinese and used online translators back on fourth to get them as close as I could. I've cross checked them all back to English in as many translators as I could find and I believe they are correct.

@mjc4wilton mjc4wilton requested a review from jokoho48 May 17, 2020 18:26
@mjc4wilton
Copy link
Contributor Author

Everything tested in-game up to this point

addons/gunbag/ACE_Settings.hpp Outdated Show resolved Hide resolved
addons/gunbag/initSettings.sqf Outdated Show resolved Hide resolved
addons/gunbag/initSettings.sqf Outdated Show resolved Hide resolved
mjc4wilton and others added 2 commits May 22, 2020 10:46
Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
@jonpas jonpas added the kind/enhancement Release Notes: **IMPROVED:** label May 22, 2020
@mjc4wilton
Copy link
Contributor Author

Everything has been tested and works in-game up to this point

addons/gunbag/functions/fnc_swapGunbag.sqf Outdated Show resolved Hide resolved
addons/gunbag/functions/fnc_swapGunbagCallback.sqf Outdated Show resolved Hide resolved
addons/gunbag/functions/fnc_swapGunbagCallback.sqf Outdated Show resolved Hide resolved
addons/gunbag/initSettings.sqf Outdated Show resolved Hide resolved
mjc4wilton and others added 4 commits May 22, 2020 21:58
Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
@mjc4wilton mjc4wilton requested a review from jokoho48 May 28, 2020 04:21
Copy link
Member

@jokoho48 jokoho48 left a comment

Choose a reason for hiding this comment

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

LGTM except for the small optimisation

@PabstMirror PabstMirror added this to the 3.13.3 milestone Jun 7, 2020
addons/gunbag/CfgVehicles.hpp Outdated Show resolved Hide resolved
addons/gunbag/CfgVehicles.hpp Outdated Show resolved Hide resolved
Copy link
Contributor

@PabstMirror PabstMirror left a comment

Choose a reason for hiding this comment

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

looks good overall

Copy link
Contributor

@PabstMirror PabstMirror left a comment

Choose a reason for hiding this comment

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

minor changes

mjc4wilton and others added 3 commits June 13, 2020 20:48
Co-authored-by: PabstMirror <pabstmirror@gmail.com>
Change default value to true
@mjc4wilton mjc4wilton requested a review from PabstMirror June 14, 2020 18:49
@PabstMirror PabstMirror merged commit 7d4a2b0 into acemod:master Jun 19, 2020
@mjc4wilton mjc4wilton deleted the gunbagFix branch June 20, 2020 22:58
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.

Cant switch gunbag weapon with active primary weapon
8 participants