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

replace usage of setCaptive with camouflageCoef trait #5864

Merged
merged 4 commits into from
Dec 7, 2017

Conversation

commy2
Copy link
Contributor

@commy2 commy2 commented Dec 6, 2017

When merged this pull request will:

  • title
  • same effect, same locality (AL, EG)
  • same ai behavior
  • better mission compatibility (many use setCaptive, very unlikely they use setUnitTrait CamoCoef mid mission)
  • no moving to the civ side which breaks map group marker and probably many other things

@commy2 commy2 added the kind/feature Release Notes: **ADDED:** label Dec 6, 2017
kymckay
kymckay previously approved these changes Dec 6, 2017
Copy link
Member

@kymckay kymckay left a comment

Choose a reason for hiding this comment

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

Seems like a much better approach for compatibility

@jonpas jonpas added this to the 3.12.0 milestone Dec 6, 2017
@kymckay kymckay dismissed their stale review December 6, 2017 16:11

Realised this affects captives module too, should add new event for camouflage

@PabstMirror
Copy link
Contributor

PabstMirror commented Dec 7, 2017

Tested a bit in mp

  • Tried effect as non-global, unitTrait still synced globally as it says it should, but it took several seconds; so I think keeping it as a global statusEffect is a good idea just like setCaptive.

  • Don't really like getting the enum errors, but probably most people don't run with show script errors. For ace_movement I added INFO("Note: getUnitTrait / loadCoef enum error can be ignored [present in ARMA 1.78 RC]"); which we might want here to prevent false issue reports.

  • Tried [player, "setHidden", "testing", true] call ace_common_fnc_statusEffect_set and while walking around an AI he threw a grenade at me (really surprised me). So they can still hear you and may continue to engage.

With setCaptive, it's like you turn into a friendly civ, so they probably actively try not to shoot at you or splash you with frag.
With this it's just that they can't see you and just like hiding in smoke they may keep firing or launch GPs.

I actually think that might be an improvement and be more realistic.

@commy2
Copy link
Contributor Author

commy2 commented Dec 7, 2017

The hearing you part could maybe be fixed by also setting audibleCoef in the same way. I don't know if that makes them stop lobbing grenades though.

@kymckay
Copy link
Member

kymckay commented Dec 7, 2017

For the cause of unconscious I don't think hearing you is an issue since you can't move anyway

@commy2 commy2 merged commit 4491225 into master Dec 7, 2017
@PabstMirror PabstMirror deleted the avoid-setCaptive branch December 7, 2017 22:37
@carlosleung
Copy link

This made me still get shoot by the enemy,When I lost consciousness in front of the him

@bux
Copy link
Member

bux commented Jan 4, 2018

@carlosleung Please don't revive old issues and pull requests.
Searching the current or resolved issues would have shown you that this is already known and even already fixed.

kymckay added a commit that referenced this pull request Jun 27, 2019
This just reimplemented the behaviour from #5864

I believe we though using `setUnconscious` command would make this
unnecessary, but testing seems to show that's not the case
PabstMirror pushed a commit that referenced this pull request Jun 27, 2019
This just reimplemented the behaviour from #5864

I believe we though using `setUnconscious` command would make this
unnecessary, but testing seems to show that's not the case
BaerMitUmlaut pushed a commit that referenced this pull request Aug 5, 2019
replace usage of setCaptive with camouflageCoef trait
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature Release Notes: **ADDED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants