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

Add revive modules for Farooq's Revive #386

Merged
merged 21 commits into from
May 14, 2019

Conversation

ServiusHack
Copy link
Contributor

When merged this pull request will:

  • The Instant Revive module will immediately revive the selected player
    when Farooq's Revive script is active. It still takes seconds for the
    player to be up because of Farooq's Revive script.

  • The Immersive Revive module sends a selected AI unit to the nearest
    down player to revive him. This gives a more realistic feeling. Also
    works on vehicles which will drive up to the player before the driver
    exits and revives the player.

@CreepPork CreepPork added feature PR that adds a new feature to Achilles. priority/low Issue or PR that has no significant impact and does not negatively impact the current user base. 3rd party mod Issue or PR that involves 3rd party mods. testing/not tested Indicates that the PR's latest commit has not been tested and needs testing before merging. labels Sep 30, 2018
@CreepPork CreepPork added this to the Backlog milestone Sep 30, 2018
@CreepPork
Copy link
Member

Hi, I'm impressed with the code quality and glad to see another contributor joining in (I'll do a review a bit later, I have some things that I will nitpick about and some stuff due to the upcoming 1.2.0 update) but I'm more concerned about making these into separate modules and not integrating it into the Heal module.

I see an issue with it being directly related to Farooq's Revive which isn't in the vanilla game and creating separate modules for just Farooq's Revive seems a bit excessive to me personally (as the custom module framework would be better suited to this), however, if you would integrate it into the heal module and perhaps add dialog support then you could add all your features and not create two separate modules.

Copy link
Member

@CreepPork CreepPork left a comment

Choose a reason for hiding this comment

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

The code is good only minor things I'd prefer differently.

@ServiusHack
Copy link
Contributor Author

@CreepPork thanks for the very fast reply. It's mostly separate modules because it's my first work on Achilles and dialogs were way over my head then. I'm more comfortable with the code base now so I'll look at integrating this into the heal module.

@CreepPork
Copy link
Member

CreepPork commented Sep 30, 2018

@CreepPork thanks for the very fast reply. It's mostly separate modules because it's my first work on Achilles and dialogs were way over my head then. I'm more comfortable with the code base now so I'll look at integrating this into the heal module.

There's nothing to worry, we all have to start somewhere. Feel free to join the Achilles Discord (the link is in the readme) and we can help you start moving it over to the Heal module.

Also, you can take a look at the dialog documentation (it still references the old dialog function as we haven't updated the docs yet but it still gives a bit of insight into the dialog).

The ACE Heal module supports reviving the selected player when Farooq's Revive
script is active. It still takes seconds for the player to be up because of
Farooq's Revive script.
The new Immersive Heal module gives a more realistic feeling compared to the
ACE Heal module by sending an AI to revive or heal a player. It has two modes
of operation:

* When placed on a player the nearest AI of the same side will be sent to
  revive the player.

* When placed on an AI the nearest player (regardless of side) will be revived.
  Also works on vehicles which will drive up to the player before the driver
  exits and revives the player.
@ServiusHack
Copy link
Contributor Author

I've pushed completely reworked commits that also implement a bit more than there was initially. Looking forward to your opinions.

@ServiusHack
Copy link
Contributor Author

Let me know if I should rebase and squash for a nicer set of commits.

neilzar
neilzar previously approved these changes Jan 30, 2019
@neilzar
Copy link
Collaborator

neilzar commented Jan 30, 2019

No need to, it automatically gets squashed when merged.

@CreepPork CreepPork requested a review from neilzar March 11, 2019 11:15
@CreepPork CreepPork modified the milestones: Backlog, 1.3.0 Mar 19, 2019
@CreepPork CreepPork added testing/not tested Indicates that the PR's latest commit has not been tested and needs testing before merging. testing/tested Indicates that the PR's latest commit has been tested and is working properly. and removed testing/not tested Indicates that the PR's latest commit has not been tested and needs testing before merging. testing/tested Indicates that the PR's latest commit has been tested and is working properly. labels Apr 27, 2019
@CreepPork CreepPork merged commit 52d9d54 into ArmaAchilles:master May 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party mod Issue or PR that involves 3rd party mods. feature PR that adds a new feature to Achilles. priority/low Issue or PR that has no significant impact and does not negatively impact the current user base. testing/tested Indicates that the PR's latest commit has been tested and is working properly.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants