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

assignGear changes #215

Merged
merged 13 commits into from
Mar 16, 2019
Merged

assignGear changes #215

merged 13 commits into from
Mar 16, 2019

Conversation

shadow-fa
Copy link
Collaborator

@shadow-fa shadow-fa commented Feb 9, 2019

@shadow-fa shadow-fa added this to the 3.5.3 milestone Feb 9, 2019
Copy link
Member

@SamLex SamLex left a comment

Choose a reason for hiding this comment

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

Partial review

#210, #211 and #214 seem to be fixed. Not looking at #213 changes until open questions resolved.

@shadow-fa shadow-fa requested a review from SamLex March 7, 2019 18:51
@SamLex SamLex added the PR:Review In Progress PR is under code review label Mar 10, 2019
Copy link
Member

@SamLex SamLex left a comment

Choose a reason for hiding this comment

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

Partial review (see comments also):

  • Headers in some of the _standard files are wrong. For example, f_assignGear_aaf_standard.sqf says "Light Loadout".
  • Extra space after addBackpack in some of the _standard and _light "mtrg" classes.
  • Light FIA, FIA AK, and Syndicate AR all still have 5 smokes.

@@ -228,7 +232,7 @@ switch (_typeofUnit) do
_unit addmagazines [_smokegrenade, 3];
_unit addmagazines [_carbinemag, 2];
_unit addmagazines [_carbinemag_tr, 2];
_unit addmagazines [_MATmag1, 2];
_unit addmagazines [_MATmag1, 4];
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be an increase in _carbinemag rather than an increase in _MATmag1?

_unit addmagazines [_MATmag1, 2];
_unit addmagazines [_carbinemag, 2];
_unit addmagazines [_carbinemag_tr, 2];
_unit addmagazines [_MATmag1, 4];
Copy link
Member

Choose a reason for hiding this comment

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

Same as AAF, should this not be 4 _carbinemag?

@SamLex SamLex removed the PR:Review In Progress PR is under code review label Mar 10, 2019
@SamLex SamLex assigned shadow-fa and unassigned SamLex Mar 10, 2019
@shadow-fa shadow-fa assigned SamLex and unassigned shadow-fa Mar 10, 2019
@shadow-fa shadow-fa requested a review from SamLex March 10, 2019 20:02
@SamLex SamLex added the PR:Review In Progress PR is under code review label Mar 16, 2019
Copy link
Member

@SamLex SamLex 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 as far as I can tell. I think all the changes documented in #213 have been applied and that #210, #211. and #214 have been fixed, but given the size of these changes it's hard to be sure. Same goes for testing, aside from basic "does anything break" testing this can only really be tested 'in the field' via MM feedback. What I can say for sure is that the _light and _standard loadouts are now synchronised between factions and each other (but not that they are correctly synchronised).

As such, I am happy to merge this. @shadow-fa Do you want someone else to review before I merge this?

@SamLex SamLex removed the PR:Review In Progress PR is under code review label Mar 16, 2019
@SamLex SamLex merged commit 30b2947 into folkarps:dev Mar 16, 2019
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.

2 participants