-
Notifications
You must be signed in to change notification settings - Fork 121
[GEN][ZH] Fix tunnel contain crashes due to uninitialized tunnel systems #1310
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
Conversation
ecd478d to
3e24e57
Compare
|
Small change, i initially missed that |
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp
Outdated
Show resolved
Hide resolved
|
I remember I also added these null checks a while ago on tunnel crash but then scrapped them when I found the error higher up. In this case, error cannot be fixed higher up? |
|
In this instance the problem is the neutral player again. And just using normal actions within the game, like those mentioned in the list of crashes this prevents. The way to properly fix this is to give the neutral player a tunnel system, but its part of the xfercrc so would mismatch the game. Same problems with the player template and other neutral player related issues. Theres nothing really higher up that can catch the missing tunnel system. And it shouldn't be handled higher up since tunnel systems aren't used beyond tunnel contain. |
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp
Outdated
Show resolved
Hide resolved
GeneralsMD/Code/GameEngine/Source/GameLogic/Object/Contain/TunnelContain.cpp
Outdated
Show resolved
Hide resolved
3e24e57 to
56466c7
Compare
|
Cleaned it up and refactored it a little based on comments |
56466c7 to
9f6ba25
Compare
|
last push was jsut some small formatting tweaks |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about we simply call the OpenContain::orderAllPassengersToExit here?
After the null check above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah that can work, it means the implementation is still left within the open contain variant, we just have a few extra function calls in the end but it should not be a lot of extra overhead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tweaked all the overridden function to call their base opencontain variant instead of reimplementing the internal logic.
9f6ba25 to
81b3f68
Compare
|
Tweaked and rebased with recent main. |
xezon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks very good.
This PR fixes various tunnel system related crashes involving the neutral player.
The initial issue was brought up by the replay attached to the associated issue. A players sneak attack tunnel was instantly subdued by a microwave tank, preventing it from fully deploying. This resulted in a crash when the player surrendered.
The tunnel became neutral and the damage being caused by the surrender attempted to retrieve the tunnel system on the neutral player.
The issue is that the neutral player does not have an associated tunnel system.
Other crashes with neutral player tunnels can also occur within kill all contained and other various contain related functions. So i added checks within all of these functions.
A few functions already checked for the presence of a valid player and valid tunnel system on that player, so it seems that it has been an issue in the past for the EA developers.
I also added overrides for some of the functions which were originally being called from the opencontain parent, this was to make it easier to implement the required functionality without adding extra null checks within opencontain.
This Fixes: