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

Cookoff - Delay full vehicle destruction #9061

Merged
merged 10 commits into from
May 24, 2024

Conversation

JonBons
Copy link
Contributor

@JonBons JonBons commented Oct 5, 2022

When merged this pull request will:

  • Delay final explosion.
Old description
  • Restricted cookoff jet/ring flame effects to a single execution
  • After the cookoff jet/ring flame effects have finished a finale can happen (based on random roll) that results in catastrophic destruction of the vehicle (ex. https://youtu.be/qfldUOrXCGk)
    • Adds Destruction Finale Chance CBA setting to adjust the random roll chance of finale
    • Adds ace_vehicle_damage_canHaveFinale to disable feature via configs

Depends upon #9060

Copy link
Contributor

@TheCandianVendingMachine TheCandianVendingMachine left a comment

Choose a reason for hiding this comment

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

lgtm. sorry for not approving this earlier, I think I got caught up on the setting and ended up losing time in the year

Copy link
Contributor

@johnb432 johnb432 left a comment

Choose a reason for hiding this comment

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

Documentation on QGVAR(canHaveFinale) needs to be added.

addons/cookoff/initSettings.inc.sqf Outdated Show resolved Hide resolved
@johnb432
Copy link
Contributor

johnb432 commented Apr 4, 2024

Imo we should drop the hard cap for flame-out stages in favor of #9758 's GVAR(cookoffDuration) setting. We can tweak 9758's implementation of the setting, but I think it should be done in a similar fashion.

@johnb432 johnb432 mentioned this pull request Apr 4, 2024
@johnb432 johnb432 added the kind/feature Release Notes: **ADDED:** label Apr 17, 2024
@johnb432 johnb432 added this to the 3.18.0 milestone Apr 17, 2024
`random 1` can return `0`, meaning there was no way of turning this off completely.
@johnb432
Copy link
Contributor

johnb432 commented Apr 17, 2024

Imo we should drop the hard cap for flame-out stages in favor of #9758 's GVAR(cookoffDuration) setting. We can tweak 9758's implementation of the setting, but I think it should be done in a similar fashion.

This was echoed in internal discussion. I'm trying to implement such changes, but it's harder than I initially anticipated.

* _After the cookoff jet/ring flame effects have finished a finale can happen (based on random roll) that results in catastrophic destruction of the vehicle (ex. https://youtu.be/qfldUOrXCGk)_
  
  * _Adds `Destruction Finale Chance` CBA setting to adjust the random roll chance of finale_
  * _Adds `ace_vehicle_damage_canHaveFinale` to disable feature via configs_

Why are we adding a separate config entry for this? We have

if (GVAR(destroyVehicleAfterCookoff) || _detonateAfterCookoff) then {

I understand it isn't quite the same (the only difference I can spot is that it happens at a specific time and that an explosion is spawned), but can't we either live with it or tweak that to get the desired behaviour, instead of adding yet another config entry?

Assuming we scrap the above from this PR, the remaining thing this PR adds is:

* _Restricted cookoff jet/ring flame effects to a single execution_

But that would be also scrapped, in favor of 9758's cookoff duration setting.


I went ahead and pushed a commit with the changes I think are necessary to make it compatible with 9758's cookoff duration setting and added a time between cookoff ending and final explosion happening. The final explosion is controlled by GVAR(destroyVehicleAfterCookoff) || _detonateAfterCookoff solely, the new config entry ace_vehicle_damage_canHaveFinale was removed as I deemed it unnecessary.

These changes fix a bug with the current implementation: QGVAR(intensity) was never updated after the final stage, meaning the PFH runs infinitely long, until the variable is manually set or the vehicle is deleted.

@TheCandianVendingMachine
Copy link
Contributor

I worry that the changes changed the intention of the original PR, but if #9758 does what you say it does it sounds like this PR was going to be defunct anyway. With that in mind, I agree with your points and am approving this

@johnb432 johnb432 changed the title Cookoff - Finale Chance for full vehicle destruction Cookoff - Delay full vehicle destruction May 24, 2024
@LinkIsGrim LinkIsGrim merged commit be77ef2 into acemod:master May 24, 2024
5 checks passed
blake8090 pushed a commit to blake8090/ACE3 that referenced this pull request Aug 18, 2024
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.

5 participants