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

Fastrope animations and in-line privatization #5501

Merged
merged 3 commits into from Sep 19, 2017
Merged

Fastrope animations and in-line privatization #5501

merged 3 commits into from Sep 19, 2017

Conversation

jonpas
Copy link
Member

@jonpas jonpas commented Sep 11, 2017

When merged this pull request will:

  • Add animations from older Arma games (support for eg. CUP which are still using those - not including any special new animations like fancy ramps)
  • Macroize animations names
  • Change to in-line privatization
  • Fix inverted comments for public onPrepare(/Cut)Common functions

@jonpas jonpas added kind/cleanup Release Notes: **CHANGED:** kind/enhancement Release Notes: **IMPROVED:** status/review-pending labels Sep 11, 2017
@jonpas jonpas added this to the 3.11.0 milestone Sep 11, 2017
BaerMitUmlaut
BaerMitUmlaut requested changes Sep 12, 2017
View reviewed changes
addons/fastroping/script_component.hpp Outdated

#define ANIMS_HOOK ["extendHookRight", "extendHookLeft"]
#define ANIMS_DOOR ["door_R", "door_L", "CargoRamp_Open", "Door_rear_source", "Door_6_source", "CargoDoorR", "CargoDoorL"]
#define ANIMS ["dvere1_posunZ", "dvere2_posunZ", "doors"]
Copy link
Member

Choose a reason for hiding this comment

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

Could we find a different name for this macro? "Anims" is so generic.

Copy link
Contributor

Choose a reason for hiding this comment

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

What are they for? The names aren't really descriptive. Could use ANIMS_BENCH, ANIMS_RAMP, or ANIMS_RAIL.

Copy link
Member Author

Choose a reason for hiding this comment

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

ANIMS_DOOR -> animateDoor (requires "door" source)
ANIMS -> animate ("user" source)

Copy link
Member Author

Choose a reason for hiding this comment

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

The only reason they are split in two is because they require different commands (above comment).

I have no idea what else to name it, suggestions?

Copy link
Contributor

Choose a reason for hiding this comment

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

ANIMS_USER then?

Copy link
Member Author

Choose a reason for hiding this comment

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

I named them ANIMS_ANIMATEDOOR and ANIMS_ANIMATE to make clear what command is which. Is that good or should I go with ANIMS_DOOR and ANIMS_USER rather? One is what command is used, the other is what type is used in config.

@jonpas jonpas merged commit 333b24e into master Sep 19, 2017
@jonpas jonpas deleted the fastropeStuff branch September 19, 2017 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@654wak654 654wak654 654wak654 left review comments

@BaerMitUmlaut BaerMitUmlaut BaerMitUmlaut approved these changes

Assignees
No one assigned
Labels
kind/cleanup Release Notes: **CHANGED:** kind/enhancement Release Notes: **IMPROVED:**
Projects
None yet
Milestone
3.11.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jonpas @654wak654 @BaerMitUmlaut