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

Medical Status - Remove status effects upon death/featureCamera #9301

Merged
merged 7 commits into from
Aug 18, 2023

Conversation

johnb432
Copy link
Contributor

@johnb432 johnb432 commented Aug 1, 2023

When merged this pull request will:

  • If a unit was unconscious and then died, they would have the setHidden, blockRadio and blockSpeaking status effects still applied. The first is not important, but the two others are, especially blockSpeaking, which (I believe) prevented spectators talking in the spectator view when using TFAR.

This PR reverses those status effects upon death.
Unsure if ACRE is affected in any negative way.

IMPORTANT

  • If the contribution affects the documentation, please include your changes in this pull request so the documentation will appear on the website.
  • Development Guidelines are read, understood and applied.
  • Title of this PR uses our standard template Component - Add|Fix|Improve|Change|Make|Remove {changes}.

@LinkIsGrim
Copy link
Contributor

IMO status effect setting/unsetting should be moved to a function, this will absolutely get confusing in the future.

@LinkIsGrim LinkIsGrim added the kind/bug-fix Release Notes: **FIXED:** label Aug 1, 2023
@johnb432
Copy link
Contributor Author

johnb432 commented Aug 2, 2023

IMO status effect setting/unsetting should be moved to a function, this will absolutely get confusing in the future.

Any suggestions?

@LinkIsGrim
Copy link
Contributor

LinkIsGrim/ACE3@2feacbb, I'm just not sure on skipSetHidden there.

@jonpas jonpas changed the title Medical status - Fix status effects upon death Medical Status - Fix status effects upon death Aug 2, 2023
@johnb432
Copy link
Contributor Author

johnb432 commented Aug 3, 2023

LinkIsGrim/ACE3@2feacbb, I'm just not sure on skipSetHidden there.

I don't think it's necessary, went ahead and removed it.

@LinkIsGrim
Copy link
Contributor

That being the case, the uncon unit will become a valid target for AI upon opening a camera. I don't particularly like that idea due to some communities using spectator for downed players.

@PabstMirror
Copy link
Contributor

imho this is an TFAR problem
all we do is set

_object setVariable ["tf_voiceVolume", [1, 0] select (_set > 0), true];
_object setVariable ["acre_sys_core_isDisabledRadio", _set > 0, true];

if the unit is dead or otherwise spectating that shouldn't matter
made a PR michail-nikolaev/task-force-arma-3-radio#1562 that I think should fix on their end?

@LinkIsGrim
Copy link
Contributor

LinkIsGrim commented Aug 5, 2023

imho this is an TFAR problem all we do is set

_object setVariable ["tf_voiceVolume", [1, 0] select (_set > 0), true];
_object setVariable ["acre_sys_core_isDisabledRadio", _set > 0, true];

if the unit is dead or otherwise spectating that shouldn't matter made a PR michail-nikolaev/task-force-arma-3-radio#1562 that I think should fix on their end?

Poke if that gets merged, this should be unnecessary then.

@jonpas
Copy link
Member

jonpas commented Aug 11, 2023

Close?

@johnb432
Copy link
Contributor Author

Close?

I feel it's poor design on TFAR's part, but I don't quite agree it's their fault.

The PR made to address it on TFAR's end hasn't been merged, so I feel it might be safer to keep this PR open until it's addressed.

@jonpas
Copy link
Member

jonpas commented Aug 11, 2023

It probably will never get merged, and if it will, it'll never get released. @dedmen ?

Does this PR address any other possible issue?

@johnb432
Copy link
Contributor Author

Does this PR address any other possible issue?

Unfortunately it doesn't, as I don't have the necessary information to reproduce the errors. It only handles death of uncon units causing them to be muted as spectators.

@jonpas
Copy link
Member

jonpas commented Aug 11, 2023

It seems like a general bad-state safety improvement to me though, so merge anyways?

@jonpas jonpas added this to the 3.16.0 milestone Aug 11, 2023
@jonpas
Copy link
Member

jonpas commented Aug 11, 2023

That being the case, the uncon unit will become a valid target for AI upon opening a camera. I don't particularly like that idea due to some communities using spectator for downed players.

Actually, if that happens, this PR is a no-go.

@johnb432
Copy link
Contributor Author

Actually, if that happens, this PR is a no-go.

What if I were to revert the changes made in d647710?

The script would only execute upon death then.

@jonpas
Copy link
Member

jonpas commented Aug 11, 2023

Somebody else will have to comment on the rest of the changes.

@dedmen
Copy link
Contributor

dedmen commented Aug 11, 2023

It probably will never get merged, and if it will, it'll never get released. @dedmen ?

Why not? Probably weeks/months but not never

@jonpas
Copy link
Member

jonpas commented Aug 11, 2023

As a "beta". :P

@johnb432
Copy link
Contributor Author

Somebody else will have to comment on the rest of the changes.

I reverted the problematic changes, should be good to be reviewed.

@jonpas
Copy link
Member

jonpas commented Aug 12, 2023

That being the case, the uncon unit will become a valid target for AI upon opening a camera. I don't particularly like that idea due to some communities using spectator for downed players.

@LinkIsGrim re-review please.

@LinkIsGrim LinkIsGrim changed the title Medical Status - Fix status effects upon death Medical Status - Remove status effects upon death/featureCamera Aug 18, 2023
@PabstMirror
Copy link
Contributor

@LinkIsGrim
Copy link
Contributor

Will TFAR be updated any time soon?

@PabstMirror
Copy link
Contributor

https://steamcommunity.com/sharedfiles/filedetails/?id=894678801 had an update Jun 14
but i'm not opposed to this PR in it's current state
has anyone tested if AI start shooting bodies when they go from uncon to death? I doubt they would

@LinkIsGrim
Copy link
Contributor

From SP testing, they don't. AFAIK dead bodies aren't valid targets for the AI brain.

@LinkIsGrim LinkIsGrim merged commit 682ae05 into acemod:master Aug 18, 2023
5 checks passed
@johnb432 johnb432 deleted the tfar-mute-fix branch January 14, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fault/3rd-party-mod kind/bug-fix Release Notes: **FIXED:**
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants