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

Fix GLA Terrorist passenger crash #208

Closed
wants to merge 1 commit into from
Closed

Fix GLA Terrorist passenger crash #208

wants to merge 1 commit into from

Conversation

alanblack166
Copy link
Contributor

@alanblack166 alanblack166 commented Sep 4, 2021 •
edited by xezon
Loading

Terrorists crashed the game in two scenarios. This commit fixes the crash in both instances.

@commy2
Copy link
Collaborator

commy2 commented Sep 4, 2021

This is WIP.

@commy2 commy2 self-requested a review September 4, 2021 22:43
@xezon
Copy link
Collaborator

xezon commented Sep 6, 2021

What is missing?

@alanblack166
Copy link
Contributor Author

Need to finish the other generals.

@commy2
Copy link
Collaborator

commy2 commented Sep 6, 2021

Also currently the added deaths are handled by two SlowDeath modules. See ModuleTag_Death01. This will make the game chose one at random.

@xezon
Copy link
Collaborator

xezon commented Sep 10, 2021

Crash is still reproducible.

@xezon xezon added the Bug Something is not working right label Jan 1, 2022
@xezon xezon force-pushed the terrorist-crash-fixes branch from c4753a9 to cfd6c90 Compare July 15, 2022 22:09
@xezon
Copy link
Collaborator

xezon commented Jul 20, 2022

Potentially obsolete by change

However, does this change still have use? Test if suicide in this branch behaves any better.

@commy2
Copy link
Collaborator

commy2 commented Jul 20, 2022

If the bug is fixed, this PR is superseded.

@xezon xezon force-pushed the terrorist-crash-fixes branch from cfd6c90 to ddc1067 Compare July 20, 2022 14:29
@xezon
Copy link
Collaborator

xezon commented Jul 20, 2022

I tested.

I am unable to observe any difference in damage output of Original vs Hanfield change. Both Terror in Technical and Battle Bus incur big damage on blow up. Behaviour on China mines also looks identical. The only difference I can observe are the visuals during blow up, where in this change the terrorists and their particles are visible during the death of the vehicle, which looks visually much more impressive. If we care for this visual, then this change seems cool.

Original Bang bus blow up

shot_20220720_162908_1

Original Bang bus aftermath

shot_20220720_162909_2

New Bang bus blow up

shot_20220720_162742_1

New Bang bus aftermath

shot_20220720_162744_2

@xezon xezon added Design Is a matter of game design Minor Severity: Minor < Major < Critical < Blocker and removed Bug Something is not working right labels Jul 20, 2022
@xezon xezon force-pushed the terrorist-crash-fixes branch from ddc1067 to 9b36395 Compare July 21, 2022 16:27
@xezon
Copy link
Collaborator

xezon commented Jul 23, 2022

I will wrap up this change some time soon. Change is nice.

@xezon xezon self-assigned this Jul 23, 2022
@xezon xezon force-pushed the terrorist-crash-fixes branch 5 times, most recently from 502a447 to d71a0bc Compare July 26, 2022 15:08
@xezon
Copy link
Collaborator

xezon commented Jul 26, 2022

I now tweaked the change as much as I can and tested Terrorist killing, crushing and driving over mines. All looked good. Crushing Terrorist in Vehicle will just kill it without them exploding in both Original and Patch. Sometimes the Terrorist just evacuate and survive when driving over mines, but that also happens in both Original and Patch. I did not apply the changes to non Skirmish Factions, because with GC_Chem the setup would be a nightmare.

@commy2
Copy link
Collaborator

commy2 commented Jul 26, 2022

So what exactly changed here? From my understanding, the CTD was already fixed by removing the bugged suicide weapon from the Tox Terrorist.

@xezon
Copy link
Collaborator

xezon commented Jul 26, 2022

The only thing I see changed here is what you can see in the images above. The terrorists and their effects are visible when their host vehicle dies. Other than that it appears to behaves the same.

@commy2
Copy link
Collaborator

commy2 commented Jul 26, 2022

I see. So this is an entirely visual change. Pull request title should reflect that.

@xezon xezon changed the title Terrorists should no longer crash the game under certain conditions Terrorists are able to exit and die outside of destroyed vehicles Jul 26, 2022
@xezon
Copy link
Collaborator

xezon commented Jul 26, 2022

Done.

@commy2
Copy link
Collaborator

commy2 commented Jul 26, 2022

DestroyRidersWhoAreNotFreeToExit is added to vGLA Bus, but not the others.

@xezon xezon force-pushed the terrorist-crash-fixes branch from d71a0bc to 1d38815 Compare July 26, 2022 16:09
@alanblack166
Copy link
Contributor Author

Maybe because the commit is not final?

@xezon
Copy link
Collaborator

xezon commented Jul 26, 2022

DestroyRidersWhoAreNotFreeToExit is added to vGLA Bus, but not the others.

Oh right, I forgot. Will add them :)

@xezon xezon force-pushed the terrorist-crash-fixes branch from 1d38815 to f27fefa Compare July 26, 2022 18:09
@xezon xezon changed the title Terrorists are able to exit and die outside of destroyed vehicles Fix GLA Terrorist passenger crash Jul 26, 2022
@xezon
Copy link
Collaborator

xezon commented Jul 26, 2022

After a bit more conversation on Discord about it, we figured proper fix for the original crash issue. This fix now no longer crashes the game with GLA Terrorists in specific scenarios.

@xezon xezon added Bug Something is not working right Critical Severity: Minor < Major < Critical < Blocker and removed Design Is a matter of game design Minor Severity: Minor < Major < Critical < Blocker labels Jul 26, 2022
@commy2 commy2 self-assigned this Jul 27, 2022
@xezon xezon mentioned this pull request Jul 27, 2022
Merged
@alanblack166 @xezon
Terrorists should no longer crash the game under certain conditions
41f2a57 
Terrorists crashed the game in two scenarios:
1. When Terrorists are inside of a Battle Bus and have less than 50% health, the game would crash as soon as the Battle Bus turned into a bunker.
2. When Terrorists are inside of a Technical, the game would crash whenever they get hit by a neutron shell.
This commit fixes the crash in both instances.
@xezon xezon force-pushed the terrorist-crash-fixes branch from f27fefa to 41f2a57 Compare July 27, 2022 17:10
@xezon
Copy link
Collaborator

xezon commented Jul 27, 2022

I had to revert the changes to GC_Chem because the crashed the game on load.

@xezon
Copy link
Collaborator

xezon commented Jul 28, 2022

Maybe close. Not sure.

@commy2 commy2 closed this Jul 28, 2022
@commy2 commy2 removed their assignment Aug 14, 2022
@xezon xezon deleted the terrorist-crash-fixes branch August 30, 2022 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@commy2 commy2 Awaiting requested review from commy2

Assignees
No one assigned
Labels
Bug Something is not working right Critical Severity: Minor < Major < Critical < Blocker
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@alanblack166 @commy2 @xezon